Storm of the Dead v0.2

[Moderator]

12:38, 21st Dec 2009
TriggerHappy:

Review for Spell

Thanks for using UnitAlive. But I despise your FirstOfGroup loop.

Status

Feel free to message me here if you have any issues with my review or if you have updated your resource and want it reviewed again.

Approved

 

[X-OMG-X]

Could you please post the triggers in the description?
I think 3 levels is enought.
Also i think the damage is a little bit overpowered.

 

[Noxtran]

The trigger

function Trig_Storm_of_the_Dead_Conditions takes nothing returns boolean
return GetSpellAbilityId()=='A000'
endfunction

function StormGroup takes nothing returns boolean
return IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(GetTriggerUnit()))==true and IsUnitType(GetFilterUnit(),UNIT_TYPE_STRUCTURE)==false and IsUnitType(GetFilterUnit(),UNIT_TYPE_DEAD)==false
endfunction

function StormDamage takes nothing returns nothing
call UnitDamageTarget(GetTriggerUnit(),GetEnumUnit(),(7.00*GetUnitAbilityLevel(GetTriggerUnit(),'A000'))*GetHeroInt(GetTriggerUnit(),true),true,false,ATTACK_TYPE_NORMAL,DAMAGE_TYPE_UNIVERSAL,WEAPON_TYPE_WHOKNOWS)
endfunction

function Trig_Storm_of_the_Dead_Actions takes nothing returns nothing
local integer a=1
local location loc=GetSpellTargetLoc()
local location array l
local group g
call PolledWait(0.70)
set g=GetUnitsInRangeOfLocMatching(400.00,loc,Condition(function StormGroup))
loop
    exitwhen a>20
    set l[1]=PolarProjectionBJ(loc,100.00, (a*18.00))
    call DestroyEffect( AddSpecialEffectLoc("Abilities\\Spells\\Other\\Monsoon\\MonsoonBoltTarget.mdl",l[1]))
    call RemoveLocation(l[1])
    set l[2]=PolarProjectionBJ(loc,200.00, (a*18.00))
    call DestroyEffect( AddSpecialEffectLoc("Abilities\\Spells\\Other\\Monsoon\\MonsoonBoltTarget.mdl",l[2]))
    call RemoveLocation(l[2])
    set l[3]=PolarProjectionBJ(loc,300.00, (a*18.00))
    call DestroyEffect( AddSpecialEffectLoc("Abilities\\Spells\\Other\\Monsoon\\MonsoonBoltTarget.mdl",l[3]))
    call RemoveLocation(l[3])
    set l[4]=PolarProjectionBJ(loc,400.00, (a*18.00))
    call DestroyEffect( AddSpecialEffectLoc("Abilities\\Spells\\Other\\Monsoon\\MonsoonBoltTarget.mdl",l[3]))
    call RemoveLocation(l[4])
    set a=a + 1
endloop
call ForGroup(g,function StormDamage)
set a=0
call RemoveLocation(loc)
set loc=null
set l[1]=null
set l[2]=null
set l[3]=null
set l[4]=null
call DestroyGroup(g)
set g=null
endfunction

//===========================================================================
function InitTrig_Storm_of_the_Dead takes nothing returns nothing
local trigger T=CreateTrigger()
call TriggerRegisterAnyUnitEventBJ(T,EVENT_PLAYER_UNIT_SPELL_EFFECT)
call TriggerAddCondition(T,Condition(function Trig_Storm_of_the_Dead_Conditions))
call TriggerAddAction(T,function Trig_Storm_of_the_Dead_Actions)
set T=null
endfunction

the damage is overpowered, and the mana cost is too much
no bug found

 

[axleion]

well,
1. your testmap is... bad, I mean about the ability requirement levels.
2. you can just loop through units instead of using forgroup call.

 

[Noxtran]

well,
1. your testmap is... bad, I mean about the ability requirement levels.
2. you can just loop through units instead of using forgroup call.

thx for ur comment
btw, i'm a newbie on JASS

 

[YourNameHere]

Your code is just.. horrible. Looks more like converted GUI with a little touch of locals.
Using locations and one local group everytime is just inefficient hence you have to ability to do better with JASS.
Lurk around some tutorials and learn how to code better.

 

[Septimus]

seems you could use a much better loop. :=/

 

[Kingz]

Afaik ForGroup() is far better than looping through units, easier not, but better yes

Also this is a converted GUI trigger, you can conclude that from the functions names.

Notes:
Don't use locations.
Don't use PolarProjectionBJ, ever.
Add the use of GroupUtils and use it's constant group, to avoid group leaking(even destroyed groups leak a bit(afaik), use GroupUtils() to recycle groups).

EDIT:

If you don't have JNGP, get it ASAP.

 

[RadU]

Hi! I can't open none of these models,skins or spells :|.I think I have a very old version of editor :|.Can someone send me a new version of editor ? Email me at [email protected]. Thanks !

 

[pippo]

Can't this be made in GUI, too?

 

[real138]

most spells can be made in GUI so i would say yes.

 

[Noxtran]

most spells can be made in GUI so i would say yes.

yeah u right
you can make this spell using GUI
thats simple

 

[real138]

so all it does is create lightning and all units in the area take damage based on casters int. making sure lol cuz thats really simple

 

[Destroyer95]

Can't this be made in GUI, too?

Almost all, or maybe all spells can be made in gui if you're good lol.

As for the spell, first thing I saw when I looked was 4 PolarProjections. Like Kingz said, don't EVER use them. Get JNGP and use global constant integers for spell id and stuff like that. Makes it easier to modify. Also, you could use a global group and add units to it and remove them when you damage them. It's instant, so it won't bug.

 

[Noxtran]

Almost all, or maybe all spells can be made in gui if you're good lol.

As for the spell, first thing I saw when I looked was 4 PolarProjections. Like Kingz said, don't EVER use them. Get JNGP and use global constant integers for spell id and stuff like that. Makes it easier to modify. Also, you could use a global group and add units to it and remove them when you damage them. It's instant, so it won't bug.

i did it already =_=

 

[Kingz]

Declare the UnitAlive native like this:

native UnitAlive takes unit u returns boolean

 

[Noxtran]

Declare the UnitAlive native like this:

native UnitAlive takes unit u returns boolean

i declared already, and this spell is approved now

 

[Destroyer95]

Post your code at the main post and use

[HIDDEN=Code][/HIDDEN]

tags.
Gratz on aproval

@TriggerHappy:

Why do you hate group loops so much? I always prefered to have it all in one function. Makes it easier to read. ForGroup may be faster, but I'll always use group loops.

 

[Kingz]

FirstOfGroup() Is extremely unefficent, ForGroup() function is made for a purpose, use it

 

[Noxtran]

Post your code at the main post and use

[HIDDEN=Code][/HIDDEN]

tags.
Gratz on aproval

@TriggerHappy:

Why do you hate group loops so much? I always prefered to have it all in one function. Makes it easier to read. ForGroup may be faster, but I'll always use group loops.

here's the code

Trigger?

scope StormoftheDead initializer Init

//===========================================================================
//=============================CONSTANT VARS=================================
//===========================================================================
    native UnitAlive takes unit id returns boolean
    
    globals
        private constant integer SPELL_ID = 'A000'
        private constant integer DUMMY_ID = 'h000'
        private constant string EFFECT = "Abilities\\Spells\\Other\\Monsoon\\MonsoonBoltTarget.mdl" //FX
        private constant real RADIUS = 400.
        private constant attacktype A_TYPE = ATTACK_TYPE_NORMAL        
        private constant damagetype D_TYPE = DAMAGE_TYPE_UNIVERSAL
        private constant real ANGLE = 18.
        private constant real RANGE1 = 100.
        private constant real RANGE2 = 200.
        private constant real RANGE3 = 300.
        private constant real RANGE4 = 400.
    endglobals

    private function Damage takes integer lvl, unit u returns real
        return lvl*7.*GetHeroInt(u,true)
    endfunction

    private function Get takes unit u returns boolean
    //the units the spell will affect
        return UnitAlive(u) == true and (IsUnitType(u, UNIT_TYPE_STRUCTURE) == false) and (IsUnitType(u, UNIT_TYPE_MAGIC_IMMUNE) == false)
    endfunction
//===========================================================================
//===================================END=====================================
//===========================================================================
    globals
        private group g
        private boolexpr b
        private group h
    endglobals

//===========================================================================
    //blade.dk copy group
    private function CopyGroup takes group g returns group
        set bj_groupAddGroupDest = CreateGroup()
        call ForGroup(g, function GroupAddGroupEnum)
        return bj_groupAddGroupDest
    endfunction
    
//===========================================================================
    private function Pick takes nothing returns boolean
        return Get(GetFilterUnit())
    endfunction   
    
//===========================================================================    
    private function Conditions takes nothing returns boolean
        return GetSpellAbilityId() == SPELL_ID
    endfunction
    
//===========================================================================
    private function Actions takes nothing returns nothing
        local unit a = GetTriggerUnit()
        local unit u
        local real locX = GetSpellTargetX()
        local real locY = GetSpellTargetY()
        local real x
        local real y
        local integer i = 0
        local integer lvl = GetUnitAbilityLevel(a,SPELL_ID)

        
        call GroupEnumUnitsInRange(g, locX, locY, RADIUS, b)
        set h = CopyGroup(g)

        loop
            set u = FirstOfGroup(h)
            exitwhen(u == null)
            call GroupRemoveUnit(h, u)
            if IsUnitEnemy(u, GetOwningPlayer(a)) then
                call UnitDamageTarget(a, u, Damage(lvl,a), true, false, A_TYPE, D_TYPE, null)
            endif
        endloop
    
        loop
            exitwhen i > 20
            //1
            set x = locX + RANGE1 * Cos((i * ANGLE) * bj_DEGTORAD)
            set y = locY + RANGE1 * Sin((i * ANGLE) * bj_DEGTORAD)
            call DestroyEffect(AddSpecialEffect(EFFECT, x, y))
            //2
            set x = locX + RANGE2 * Cos((i * ANGLE) * bj_DEGTORAD)
            set y = locY + RANGE2 * Sin((i * ANGLE) * bj_DEGTORAD)
            call DestroyEffect(AddSpecialEffect(EFFECT, x, y))
            //3
            set x = locX + RANGE3 * Cos((i * ANGLE) * bj_DEGTORAD)
            set y = locY + RANGE3 * Sin((i * ANGLE) * bj_DEGTORAD)
            call DestroyEffect(AddSpecialEffect(EFFECT, x, y))
            //4
            set x = locX + RANGE4 * Cos((i * ANGLE) * bj_DEGTORAD)
            set y = locY + RANGE4 * Sin((i * ANGLE) * bj_DEGTORAD)
            call DestroyEffect(AddSpecialEffect(EFFECT, x, y))
            set i = i + 1
        endloop
    
        set a = null
        set u = null
    endfunction
    
//===========================================================================
    private function Init takes nothing returns nothing
        local trigger StormoftheDeadTrg = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(StormoftheDeadTrg, EVENT_PLAYER_UNIT_SPELL_EFFECT )
        call TriggerAddCondition(StormoftheDeadTrg, Condition( function Conditions ) )
        call TriggerAddAction( StormoftheDeadTrg, function Actions )

        set g = CreateGroup()
        set h = CreateGroup()
        set b = Condition(function Pick)
        call Preload(EFFECT)
        
        //preloads ability
        set bj_lastCreatedUnit = CreateUnit(Player(PLAYER_NEUTRAL_PASSIVE), DUMMY_ID, 0, 0, 0)
        call UnitAddAbility(bj_lastCreatedUnit, SPELL_ID)
        call KillUnit(bj_lastCreatedUnit)
    endfunction
endscope

 

[Destroyer95]

FirstOfGroup() Is extremely unefficent, ForGroup() function is made for a purpose, use it

It has a purpose? xD
I just thought it was made for GUI but ok, you're the boss. I don't know, but it seems much better when it's in one function. And when doing some complicated stuff, you'll have to find a way (found one - hashtables) to transfer data to the group function. And added to all that, the first tut I read used a group loop.

 

[Noxtran]

It has a purpose? xD
I just thought it was made for GUI but ok, you're the boss. I don't know, but it seems much better when it's in one function. And when doing some complicated stuff, you'll have to find a way (found one - hashtables) to transfer data to the group function. And added to all that, the first tut I read used a group loop.

Using "ForGroup" isn't neat for me..
but ForGroup is very useful on complicated stuff, such as knocbacking all units in one Unit Group

 

[Destroyer95]

Well it's just as easy to use a loop if you use a struct for knockbacking. That's what I think.
Oh by the way, Kingz, do you wanna judge in a spellmaking contest between me and real138?