That is annoying, but seeing the full structure with words I have to translate (I studied french four years in high school) is much more valuable than pseudo-triggers.
Making that easy is the whole point of such an indexer. It gives each unit a unique number for its CV and then you can use that as the index of an array variable to store data specific to that unit in that array slot. You will have to change the one thing already using custom value to instead set/read/use SomeNewIntegerVariable[(Custom value of <the unit>)] rather than using CV directly, but that is a simple change.
Instead of repeating huge sections of your triggers with different If/Then/Else blocks, you can use a small few if-blocks to set some variables and then use those variables to determine how to proceed. This avoids code duplication which can be annoying to change or debug later. An example to show how you might increase the duration or area with spell level:
-
Set lvl = (Level of (Ability being cast) for (Triggering Unit))
-
Set SpellArea = (100.00 + (100.00 x (Real(lvl))))
-
If (All conditions are true) then do (Then actions) else do (Else actions)
-
If - Conditions
-
Then - Actions
-
Else - Actions
-
-------- Note that these don't have to be nested --------
-
If (All conditions are true) then do (Then actions) else do (Else actions)
-
If - Conditions
-
Then - Actions
-
Else - Actions
-
If (All conditions are true) then do (Then actions) else do (Else actions)
-
If - Conditions
-
lvl greater than or equal to 3
-
Then - Actions
-
Else - Actions
-
Unit Group - Pick every unit within SpellArea of...
-
Loop - Actions
-
-------- etc. --------
-
For each (Integer A) from 1 to SpellDurationInt do (Actions)
Don't do what you're doing with regions to find units. Regions are rectangles rather than circles and you are dynamically creating new regions with each search (which would need to be destroyed and cleaned up before they become memory leaks). Instead you should use
Units In Range of Point.
You cannot put a
Wait inside of a
Unit Group - Pick... loop or a
Player Group - Pick... loop. When such functions are used it starts an individual process thread for each unit, and when one such thread encounters a
Wait it will simply crash and nothing after that (in the
Loop) will execute. Put messages after your waits and you will see they never happen. The trigger execution before/after the
Pick is not affected because only the sub-threads crash not the main one. This is likely why the units never seem to return to their original owner.
In order to periodically order the units to attack something nearby every 1 second you will need to store the units in some group variable and then save a reference to that group variable somewhere that you can access it periodically. This could be done with a periodic trigger and some
dynamic indexing (though it should probably have a frequency higher than 1/s), by using a timer array and attaching information about the group to that specific timer instance, or by some other method to store data I haven't considered. Those are ultimately not difficult methods, but if you are unfamiliar with the process they may seem daunting.
Something nearly as effective and much simpler is to abuse JASS's ability to shadow GUI global variables locally to make them act as though they are local to the function.
For something like this to work you need to consider/understand when and how GUI creates and uses a sub-function, because inside of those sub functions the local variable won't exist! Luckily here you won't need to refer to the integer counters or unit group we'll shadow, and GUI doesn't use sub-functions for integer loops anyway:
-
Actions
-
Custom script: local integer udg_CounterVariable //these MUST go first in the actions
-
Custom script: local integer udg_SpellDurationInt //match the name to the name of your variable but keep the udg_ prefx
-
Custom script: local group udg_GroupVariable
-
-------- do your other stuff --------
-
Set GroupVariable = (Units within ...)
-
Set CounterVariable = 0
-
Set SpellDurationInt = <whatever>
-
For each Integer CounterVariable from 1 to SpellDurationInt do (Actions)
-
Loop - Actions
-
Unit Group - Pick every unit in GroupVariable and do (Actions)
-
Loop - Actions
-
-------- stuff --------
-
-------- since this is a new thread for each unit, the local GroupVariable would NOT 'exist' here --------
-
Wait 1.00 seconds
-
-------- at end of trigger --------
-
Custom script: call DestroyGroup(udg_GroupVariable)
-
Custom script: set udg_GroupVariable = null //integers don't need to be nulled and destroyed, only the group because it is a handle
Note that the wait in the loop is okay here because while the wait is happening there's no way to overwrite the relevant variables (they're local!). This means any number of instances of this spell can be active at once without breaking the looping process for any of them (and they can loop any number of times each, too). It's still not good practice but it can be acceptably done with care.
This, however, will not work for multiple overlapping casts. In such a case a unit affected by a second cast would prematurely return to its original owner before the second cast finished (since the end of the first cast would return it). The simplest solution I see is to attach two bits of data to each unit when this spell is cast on them: their owning player and the number of instances of this spell they are currently affected by. But you'll only want to store the owning player if an owning player
isn't already stored for this unit, and you'll only change the ownership back if the instance count is 0 (this is the last one to finish). This information will also have to be nulled when the final instance affecting the unit ends:
-
Set CV = (Custom value of (Picked unit))
-
-------- when the unit is changed to the temporary owner --------
-
Set InstanceCount[CV] = (InstanceCount[CV] + 1)
-
If (All conditions are true) then do (Then actions) else do (Else actions)
-
If - Conditions
-
OriginalOwner[CV] equal to No Player
-
Then - Actions
-
Set OriginalOwner[CV] = (Owner of (Picked Unit))
-
Else - Actions
-
-------- when the instance ends --------
-
Set InstanceCount[CV] = (InstanceCount[CV] - 1)
-
If (All conditions are true) then do (Then actions) else do (Else actions)
-
If - Conditions
-
InstanceCount[CV] less than or equal to 0
-
Then - Actions
-
Unit - Change owner of (Picked unit) to OriginalOwner[CV] ...
-
Set OriginalOwner[CV] = No Player
-
Else - Actions
Ask any further questions or for clarification if these methods don't make sense.
Events : a unit starts the effect of an ability
