[Solved] Trigger Malfunction

[Maestros]

So a couple of years back i was more noob than i am now and tried to make a spell! Was a total fail!
I decided to use my over the years experience to do that spell properly, MUI, leakless etc etc.
But here is the thing.

The spell is being cast alright, and the first time it is cast, it is executed perfectly! Now the second time it is cast, it is not! Nor any next time!
The thing that fails is the dummy units casting a spell on the triggering unit.

Code

• Deaths Healing Grasp
  • Events
    • Unit - A unit Starts the effect of an ability
  • Conditions
    • (Ability being cast) Equal to Deaths Healing Grasp
  • Actions
    • Set Index = (Index + 1)
    • Set TriggeringUnit[Index] = (Triggering unit)
    • For each (Integer A) from 1 to 12, do (Actions)
      • Loop - Actions
        • Set ofpoint[2] = (Position of TriggeringUnit[Index])
        • Set Real = (Real + 30.00)
        • Set ofpoint[3] = ((Position of TriggeringUnit[Index]) offset by 600.00 towards Real degrees)
        • Special Effect - Create a special effect at ofpoint[3] using Abilities\Spells\Undead\DeathCoil\DeathCoilSpecialArt.mdl
        • Special Effect - Destroy (Last created special effect)
        • Unit - Create 1 Death Knight Dummy Unit for Neutral Passive at ofpoint[3] facing (Angle from (Position of (Last created unit)) to ofpoint[2]) degrees
        • Unit Group - Add (Last created unit) to unitgroup[Index]
        • Unit - Add a 3.00 second Generic expiration timer to (Last created unit)
        • Animation - Change (Last created unit)'s vertex coloring to (100.00%, 100.00%, 100.00%) with 50.00% transparency
        • Unit - Make (Last created unit) face TriggeringUnit[Index] over 0.00 seconds
        • Custom script: call RemoveLocation (udg_ofpoint[2])
        • Custom script: call RemoveLocation (udg_ofpoint[3])
    • Wait 1.00 seconds
    • Set Index2 = (Index2 + 1)
    • Unit Group - Pick every unit in unitgroup[Index2] and do (Actions)
      • Loop - Actions
        • Unit - Order (Picked unit) to Undead Death Knight - Death Coil TriggeringUnit[Index2]
    • Custom script: call DestroyGroup(udg_unitgroup[udg_Index2])
    • Wait 2.00 seconds
    • Set Index3 = (Index3 + 1)
    • Set Real = 0.00
    • For each (Integer B) from 1 to 12, do (Actions)
      • Loop - Actions
        • Set Real = (Real + 30.00)
        • Set ofpoint[1] = ((Position of TriggeringUnit[Index3]) offset by 600.00 towards Real degrees)
        • Special Effect - Create a special effect at ofpoint[1] using Objects\Spawnmodels\Undead\UndeadDissipate\UndeadDissipate.mdl
        • Special Effect - Destroy (Last created special effect)
        • Custom script: call RemoveLocation (udg_ofpoint[1])

Action that Fails

• Unit Group - Pick every unit in unitgroup[Index2] and do (Actions)
  • Loop - Actions
    • Unit - Order (Picked unit) to Undead Death Knight - Death Coil TriggeringUnit[Index2]

Possible Problematic Lines

• Set Index = (Index + 1)
• Unit - Create 1 Death Knight Dummy Unit for Neutral Passive at ofpoint[3] facing (Angle from (Position of (Last created unit)) to ofpoint[2]) degrees
• Unit Group - Add (Last created unit) to unitgroup[Index]
• Wait 1.00 seconds
• Set Index2 = (Index2 + 1)
  • Unit Group - Pick every unit in unitgroup[Index2] and do (Actions)
    • Loop - Actions
      • Unit - Order (Picked unit) to Undead Death Knight - Death Coil TriggeringUnit[Index2]

PS: I have checked for mistakes or bugs in the indexing procces, the flaw isnt there.

 

[edo494]

well Im not best when it comes to GUI but I think you dont need index2 or index3 and Triggering Unit is MUI as well so no need to make variable TriggeringUnit
Maybe the problem is that index2 is not equal to index when you pick unit in the group?

 

[Maestros]

well Im not best when it comes to GUI but I think you dont need index2 or index3 and Triggering Unit is MUI as well so no need to make variable TriggeringUnit
Maybe the problem is that index2 is not equal to index when you pick unit in the group?

How can it not be equal?

Spell is cast, index = 1, spell is cast again, index = 2, wait 1 second, index2 = 1, bla bla wait 1 second, index2 = 2, plus as i said, i checked the indexing performance, the issue isnt there!
Plus if the indexing wasnt correct then, index3, which i use for point array, wouldnt work in the end as well but it does!
I also use them for point arrays so i need them anyways.
I have spent a lot of time thinking and recoding this so i believe the problem must be something i am not aware of...

 

[claptomanic]

the problem is that you destroy the unitgroup

• Unit Group - Pick every unit in unitgroup[Index2] and do (Actions)
• Loop - Actions
• Unit - Order (Picked unit) to Undead Death Knight - Death Coil TriggeringUnit[Index2]
• Custom script: call DestroyGroup(udg_unitgroup[udg_Index2])

so you cant add a unit to it anymore (the group isnt existing anymore). remove every unit from the group instead and it should work.


to make it easier....if you create the group with

• set UnitGroupXY = blablabla

inside the trigger the group is created over and over again and it doesnt matter if you destroy it...

but you just add units to it and destroy it so the group isnt existing anymore after the first run of the trigger and you cant use it again.

hope i was able to help you

 

[edo494]

claptomanic you have eagle-eye but for me GUI is little bit hard to read really
but in more dept thinking, he will destroy group[1] but on second cast he uses group[2] which is not destroyed hmm...am I wrong?(most likely )

 

[claptomanic]

initially just the unit group with the index 1 is created...every other unit group does not exist until its created so ^^

oh btw....you leak a position :

• Unit - Create 1 Death Knight Dummy Unit for Neutral Passive at ofpoint[3] facing (Angle from (Position of (Last created unit)) to ofpoint[2]) degrees

beside that you dont need to set triggering unit to a variable. also integer b can be replaced with integer a because the loop is seperate.

claptomanic you have eagle-eye

thanks i try my best ^^

 

[Maestros]

the problem is that you destroy the unitgroup

• Unit Group - Pick every unit in unitgroup[Index2] and do (Actions)
• Loop - Actions
• Unit - Order (Picked unit) to Undead Death Knight - Death Coil TriggeringUnit[Index2]
• Custom script: call DestroyGroup(udg_unitgroup[udg_Index2])

so you cant add a unit to it anymore (the group isnt existing anymore). remove every unit from the group instead and it should work.


to make it easier....if you create the group with

• set UnitGroupXY = blablabla

inside the trigger the group is created over and over again and it doesnt matter if you destroy it...

but you just add units to it and destroy it so the group isnt existing anymore after the first run of the trigger and you cant use it again.

hope i was able to help you

TBH i thought that this might be the issue, but i did NOT know that when i destroy the group the "variable" is also destroyed, or i mean the value or the variable.. well you get what i mean, i didnt know it was totally destroyed.
Thanks, +rep for teaching me


EDIT: but if i just empty the unit group, wont it be leaking? i mean then whats the whole point of destroying the unit groups :? I rejected the thought of the unit group destruction being the thing causing the malfunction cause i know that they leak and i have to destroy them



EDIT 2:

initially just the unit group with the index 1 is created...every other unit group does not exist until its created so ^^

oh btw....you leak a position :

• Unit - Create 1 Death Knight Dummy Unit for Neutral Passive at ofpoint[3] facing (Angle from (Position of (Last created unit)) to ofpoint[2]) degrees

beside that you dont need to set triggering unit to a variable. also integer b can be replaced with integer a because the loop is seperate.

• Unit - Create 1 Death Knight Dummy Unit for Neutral Passive at ofpoint[3] facing (Angle from (Position of (Last created unit)) to ofpoint[2]) degrees
• Custom script: call RemoveLocation (udg_ofpoint[2])

hmm?


EDIT 3:

Triggering Unit is MUI as well so no need to make variable TriggeringUnit

Does it not perform better this way? I mean, setting it to (Trigger Unit) isnt it an extra function in the JASS Script?
Seting it into a variable and then using that variable doesnt that make it perform better?

 

[claptomanic]

EDIT: but if i just empty the unit group, wont it be leaking? i mean then whats the whole point of destroying the unit groups :? I rejected the thought of the unit group destruction being the thing causing the malfunction cause i know that they leak and i have to destroy them

youre half correct ^^

the fact is unit groups are leaking....yes. but if you reuse the unit group over and over again its more efficient to reuse one group instead of recreating it over an over again.

the leaking would cause disadvanteges if you were creating the unit group at the beginning of the trigger but never destroy it. so everytime the spell is cast a new unit group overrides the last created and so the last created would take memory space and thats a leak. if youre creating a unit group in the trigger you have to destroy it otherwise it would leak....if you just add units to a group and reuse it just remove every unit from the unit group and reuse it and it wont leak.
i hope you get what i mean.

• Unit - Create 1 Death Knight Dummy Unit for Neutral Passive at ofpoint[3] facing (Angle from (Position of (Last created unit)) to ofpoint[2]) degrees
• Custom script: call RemoveLocation (udg_ofpoint[2])

hmm?

no the leak is '(Position of (Last created unit))' ^^

Does it not perform better this way? I mean, setting it to (Trigger Unit) isnt it an extra function in the JASS Script?
Seting it into a variable and then using that variable doesnt that make it perform better?

afaik thats not needed but i could be wrong....i never use a variable for triggering unit and i never got a problem with the perfomance. ;D

 

[Maestros]

youre half correct ^^

the fact is unit groups are leaking....yes. but if you reuse the unit group over and over again its more efficient to reuse one group instead of recreating it over an over again.

the leaking would cause disadvanteges if you were creating the unit group at the beginning of the trigger but never destroy it. so everytime the spell is cast a new unit group overrides the last created and so the last created would take memory space and thats a leak. if youre creating a unit group in the trigger you have to destroy it otherwise it would leak....if you just add units to a group and reuse it just remove every unit from the unit group and reuse it and it wont leak.
i hope you get what i mean.



So reusing the same group over and over, won't cause a leak, it will just take some memory space for the group, which is minimum.
Got it, thanks a lot, i will just change the destroying function with an emptying one!


no the leak is '(Position of (Last created unit))' ^^


Oooooh...
Jeez, i remade the spell, in the old way i had an extra point variable which i deleted for some uknown reason... i believe i know now what was its purpose xD


afaik thats not needed but i could be wrong....i never use a variable for triggering unit and i never got a problem with the perfomance. ;D

I am not sure also, but something tells me that, in theory at least, geting the (Trigger Unit) requires a function which you can overide with the variable setting.. i am expeting a reply from edo who is a JASS coder to be sure

read above!

 

[edo494]

setting the triggering unit to variable is usefull but with waits I wont do it most likely(locals FTW! )

also if you destroy the unit group you must do
set unitgroup[index] = blablabla otherwise it wont be created again and you will have a global pointer pointing to a non existing stuff, its like if you have unit, you have a variable and you remove the unit the variable still points to the unit even trough the unit is already dead and so doing like Make variable(removed unit) facing centre of playable map over 0.00 seconds will order null unit to face location
by the way, you use unit group but it looks like the group contains only 1 unit so you could just use some unit array for that instead of group

 

[Maestros]

setting the triggering unit to variable is usefull but with waits I wont do it most likely(locals FTW! )

also if you destroy the unit group you must do
set unitgroup[index] = blablabla otherwise it wont be created again and you will have a global pointer pointing to a non existing stuff, its like if you have unit, you have a variable and you remove the unit the variable still points to the unit even trough the unit is already dead and so doing like Make variable(removed unit) facing centre of playable map over 0.00 seconds will order null unit to face location
by the way, you use unit group but it looks like the group contains only 1 unit so you could just use some unit array for that instead of group

The unit group contains 12 units per casting
Yes but, i remember hearing from somewhere that whatever is inside parenthesys in GUI, its a GetSomething function in JASS, is that correct?
And if so, if you use the (Triggering Unit) won't it be calling the same function over and over again? which would be resourcefull?

 

[claptomanic]

locals okay...i usually use them....but why you make a difference between triggers with wait and without wait to set the variable for triggering unit and not setting it Oo

 

[edo494]

I wont use non array global variable for Triggering Unit because it will be overwrite when you cast it while one instance is stopped with wait.
Using global array variable just for Triggering Unit is total waste, you waste literally 8000 unit pointers for sake of having array.
Yes everything in GUI which is in between () is a function call but its not that horibble imo
GUI itself is that slow compared to Jass that some more function calls(Triggering Unit is native so its extremly fast) is not a big deal

 

[Maestros]

I wont use non array global variable for Triggering Unit because it will be overwrite when you cast it while one instance is stopped with wait.
Using global array variable just for Triggering Unit is total waste, you waste literally 8000 unit pointers for sake of having array.
Yes everything in GUI which is in between () is a function call but its not that horibble imo
GUI itself is that slow compared to Jass that some more function calls(Triggering Unit is native so its extremly fast) is not a big deal

+ rep as well

thanks guys, so ill just empty the group instead of destroying it, and use Triggering Unit instead of setting a global on it. Thank you both



~~~~~~~~~~~~~~~~~~~~I hereby declare this thread SOLVED~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ( lol )

 

[Magtheridon96]

I came here to help, and I found that the problem is solved.
I guess I'll help by marking the thread instead :3