[Spell] Need help with this spell

[Spartipilo]

The function you made is somehow "the same". It's kinda usless thou it works.

There is no static value, because what you're doing is the SUM once per instance. The only value that changes in the loop is the angle. You don't need to sum x/y + 300/600/900 72 times every 0.2 seconds when you can do it just 6 times and achieve the same results.

EDIT: You're right, the multiplication comes first before the sum. In that case do the multiplication by the "ca" and "sa" variables. it still improves the efficiency a lot.

 

[deathismyfriend]

@dimf Okay, thanks for the info. Now I know what udg_ stands for. By the way, did you receive the pm I sent you?

np and yes i sent u one back

 

[sniper_zero]

A global group can still be MUI if the filtering is composed of locals, right? It's not much of an issue even if some unwanted units are added there as long as you're picking the right units.

Do I understand it correctly?

 

[Spartipilo]

GroupEnumUnitsInRange clears the group before adding units, so, you're good with a global group for instant group actions.

 

[sniper_zero]

Okay, thanks for the info. That was worrying me for a bit even though the spell seems to work fine.

EDIT:
If you do just this,
local <variabletype> name
what's the value at the start? Is it null? Also, do you still need to null it at the end even if you don't assign any value to it?

 

[Spartipilo]

The value of a local variable with no allocated data is null, and doesn't leak if you don't destroy/null it later, since there is nothing o null/destroy.

EDIT: Did you finish de spell? Would you post the script here?

 

[sniper_zero]

Sure, let me just remove some things first. I went a bit overboard with the nulling.

EDIT: The commentary hasn't improved much though.

The value of a local variable with no allocated data is null, and doesn't leak if you don't destroy/null it later, since there is nothing o null/destroy.

This goes for variables in the globals block too I assume?

 

[Spartipilo]

Yeah. Globals with no value are null by default and don't leak because you can access them any time you want, but they do reserve a bit of memory even when no data is stored since it's expected that you put something there. Anyway, you could have hundreds and thousands of them with no value and you wouldn't even notice it.

Locals instead are recycled when the function ends. The idea of "nullying" them is clear the data they hold before they get recycled and you loose the access to them, therefore creating leaks.

 

[sniper_zero]

trigger

function PolarProjectionX takes real x, real dist, real angle returns real
    return x + dist * Cos(angle)
endfunction

function PolarProjectionY takes real y, real dist, real angle returns real
    return y + dist * Sin(angle)
endfunction

function Vanishing_World_Periodic takes nothing returns nothing
    //Variable setup
    local timer t = GetExpiredTimer()
    local integer id = GetHandleId(t)
    local unit u = LoadUnitHandle(udg_Hash, id, 0)
    local real tx = LoadReal(udg_Hash, id, 1)
    local real ty = LoadReal(udg_Hash, id, 2)
    local real final_damage = LoadReal(udg_Hash, id, 3)
    local real periodic_damage = LoadReal(udg_Hash, id, 4)
    local integer count = LoadInteger(udg_Hash, id, 5)
    local real rand_distance = GetRandomReal(200, 900)
    local real rand_angle = GetRandomReal(0, 6.263)
    local real rx = tx + rand_distance * Cos(rand_angle)
    local real ry = ty + rand_distance * Sin(rand_angle)
    local unit u1 //will be used for FirstofGroup
    local unit shadowball
    local real sx
    local real sy
    local group g
    local unit u2
    local integer i = 0
    local real angle = 0.314
    local real px //x coordinate of the polar projection
    local real py //y coordinate of the polar projection
    local player owner = GetOwningPlayer(u)
    //End of variable setup
    
    if count < 31 then
        set count = count + 1// 1 Shadow Ball has spawned.
        call SaveInteger(udg_Hash, id, 5, count)
        call BJDebugMsg(I2S(count))
        call BJDebugMsg(I2S(id))
        if count < 25 then //It'll keep creating shadow balls until the 25th tick
            set g = CreateGroup()
            set shadowball = CreateUnit(owner, 'e001', rx, ry, 270)
            set sx = GetUnitX(shadowball)
            set sy = GetUnitY(shadowball)
            call KillUnit(shadowball)
            call DestroyEffect(AddSpecialEffect("DarkNova.mdx", sx, sy))
            call GroupEnumUnitsInRange(g, sx, sy, 350, null)
            loop
                set u1 = FirstOfGroup(g)
                exitwhen u1 == null
                if IsUnitEnemy(u1, owner) then
                    call UnitDamageTarget(u, u1, periodic_damage, false, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL, null)
                endif
                call GroupRemoveUnit(g, u1)
            endloop
    //clean up
            set u = null
            set u1 = null
            set u2 = null
            set shadowball = null
            set owner = null
            call DestroyGroup(g)
            set g = null
            set t = null
            
    //Creating the big Shadow Ball at the 26th tick
        elseif count == 26 then
            set shadowball = CreateUnit(owner, 'e002', tx, ty, 270)
            call SaveUnitHandle(udg_Hash, id, 6, shadowball)
            set u = null
            set shadowball = null
            set owner = null
            set t = null
            
        elseif count >26 and count <31 then//to destroy leaks between 26 and 31
            set u = null
            set owner = null
            set t = null
            
    //Waiting for 1 second to pass
        elseif count == 31 then
            set g = CreateGroup()
            set shadowball = LoadUnitHandle(udg_Hash, id, 6)//LOAD the unit because this is periodic trigger uses LOCAL variables. You cannot refer to a variable from a previous instance of this function.
            call KillUnit(shadowball)
            set u2 = CreateUnit(owner, 'e003', tx, ty, 270)
            call KillUnit(u2)
            loop
                exitwhen i == 20
                set i = i + 1
                set px = PolarProjectionX(tx, 300, angle)
                set py = PolarProjectionY(ty, 300, angle)
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set px = PolarProjectionX(tx, 600, angle)
                set py = PolarProjectionY(ty, 600, angle)
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set px = PolarProjectionX(tx, 900, angle)
                set py = PolarProjectionY(ty, 900, angle)
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set angle = angle + 0.314
            endloop
            set i = 0
            set angle = 0.523
            loop
                exitwhen i == 12
                set i = i + 1
                set px = PolarProjectionX(tx, 300, angle)
                set py = PolarProjectionY(ty, 300, angle)
                call DestroyEffect(AddSpecialEffect("StarExplosion.mdx", px, py))
                set px = PolarProjectionX(tx, 600, angle)
                set py = PolarProjectionY(ty, 600, angle)
                call DestroyEffect(AddSpecialEffect("StarExplosion.mdx", px, py))     
                set px = PolarProjectionX(tx, 900, angle)
                set py = PolarProjectionY(ty, 900, angle)
                call DestroyEffect(AddSpecialEffect("StarExplosion.mdx", px, py))       
                set angle = angle + 0.523
            endloop
            call GroupEnumUnitsInRange(g, tx, ty, 950, null)
            loop
                set u1 = FirstOfGroup(g)
                exitwhen u1 == null
                if IsUnitEnemy(u1, GetOwningPlayer(u)) then
                    call UnitDamageTarget(u, u1, final_damage, false, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL, null)
                endif
                call GroupRemoveUnit(g, u1)
            endloop
            call PauseTimer(t)
            call DestroyTimer(t)
            set t = null
            set u = null
            set u1 = null
            set u2 = null
            set shadowball = null
            set g = null
            set owner = null
            call DestroyGroup(g)
            call FlushChildHashtable(udg_Hash, id)            
        endif
    endif    
endfunction

function Vanishing_World_Actions takes nothing returns nothing
    //Variable Setup
    local unit u = GetTriggerUnit()
    local timer t = CreateTimer()
    local real tx = GetSpellTargetX()// X coordinate of the spell target location
    local real ty = GetSpellTargetY()// Y coordinate of the spell target location
    local real final_damage = 1000 * GetHeroInt(u,true)//Final damage at the end
    local real periodic_damage = (GetHeroInt(u,true) * 0.10) * 100//damage during the periodic actions
    local integer id = GetHandleId(t)//Id of the timer, will be used as the parentKey
    //End of variable set up
    
    //Setting up the hashtable
    call SaveUnitHandle(udg_Hash, id, 0, u)
    call SaveReal(udg_Hash, id, 1, tx)
    call SaveReal(udg_Hash, id, 2, ty)
    call SaveReal(udg_Hash, id, 3, final_damage)//I'm storing the damage so the spell will still deal damage even if the Hero dies.
    call SaveReal(udg_Hash, id, 4, periodic_damage)
    call SaveInteger(udg_Hash, id, 5, 0)//Storing the count so, the trigger will know when to stop.
    //End of hashtable setup
    
    // Start of the periodic loop
    call TimerStart(t, 0.2, true, function Vanishing_World_Periodic)
    set t = null
endfunction

function Trig_Vanishing_World_v2_Conditions takes nothing returns boolean
    if GetSpellAbilityId() == 'A000' then
        call Vanishing_World_Actions()
    endif
    return false
endfunction

//===========================================================================
function InitTrig_Vanishing_World_v2 takes nothing returns nothing
    local trigger t = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( t, Condition( function Trig_Vanishing_World_v2_Conditions ) )
    set udg_Hash = InitHashtable()
    set t = null
endfunction

It still doesn't use the global group because when I tried it, none of the units were being damaged. Still working on a way to fix that. Until then, I'm currently sticking with this one.

 

[Spartipilo]

Check the test map i gave before and see how I use the global group for damage .

            set shadowball = CreateUnit(owner, 'e001', rx, ry, 270)
            set sx = GetUnitX(shadowball) // You already know this. It's = rx
            set sy = GetUnitY(shadowball) // You already know this. It's = ry
            // so do this
            set sx = rx
            set sy = ry

            set shadowball = CreateUnit(owner, 'e001', rx, ry, 270)
            set sx = GetUnitX(shadowball)
            set sy = GetUnitY(shadowball)
            call KillUnit(shadowball) // Why do you kill the unit you just created?

- No need to null u1 since you exit loop when u1 == null

- In the count < 25 ITE, null only the variables you declared inside, which are "shadowball", "g", and "DestroyGroup(g)" (though you should replace it with the global). The rest of them you declared in the function header must be nulled just once at the end of the function, and not inside each ITE.

- In count == 26 just null "shadowball". The rest must be nulled at the end of the function.

- No need for this. These are all nulled just once at the end of the function.

        elseif count >26 and count <31 then//to destroy leaks between 26 and 31
            set u = null
            set owner = null
            set t = null

- This can still be improved a lot with some of the tips i gave you before. There's no need for those functions.

            loop
                exitwhen i == 20
                set i = i + 1
                set px = PolarProjectionX(tx, 300, angle)
                set py = PolarProjectionY(ty, 300, angle)
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set px = PolarProjectionX(tx, 600, angle)
                set py = PolarProjectionY(ty, 600, angle)
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set px = PolarProjectionX(tx, 900, angle)
                set py = PolarProjectionY(ty, 900, angle)
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set angle = angle + 0.314
            endloop

- See here, at the end of the function

            set g = null // You null THIS
            set owner = null
            call DestroyGroup(g) // Then you DestroyGroup(null)

- Since you declared "owner", "u" and "t" in the function header, you have to null these 3 at the end of the function outside any ITE.

 

[sniper_zero]

Hold on, I'll try to comment on each one.

Thanks for pointing that out. I change it as soon as possible.

I kill it for the eye candy.

I wanted to be sure with the leak removal but I'll change it now.

So I should remove the whole elseif block for 26 and 31?

If I did it that way, I won't get a circle anymore. The order of operations gets screwed up so the coordinates do as well. I tried to multiply the distances and Cos/Sin(angles) first but they just returned static values. I'll check again if I can do something about that.

About the order of nulling and destroying, it was Maker who told me that I should Destroy groups and timers first before declaring them as null.

I'll change those immediately.

EDIT: I really can't do what you suggested for the polar projection. I tried modifying it to make it use the right order of operations but the value of angle is the problem. If I give it a value, cos and sin remain static. If I don't, the polar projection won't even happen. I can't do the calculation inside the loop either because that would be doing what you told me not to do in the first place.

loopy

function loopy takes nothing returns nothing
    local unit u = GetTriggerUnit()
    local real px = GetUnitX(u)
    local real py = GetUnitY(u)
    local real angle//If I set a value to this, cos and sin will use the same values throughout the loop, making the circle shape impossible.
//So I have to change the value inside the loop.
    local real cos = Cos(angle)//Will not work properly because I'm getting the Cos of a null value.
    local real sin = Sin(angle)
    local real lx
    local real ly
    local real lx1 = 200 * cos
    local real ly1 = 200 * sin
    local integer i = 0
    
    loop
        exitwhen i == 20
        set i = i+1
        set angle = 0.314
        set lx = px + lx1
        set ly = py + ly1
        call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", lx, ly))
        set angle = angle + 0.314
    endloop
endfunction

 

[Spartipilo]

Son... please xD

I'm telling you to declare the Cos and Sin of the angle INSIDE THE LOOP. Else, you're looking for Cos(angle) and Sin(angle) 6 times per loop instance, and they're all returning the same value.

You did the opposite of what maker told you to, i guess. There is no use in nullying "g" and then destroying "g". You should invert the order of those 2. First destroy the "g" group, then null the "g" variable.

So I should remove the whole elseif block for 26 and 31?

I guess, if anything inside is needed

// replace this
            loop
                exitwhen i == 20
                set i = i + 1
                set px = PolarProjectionX(tx, 300, angle)
                set py = PolarProjectionY(ty, 300, angle)
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set px = PolarProjectionX(tx, 600, angle)
                set py = PolarProjectionY(ty, 600, angle)
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set px = PolarProjectionX(tx, 900, angle)
                set py = PolarProjectionY(ty, 900, angle)
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set angle = angle + 0.314
            endloop

// with this
    local real ca // c = cos. a = angle. ca = Cos(angle)
    local real sa // s = sin. a = angle. sa = Sin(angle)
            loop
                exitwhen i == 20
                set i = i + 1
                set ca = Cos(angle)
                set sa = Sin(angle)
                set px = tx + 300 * ca
                set py = ty + 300 * sa
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set px = tx + 600 * ca
                set py = ty + 600 * sa
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set px = tx + 900 * ca
                set py = ty + 900 * sa
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set angle = angle + 0.314
            endloop

I think you can avoid creating the unit and killing it, and just use the unit model as an special effect. Destroying special effects playis the special effect death animation, which is the same is played when a unit dies. Try it, since destroying special effect is a lot better than creating/killing unit (creating units leaks even if you remove them and null variables later, so it's better to avoid unneccesary creations. That's why there are dummy recycling systems).

 

[sniper_zero]

http://www.hiveworkshop.com/forums/jass-class-702/inquiry-order-doing-things-238296/#post2385673

I don't think I did the opposite.

Making it as a unit allows me to change its scaling...so yeah.

I'll work be working on the loop now.

 

[Spartipilo]

Oooh.. you're right... The scaling... You could modify the model thou

 

[Spartipilo]

Maker said: Destroy then null.

You did: null then destroy

            set g = null // NULL
            set owner = null // THEN
            call DestroyGroup(g) // DESTROY
            // At this point "DestroyGroup(g)" is equal to "DestroyGroup(null)" since "g = null"

BTW i don't mean to be "mean" or criticize you inside all this. I just read your code and give an opinion about how it would be improved (since i'm a bit obssesed with logic, math, programming and efficiency). I know i'm a bit tough sometimes, so, please, don't take this personal

 

[sniper_zero]

You're right. My bad...I swear the script gets harder to check every time.

I could but I'm not that good that I can modify the model just like that. Plus, I don't have permission from the maker mostly because I don't remember who the maker is anymore.

 

[Spartipilo]

You'll get the hang of it in no time. This is way more advanced that my first attempts to work with jass.

 

[sniper_zero]

I'll post the script again later when I get home.

 

[sniper_zero]

trigger

globals
    group Vanish = CreateGroup()
endglobals

function Vanishing_World_Periodic takes nothing returns nothing
    //Variable setup
    local timer t = GetExpiredTimer()
    local integer id = GetHandleId(t)
    local unit u = LoadUnitHandle(udg_Hash, id, 0)
    local real tx = LoadReal(udg_Hash, id, 1)
    local real ty = LoadReal(udg_Hash, id, 2)
    local real final_damage = LoadReal(udg_Hash, id, 3)
    local real periodic_damage = LoadReal(udg_Hash, id, 4)
    local integer count = LoadInteger(udg_Hash, id, 5)
    local real rand_distance = GetRandomReal(200, 900)
    local real rand_angle = GetRandomReal(0, 6.263)
    local real rx = tx + rand_distance * Cos(rand_angle)
    local real ry = ty + rand_distance * Sin(rand_angle)
    local unit u1 //will be used for FirstofGroup
    local unit shadowball
    local unit u2
    local integer i = 0
    local real angle = 0.314
    local real px //x coordinate of the polar projection
    local real py //y coordinate of the polar projection
    local real ca
    local real sa
    local player owner = GetOwningPlayer(u)
    //End of variable setup
    
    if count < 31 then
        set count = count + 1// 1 Shadow Ball has spawned.
        call SaveInteger(udg_Hash, id, 5, count)
        //call BJDebugMsg(I2S(count))
        //call BJDebugMsg(I2S(id))
        if count < 25 then //It'll keep creating shadow balls until the 25th tick
            set shadowball = CreateUnit(owner, 'e001', rx, ry, 270)
            call KillUnit(shadowball)
            call DestroyEffect(AddSpecialEffect("DarkNova.mdx", rx, ry))
            call GroupEnumUnitsInRange(Vanish, rx, ry, 350, null)
            loop
                set u1 = FirstOfGroup(Vanish)
                exitwhen u1 == null
                if IsUnitEnemy(u1, owner) then
                    call UnitDamageTarget(u, u1, periodic_damage, false, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL, null)
                endif
                call GroupRemoveUnit(Vanish, u1)
            endloop
    //clean up
            set shadowball = null
            //set Vanish = null
            
    //Creating the big Shadow Ball at the 26th tick
        elseif count == 26 then
            set shadowball = CreateUnit(owner, 'e002', tx, ty, 270)
            call SaveUnitHandle(udg_Hash, id, 6, shadowball)
            set shadowball = null            
    //Waiting for 1 second to pass
        elseif count == 31 then
            set shadowball = LoadUnitHandle(udg_Hash, id, 6)//LOAD the unit because this is periodic trigger uses LOCAL variables. You cannot refer to a variable from a previous instance of this function.
            call KillUnit(shadowball)
            set u2 = CreateUnit(owner, 'e003', tx, ty, 270)
            call KillUnit(u2)
            loop//Just creates the SFX
                exitwhen i == 20
                set i = i + 1
                set ca = Cos(angle)
                set sa = Sin(angle)
                set px = tx + 300 * ca
                set py = ty + 300 * sa
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set px = tx + 600 * ca
                set py = ty + 600 * sa
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set px = tx + 900 * ca
                set py = ty + 900 * sa
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set angle = angle + 0.314
            endloop
            set i = 0
            set angle = 0.523
            loop//Creates another batch of SFX
                exitwhen i == 12
                set i = i + 1
                set ca = Cos(angle)
                set sa = Sin(angle)
                set px = tx + 300 * ca
                set py = ty + 300 * sa
                call DestroyEffect(AddSpecialEffect("StarExplosion.mdx", px, py))
                set px = tx + 600 * ca
                set py = ty + 600 * sa
                call DestroyEffect(AddSpecialEffect("StarExplosion.mdx", px, py))     
                set px = tx + 600 * ca
                set py = ty + 600 * sa
                call DestroyEffect(AddSpecialEffect("StarExplosion.mdx", px, py))       
                set angle = angle + 0.523
            endloop
            call GroupEnumUnitsInRange(Vanish, tx, ty, 950, null)
            loop
                set u1 = FirstOfGroup(Vanish)
                exitwhen u1 == null
                if IsUnitEnemy(u1, owner) then
                    call UnitDamageTarget(u, u1, final_damage, false, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL, null)
                endif
                call GroupRemoveUnit(Vanish, u1)
            endloop
            call PauseTimer(t)
            call DestroyTimer(t)
            set t = null
            set u = null
            set u1 = null
            set u2 = null
            set shadowball = null
            //call DestroyGroup(Vanish)            
            //set Vanish = null
            set owner = null
            call FlushChildHashtable(udg_Hash, id)            
        endif
    endif
    
    //Clean up
    set u = null
    set owner = null
    set t = null
endfunction

function Vanishing_World_Actions takes nothing returns nothing
    //Variable Setup
    local unit u = GetTriggerUnit()
    local timer t = CreateTimer()
    local real tx = GetSpellTargetX()// X coordinate of the spell target location
    local real ty = GetSpellTargetY()// Y coordinate of the spell target location
    local real heroint = GetHeroInt(u,true)
    local real final_damage = 1000 * heroint//Final damage at the end
    local real periodic_damage = (heroint * 0.10) * 100//damage during the periodic actions
    local integer id = GetHandleId(t)//Id of the timer, will be used as the parentKey
    //End of variable set up
    
    //Setting up the hashtable
    call SaveUnitHandle(udg_Hash, id, 0, u)
    call SaveReal(udg_Hash, id, 1, tx)
    call SaveReal(udg_Hash, id, 2, ty)
    call SaveReal(udg_Hash, id, 3, final_damage)//I'm storing the damage so the spell will still deal damage even if the Hero dies.
    call SaveReal(udg_Hash, id, 4, periodic_damage)
    call SaveInteger(udg_Hash, id, 5, 0)//Storing the count so, the trigger will know when to stop.
    //End of hashtable setup
    
    // Start of the periodic loop
    call TimerStart(t, 0.2, true, function Vanishing_World_Periodic)
    set t = null
endfunction

function Trig_Vanishing_World_v2_Conditions takes nothing returns boolean
    if GetSpellAbilityId() == 'A000' then
        call Vanishing_World_Actions()
    endif
    return false
endfunction

//===========================================================================
function InitTrig_Vanishing_World_v2_Copy_2 takes nothing returns nothing
    local trigger t = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( t, Condition( function Trig_Vanishing_World_v2_Conditions ) )
    set udg_Hash = InitHashtable()
    set t = null
endfunction

Here's the new script. I think all the changes are made. The shadow balls are still units but it can't be helped because of their scaling.

The global group Vanish is neither nulled or destroyed. That's how it's supposed to be, right? It's automatically nulled anyway when u1 is nulled, isn't it?

 

[Spartipilo]

You could add "VanishGroup" and "VanishHash" in the globals block (The variable names helps for easier reading)

No need to null "t" after you pause/Destroy timer, since "t" is nulled at the end of the function every time. same as "u" and "owner". "u1" is already nulled in the loop (exitwhen u1 == null) so, no need to null it.

"local real periodic_damage = (heroint * 0.10) * 100//damage during the periodic actions"

((Int * 0.1) * 100) Is equal to (Int * 10).

You don't use " local real angle = 0.314" untill count == 31. Set angle = 0.314 then, else, you would be setting the value 30 times without giving it any use (resource waste)

The next step to improve this is making it easily configurable. You can create a bunch of globals to manipulate the number of balls to create, the number of loop iterations for the special effects, the angles for them, the ability speed, the count limits, and a lot of stuff, so you can manipulate the whole ability behavior by just changing some values. It makes it easier for you (or anyone if you share this) to make extraordinary changes on many ways and reaaally easy.

 

[sniper_zero]

I'll do the changes.

No need to null "t" after you pause/Destroy timer, since "t" is nulled at the end of the function every time. same as "u" and "owner". "u1" is already nulled in the loop (exitwhen u1 == null) so, no need to null it.

But this...in which part of the script is this?

And configuration, huh? Well, I'm fine with it as it is for now. I'll use it to practice my script checking skills. Besides, I have a feeling it'll be rejected here due to the bit of lag produced by the eye candy at the end.

Well, the slow down only happens to computers without video cards and 1gb of RAM (like mine...) so that may not be an issue. I tested it on a comp with a video card and it doesn't slow down.

By the way, is it okay not to null or destroy VanishGroup?

 

[Spartipilo]

This shouldn't slow down if it's all preloaded.

But this...in which part of the script is this?

R:

            call PauseTimer(t)
            call DestroyTimer(t)
            set t = null // This is always nulled at the end of the function. No need to null here
            set u = null // This is always nulled at the end of the function. No need to null here
            set u1 = null // This is always nulled at the loop. No need to null here.
            set u2 = null
            set shadowball = null
            //call DestroyGroup(Vanish)            
            //set Vanish = null
            set owner = null // This is always nulled at the end of the function. No need to null here
            call FlushChildHashtable(udg_Hash, id)            
        endif
    endif
    
    //Clean up
    set u = null
    set owner = null
    set t = null

Of course, there's no need to null nor destroy the global group

In the Vanish ability I gave you I did some global configurations like number of charges, damages, time of the ability, and else. Everything else in the functions adjusts automatically to these values. You can check there and think about different configurations you could add. (The special effect models and shadowball models too, just in case someone wants the same ability with other models). This is made because when you have 50 or 100 or 900 abilities, you can't keep track of the position of each value of the ability in each script. That's why each ability has a global block of configurations.

This would be awesome avoiding the unit killing, and just using the dying animation as a special effect.

 

[sniper_zero]

This shouldn't slow down if it's all preloaded.

It's more of a specs issue, I think. The particle emitters are just that demanding so they shouldn't be used en masse if your computer doesn't have a video card.

And preloading? As in giving the ability to some dummy unit at map initialization then removing it?

I do think configuration makes it a whole lot easier. I just want to train my eyes for checking my own scripts.

 

[deathismyfriend]

yes that is how preloading works.

see my tutorial things a GUIer should know for more info on preloading.

can u post ur new code when u get a chance. that way i can look at the new stuff to. Even tho spartipilo got pretty much everything lol.

 

[sniper_zero]

This is pretty much what I did although there isn't that much change.

trigger

globals
    hashtable Hash = InitHashtable()
    group Vanish = CreateGroup()
endglobals

function Vanishing_World_Periodic takes nothing returns nothing
    //Variable setup
    local timer t = GetExpiredTimer()
    local integer id = GetHandleId(t)
    local unit u = LoadUnitHandle(Hash, id, 0)
    local real tx = LoadReal(Hash, id, 1)
    local real ty = LoadReal(Hash, id, 2)
    local real final_damage
    local real periodic_damage
    local integer count = LoadInteger(Hash, id, 5)
    local real rand_distance
    local real rand_angle
    local real rx
    local real ry
    local unit u1 //will be used for FirstofGroup
    local unit shadowball
    local unit u2
    local integer i
    local real angle
    local real px //x coordinate of the polar projection
    local real py //y coordinate of the polar projection
    local real ca
    local real sa
    local player owner = GetOwningPlayer(u)
    //End of variable setup
    
    if count < 31 then
        set count = count + 1// 1 Shadow Ball has spawned.
        call SaveInteger(Hash, id, 5, count)
        //call BJDebugMsg(I2S(count))
        //call BJDebugMsg(I2S(id))
        if count < 25 then //It'll keep creating shadow balls until the 25th tick
            set rand_distance = GetRandomReal(200, 900)
            set rand_angle = GetRandomReal(0, 6.263)
            set rx = tx + rand_distance * Cos(rand_angle)
            set ry = ty + rand_distance * Sin(rand_angle)
            set periodic_damage = LoadReal(Hash, id, 4)
            set shadowball = CreateUnit(owner, 'e001', rx, ry, 270)
            call KillUnit(shadowball)
            call DestroyEffect(AddSpecialEffect("DarkNova.mdx", rx, ry))
            call GroupEnumUnitsInRange(Vanish, rx, ry, 350, null)
            loop
                set u1 = FirstOfGroup(Vanish)
                exitwhen u1 == null
                if IsUnitEnemy(u1, owner) then
                    call UnitDamageTarget(u, u1, periodic_damage, false, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL, null)
                endif
                call GroupRemoveUnit(Vanish, u1)
            endloop
    //clean up
            set shadowball = null
            //set Vanish = null
            
    //Creating the big Shadow Ball at the 26th tick
        elseif count == 26 then
            set shadowball = CreateUnit(owner, 'e002', tx, ty, 270)
            call SaveUnitHandle(Hash, id, 6, shadowball)
            set shadowball = null            
    //Waiting for 1 second to pass
        elseif count == 31 then
            set final_damage = LoadReal(Hash, id, 3)
            set i = 0
            set angle = 0.314
            set shadowball = LoadUnitHandle(Hash, id, 6)//LOAD the unit because this is periodic trigger uses LOCAL variables. You cannot refer to a variable from a previous instance of this function.
            call KillUnit(shadowball)
            set u2 = CreateUnit(owner, 'e003', tx, ty, 270)
            call KillUnit(u2)
            loop//Just creates the SFX
                exitwhen i == 20
                set i = i + 1
                set ca = Cos(angle)
                set sa = Sin(angle)
                set px = tx + 300 * ca
                set py = ty + 300 * sa
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set px = tx + 600 * ca
                set py = ty + 600 * sa
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set px = tx + 900 * ca
                set py = ty + 900 * sa
                call DestroyEffect(AddSpecialEffect("DarkPillar.mdx", px, py))
                call DestroyEffect(AddSpecialEffect("Nebula.mdx", px, py)) 
                set angle = angle + 0.314
            endloop
            set i = 0
            set angle = 0.523
            loop//Creates another batch of SFX
                exitwhen i == 12
                set i = i + 1
                set ca = Cos(angle)
                set sa = Sin(angle)
                set px = tx + 300 * ca
                set py = ty + 300 * sa
                call DestroyEffect(AddSpecialEffect("StarExplosion.mdx", px, py))
                set px = tx + 600 * ca
                set py = ty + 600 * sa
                call DestroyEffect(AddSpecialEffect("StarExplosion.mdx", px, py))     
                set px = tx + 900 * ca
                set py = ty + 900 * sa
                call DestroyEffect(AddSpecialEffect("StarExplosion.mdx", px, py))       
                set angle = angle + 0.523
            endloop
            call GroupEnumUnitsInRange(Vanish, tx, ty, 950, null)
            loop
                set u1 = FirstOfGroup(Vanish)
                exitwhen u1 == null
                if IsUnitEnemy(u1, owner) then
                    call UnitDamageTarget(u, u1, final_damage, false, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL, null)
                endif
                call GroupRemoveUnit(Vanish, u1)
            endloop
            call PauseTimer(t)
            call DestroyTimer(t)
            set u2 = null
            set shadowball = null
            //call DestroyGroup(Vanish)            
            //set Vanish = null
            call FlushChildHashtable(Hash, id)            
        endif
    endif
    
    //Clean up
    set u = null
    set owner = null
    set t = null
endfunction

function Vanishing_World_Actions takes nothing returns nothing
    //Variable Setup
    local unit u = GetTriggerUnit()
    local timer t = CreateTimer()
    local real tx = GetSpellTargetX()// X coordinate of the spell target location
    local real ty = GetSpellTargetY()// Y coordinate of the spell target location
    local real heroint = GetHeroInt(u,true)
    local real final_damage = 1000 * heroint//Final damage at the end
    local real periodic_damage = heroint * 10//damage during the periodic actions
    local integer id = GetHandleId(t)//Id of the timer, will be used as the parentKey
    //End of variable set up
    
    //Setting up the hashtable
    call SaveUnitHandle(Hash, id, 0, u)
    call SaveReal(Hash, id, 1, tx)
    call SaveReal(Hash, id, 2, ty)
    call SaveReal(Hash, id, 3, final_damage)//I'm storing the damage so the spell will still deal damage even if the Hero dies.
    call SaveReal(Hash, id, 4, periodic_damage)
    call SaveInteger(Hash, id, 5, 0)//Storing the count so, the trigger will know when to stop.
    //End of hashtable setup
    
    // Start of the periodic loop
    call TimerStart(t, 0.2, true, function Vanishing_World_Periodic)
    set t = null
endfunction

function Trig_Vanishing_World_v2_Conditions takes nothing returns boolean
    if GetSpellAbilityId() == 'A000' then
        call Vanishing_World_Actions()
    endif
    return false
endfunction

//===========================================================================
function InitTrig_Vanishing_World_v2_globals takes nothing returns nothing
    local trigger t = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( t, Condition( function Trig_Vanishing_World_v2_Conditions ) )
    set t = null
endfunction

 

[Spartipilo]

Check this. I used custom functions for damage (wich works in both cases) and for the special effect (wich works in both cases). I also added some globals to manipulate a bit of the behavior. I also improved the script by removing unneccesary locals and some other stuff like this:

set u = CreateUnit()
call KillUnit(u)
// Turned to 
call KillUnit(CreateUnit())

The script, just as it is, should work exactly as the one you have since I changed anything in the values.

Hidden

globals
    hashtable Vanish_Hash = InitHashtable() // Hashtable
    group Vanish_Group = CreateGroup() // Dmg Group
    integer Vanish_SShadowBallId = 'e001' // Small Shadowball
    integer Vanish_BShadowballId = 'e002' // Big Shadowball
    integer Vanish_AbilId = 'A000'
    real Vanish_SmallDmgAoE = 350
    string Vanish_SmallDmgEffect = "DarkNova.mdx"
    string Vanish_NebulaEffect = "Nebula.mdx"
    string Vanish_StarEffect = "StarExplosion.mdx"
    real Vanish_BigDmgAoE = 900
    real Vanish_PeriodicDmg = 10
    real Vanish_FinalDmg = 1000
    integer Vanish_SmallCountLimit = 25
    integer Vanish_BigCountLimit = 31
    real Vanish_Timer = 0.2
endglobals

// Creates the Special Effects
function Vanish_EffectLoop takes integer iterations, real initangle, real x, real y, boolean effectA returns nothing
    local integer i = 0
    local real ca
    local real sa
    local real a = x
    local real b = y
    
    loop
        exitwhen i == iterations
        set i = i + 1
        set ca = Cos(initangle)
        set sa = Sin(initangle)

        set a = x + 300 * ca
        set b = y + 300 * sa
        
        if effectA == true then
            call DestroyEffect(AddSpecialEffect(Vanish_SmallDmgEffect, x, y))
            call DestroyEffect(AddSpecialEffect(Vanish_NebulaEffect, x, y))
        else
            call DestroyEffect(AddSpecialEffect(Vanish_StarEffect, x, y))
        endif
        
        set a = x + 600 * ca
        set b = y + 600 * sa
        
        if effectA == true then
            call DestroyEffect(AddSpecialEffect(Vanish_SmallDmgEffect, x, y))
            call DestroyEffect(AddSpecialEffect(Vanish_NebulaEffect, x, y))
        else
            call DestroyEffect(AddSpecialEffect(Vanish_StarEffect, x, y))
        endif
        
        set a = x + 900 * ca
        set b = y + 900 * sa
        
        if effectA == true then
            call DestroyEffect(AddSpecialEffect(Vanish_SmallDmgEffect, x, y))
            call DestroyEffect(AddSpecialEffect(Vanish_NebulaEffect, x, y))
        else
            call DestroyEffect(AddSpecialEffect(Vanish_StarEffect, x, y))
        endif
        
        set initangle = initangle + initangle
    endloop
endfunction

// Deals the AoE Damage
function Vanish_Damage takes player owner, unit source, real x, real y, real aoe, real damage returns nothing
    local unit target    
    call GroupEnumUnitsInRange(Vanish_Group, x, y, aoe, null)
        loop
        set target = FirstOfGroup(Vanish_Group)
        exitwhen target == null
            if IsUnitEnemy(target, owner) then
                call UnitDamageTarget(source, target, damage, false, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL, null)
            endif
        call GroupRemoveUnit(Vanish_Group, target)
        endloop
endfunction

// Main Periodic Actions
function Vanishing_World_Periodic takes nothing returns nothing
    //Variable setup
    local timer t = GetExpiredTimer()
    local integer id = GetHandleId(t)
    local unit u = LoadUnitHandle(Vanish_Hash, id, 0)
    local real tx = LoadReal(Vanish_Hash, id, 1)
    local real ty = LoadReal(Vanish_Hash, id, 2)
    local real final_damage
    local real periodic_damage
    local integer count = LoadInteger(Vanish_Hash, id, 5)
    local real rand_distance
    local real rand_angle
    local real rx
    local real ry
    local unit shadowball
    local integer i
    local real angle
    local player owner = GetOwningPlayer(u)
    
    if count < Vanish_BigCountLimit then
        set count = count + 1
        call SaveInteger(Vanish_Hash, id, 5, count)
        if count < Vanish_SmallCountLimit then
            set rand_distance = GetRandomReal(200, 900)
            set rand_angle = GetRandomReal(0, 6.263)
            set rx = tx + rand_distance * Cos(rand_angle)
            set ry = ty + rand_distance * Sin(rand_angle)
            set periodic_damage = LoadReal(Vanish_Hash, id, 4)
            call KillUnit(CreateUnit(owner, Vanish_SShadowBallId, rx, ry, 270))
            call DestroyEffect(AddSpecialEffect(Vanish_SmallDmgEffect, rx, ry))
            call Vanish_Damage(owner, u, rx, ry, Vanish_SmallDmgAoE, periodic_damage)
            
        elseif count == Vanish_SmallCountLimit+1 then
            set shadowball = CreateUnit(owner, Vanish_BShadowballId, tx, ty, 270)
            call SaveUnitHandle(Vanish_Hash, id, 6, shadowball)
            set shadowball = null 
            
    //Waiting for 1 second to pass
        elseif count == Vanish_BigCountLimit then
            set final_damage = LoadReal(Vanish_Hash, id, 3)
            call KillUnit(LoadUnitHandle(Vanish_Hash, id, 6))
            call KillUnit(CreateUnit(owner, 'e003', tx, ty, 270))
            
            call Vanish_EffectLoop(20, 0.314, tx, ty, true)
            call Vanish_EffectLoop(20, 0.523, tx, ty, false)        
            call Vanish_Damage(owner, u, tx, ty, Vanish_BigDmgAoE, final_damage)
            
            call PauseTimer(t)
            call DestroyTimer(t)
            call FlushChildHashtable(Vanish_Hash, id)            
        endif
    endif
    
    set u = null
    set owner = null
    set t = null
endfunction

function Vanishing_World_Actions takes nothing returns nothing
    //Variable Setup
    local unit u = GetTriggerUnit()
    local timer t = CreateTimer()
    local integer id = GetHandleId(t)//Id of the timer, will be used as the parentKey
    local real tx = GetSpellTargetX()// X coordinate of the spell target location
    local real ty = GetSpellTargetY()// Y coordinate of the spell target location
    local real heroint = GetHeroInt(u,true)
    local real final_damage = Vanish_FinalDmg * heroint//Final damage at the end
    local real periodic_damage = Vanish_PeriodicDmg * heroint //damage during the periodic actions
    
    call SaveUnitHandle(Vanish_Hash, id, 0, u)
    call SaveReal(Vanish_Hash, id, 1, tx)
    call SaveReal(Vanish_Hash, id, 2, ty)
    call SaveReal(Vanish_Hash, id, 3, final_damage)
    call SaveReal(Vanish_Hash, id, 4, periodic_damage)
    call SaveInteger(Vanish_Hash, id, 5, 0)
    
    call TimerStart(t, Vanish_Timer, true, function Vanishing_World_Periodic)
    set t = null
    set u = null
endfunction

function Trig_Vanishing_World_v2_Conditions takes nothing returns boolean
    if GetSpellAbilityId() == Vanish_AbilId then
        call Vanishing_World_Actions()
    endif
    return false
endfunction

//===========================================================================
function InitTrig_Vanishing_World_v2_globals takes nothing returns nothing
    local trigger t = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( t, Condition( function Trig_Vanishing_World_v2_Conditions ) )
    set t = null
endfunction

 

[sniper_zero]

Oh, thanks. I didn't expect that.

set u = CreateUnit()
call KillUnit(u)
// Turned to
call KillUnit(CreateUnit())

But this is done intentionally on my part. It's probably because of the model but the unit is killed too fast with KillUnit(CreateUnit()).

With
set u = CreateUnit()
call KillUnit(u)

The unit stays a bit longer, making it more noticeable.

EDIT: So that's what my script would look like with proper documentation and globals. It looks so...organized.

EDIT: I don't see the conditions for effectA. What does it check?

Nevermind, it was just me being confused again.

 

[Spartipilo]

This still isn't "that" organized. A lot of comments and better variable names and stuff are required, but it's a good start. It was just an example so you can have some ideas about how to improve script read-ability for you and for the game (so both, you and the game can work with the script faster and better)

Are you sure about the instant unit kill? Scripts are ran instantly, i don't think there's any noticeable difference. Anyway, if you test and see different results, you can always check back.

Yeah, the main goal is the order. As said before, there's not much to think wit a single ability, but as you work with more, and more abilities, you'll eventually get lost in so many script lines ("Where the hell did I loaded the damage?", "Where is this value coming from?", etc.)

EffectA checks if it's the first special effect thing (Which does 2 special effect destroy) or the second one, which destroys just one. That function which repeats three times can also be declared as an independent function.

I didn't test it, but it should work. At least it allows saving with no jasshelper errors ^^

 

[sniper_zero]

Are you sure about the instant unit kill? Scripts are ran instantly, i don't think there's any noticeable difference. Anyway, if you test and see different results, you can always check back.

Yeah, I tested it while we were still fixing this spell. Doing KillUnit(CreateUnit()) is faster than set u = CreateUnit() and KillUnit(u) which is exactly why I did CreateUnit() and KillUnit() separately. There's a noticeable difference depending on the model being used.

And thanks again. I'll remember the effectsA function next time I do another sfx heavy spell.

 

[Spartipilo]

Huh, ok. Have a nice day.