[Trigger] Struggling with a "pick every unit and do" loop nested in a "For each integer i" loop

[sadathy]

Hey guys, I think this is my first post though I have been lurking the forums for a far while picking up tips and stuff

I am completely lost when it comes to this trigger I've tried to write

Intended Result: Every 0.02 seconds of game time, if unit is in check group, they are moved in a predefined direction and damage all nearby enemy units that have not already been damaged by this instance of the spell

Actual Result: Every 0.02 seconds of game time, the unit moves forward and damages all nearby enemy units, regardless of if they have been damaged before

Here's my trigger, or at least the section that handles movement and damage, I initialize? (not sure if that's the right term in this system) all my variables in a cast function that then switches this function on...

• SavageryMover
  • Events
    • Time - Every 0.02 seconds of game time
  • Conditions
  • Actions
    • Do Multiple ActionsFor each (Integer myLoop) from 1 to 8192, do (Actions)
      • Loop - Actions
        • Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • (savageryCaster[myLoop] is in savageryChannelingIyaris) Equal to (==) True
          • Then - Actions
            • Set tempPoint = (Position of savageryCaster[myLoop])
            • Set newPoint = (tempPoint offset by 8.00 towards savageryAngle[myLoop] degrees)
            • Set tempReal = (X of newPoint)
            • Custom script: call SetUnitX(udg_savageryCaster[udg_myLoop], udg_tempReal)
            • Set tempReal = (Y of newPoint)
            • Custom script: call SetUnitY(udg_savageryCaster[udg_myLoop], udg_tempReal)
            • Set tempGroup = (Units within 250.00 of newPoint)
            • Unit Group - Pick every unit in tempGroup and do (Actions)
              • Loop - Actions
                • Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
                  • If - Conditions
                    • ((Picked unit) is A structure) Not equal to (!=) True
                    • ((Picked unit) is alive) Equal to (==) True
                    • ((Owner of savageryCaster[myLoop]) is an enemy of (Owner of (Picked unit))) Equal to (==) True
                  • Then - Actions
                    • Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
                      • If - Conditions
                        • ((Picked unit) is in savageryAlreadyHit[myLoop]) Equal to (==) False
                      • Then - Actions
                        • Unit - Cause savageryCaster[myLoop] to damage (Picked unit), dealing ((25.00 + (50.00 x (Real((Level of Savagery for savageryCaster[myLoop]))))) + (Real((Intelligence of savageryCaster[myLoop] (Include bonuses))))) damage of attack type Hero and damage type Normal
                        • Unit Group - Add (Picked unit) to savageryAlreadyHit[myLoop]
                      • Else - Actions
                  • Else - Actions
              • Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
                • If - Conditions
                  • Multiple ConditionsOr - Any (Conditions) are true
                    • Conditions
                      • (Distance between newPoint and savageryTarget[myLoop]) Less than (<) 8.00
                      • savageryTimeout[myLoop] Less than (<) 0.00
                • Then - Actions
                  • Unit Group - Remove savageryCaster[myLoop] from savageryChannelingIyaris
                  • Unit - Remove Spell Immunity (Archimonde) from savageryCaster[myLoop]
                  • Animation - Change savageryCaster[myLoop]'s animation speed to 100.00% of its original speed
                • Else - Actions
          • Else - Actions
    • Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Number of units in savageryChannelingIyaris) Equal to (==) 0
      • Then - Actions
        • Trigger - Turn off (This trigger)
      • Else - Actions

Just wondering if anyone can spot any issues I've missed, from what testing I've done it would suggest that the function that adds a unit to a unit group simply does not work, but I know that's not the case since I have the function working in other triggers... perhaps I am approaching this the wrong way - it would be a lot simpler if I wasn't trying to make the spell MUI

Thanks in advance

~Sadathy

EDIT: Forgot to mention I am completely aware that this leaks, I prefer to put in the stuff that cleans up leaks after I have a spell working completely - also my variable names might not be the best in certain cases

 

[KILLCIDE]

Is this UMSWE?

 

[sadathy]

Is this UMSWE?

It is indeed

 

[KILLCIDE]

It's sort of hard to read... but I have an idea of why units are still being damaged. WC is really weird when it comes to unit group arrays, can I see how you create this unit group in your cast trigger?

 

[sadathy]

It's sort of hard to read... but I have an idea of why units are still being damaged. WC is really weird when it comes to unit group arrays, can I see how you create this unit group in your cast trigger?

Sure, here you go:

• SavageryCast
  • Events
    • Unit - A unit Starts the effect of an ability
  • Conditions
    • (Ability being cast) Equal to (==) Savagery
  • Actions
    • Unit Group - Add (Triggering unit) to savageryChannelingIyaris
    • Unit Group - Remove bloodBondLinkNOTIYARI[(Custom value of (Triggering unit))] from AngryIyaris
    • Unit Group - Remove all units from savageryAlreadyHit[(Custom value of (Triggering unit))]
    • Set savageryAngle[(Custom value of (Triggering unit))] = (Facing of (Triggering unit))
    • Set savageryCaster[(Custom value of (Triggering unit))] = (Triggering unit)
    • Set tempPoint = (Position of (Triggering unit))
    • Set savageryTarget[(Custom value of (Triggering unit))] = (tempPoint offset by 1200.00 towards savageryAngle[(Custom value of (Triggering unit))] degrees)
    • Set savageryTimeout[(Custom value of (Triggering unit))] = 4.00
    • Unit - Add Spell Immunity (Archimonde) to (Triggering unit)
    • Animation - Change (Triggering unit)'s animation speed to 400.00% of its original speed
    • Unit - Order (Triggering unit) to Special - Channel
    • Trigger - Turn on SavageryMover <gen>
    • Custom script: call RemoveLocation(udg_tempPoint)

I'm guessing the GUI needs to declare certain things in the correct order, if this is what you wanted to see. Is that related to the way the scripts hidden underneath work?

By the way Ignore the bloodBondLink group, that is related to another spell which I turn off whilst this one is running

EDIT: Also, would you suggest I comment up my triggers before I post them on the hive to make readability less of an issue? I'm kind of embarrassed it's not commented since when I work actual programming languages I comment really well I just haven't picked up the habit in wc3

 

[KILLCIDE]

Comments do help, but I was just saying its hard to read because I've never used UMSWE before. You sort have the idea right for making it MUI through dynamic indexing, but you need a Max Index so that you have some type of reference for the loop integer. For your unit group issue, I suggest making the unit groups like this when they are arrays:

Custom script: if udg_savageryAlreadyhit[array] == null then Custom script: set udg_savageryAlreadyhit[array] = CreateGroup() Custom script: endif

Then when you deindex, destroy the group, switch the iteration, then NULL the instance.

Custom script: call DestroyGroup (udg_savageryAlreadyhit[myLoop]) Set savageryAlreadyhit[myLoop] = savageryAlreadyhit[array] Custom script: set udg_savageryAlreadyhit[array] = null

 

[sadathy]

Comments do help, but I was just saying its hard to read because I've never used UMSWE before. You sort have the idea right for making it MUI through dynamic indexing, but you need a Max Index so that you have some type of reference for the loop integer. For your unit group issue, I suggest making the unit groups like this when they are arrays:

Custom script: if udg_savageryAlreadyhit[array] == null then Custom script: set udg_savageryAlreadyhit[array] = CreateGroup() Custom script: endif

Then when you deindex, destroy the group, switch the iteration, then NULL the instance.

Custom script: call DestroyGroup (udg_savageryAlreadyhit[myLoop]) Set savageryAlreadyhit[myLoop] = savageryAlreadyhit[array] Custom script: set udg_savageryAlreadyhit[array] = null

Right, I am using Bribe's GUI indexing system for this stuff, so the max index should always be 8192 and I am effectively just "tying" each variable to a unique caster

If I am understanding right, are you suggesting that I call this during the cast trigger:

Custom script: if udg_savageryAlreadyhit[array] == null then Custom script: set udg_savageryAlreadyhit[array] = CreateGroup() Custom script: endif

If so I'd just replace 'array' with the custom value of the triggering unit (Not sure how I'd call this in script but I could just set a temporary integer equal to it then pull that integer in the custom script)

Custom script: call DestroyGroup (udg_savageryAlreadyhit[myLoop]) Set savageryAlreadyhit[myLoop] = savageryAlreadyhit[array] Custom script: set udg_savageryAlreadyhit[array] = null

Then I call this when my trigger detects that either the unit has reached their intended location or the spell has "timed out"


I'll try and implement this then get back

POST TESTING:
Okay so I tried to add the code you gave me to the triggers and it doesn't seem to have made a difference. Part of the problem is that I perhaps don't fully understand what it is that the functions do so I may have put them in the wrong place.

Would you mind trying to explain to me what it is I'm doing when I call these things, my understanding is as such

• Set tempInt = (Custom value of (Triggering unit))
• Custom script: if udg_savageryAlreadyHit[udg_myLoop] == null then
• Custom script: set udg_savageryAlreadyHit[udg_myLoop] = CreateGroup()
• Custom script: endif

I believe that what I'm doing here is setting a temporary integer to be equal to the unit's custom value [unique index], I then use this to look up the unique index in the unit group array and in the case that it does not point to a handle, I create one for it to point to?

• Custom script: call DestroyGroup(udg_savageryAlreadyHit[udg_myLoop])
• Set savageryAlreadyHit[myLoop] = (udg_savageryAlreadyHit[udg_myLoop])
• Custom script: set udg_savageryAlreadyHit[udg_myLoop] = null

I believe that I am destroying the handle that is pointed at by this particular index of the Unit Group array, I then do "something" no idea what, as far as I can tell this does literally nothing, perhaps it is redundant as I have used it wrong.
I also don't understand why, after destroying the handle, I use this = null statement, I can only assume this relates to the if statement in the other trigger working properly? but how I have no idea

Either way I really appreciate you trying to help me so far, I hope me explaining how I am looking at this code helps you see why I am doing it wrong

EDIT: Hold up I just looked over this and realized I referred to "udg_myLoop" in the cast trigger, I'll amend this and see if it was the issue

EDIT2: Okay, thank you very much KILLCIDE, seriously! After fixing up that variable reference I got the triggers working perfectly. I can only assume that when wc3 initializes the global variables made in the variable editor, it doesn't populate the unit group arrays properly, perhaps to avoid creating a bunch of handles for no reason which makes a lot of sense now I think about it

I hope my musings made sense, I'll leave this all here so I can come back to it later if that's all fine and dandy

 

[KILLCIDE]

Hey Sadathy, sorry for not responding I was out running errands. However, I'm glad to see that you managed to figure it all out! I apologize for the not-so descriptive solution, but yes, I meant to put that custom script block in the cast trigger thank you for keeping the post up and not editing out by the way, it means a lot to the community! Keep it up!

All though using Bribe's Indexer to make things MUI is an okay solution, I wouldn't recommend it. If that same unit were to somehow cast the same spell again, it would bug out if its previous cast hasn't finished since it's sharing the same ID.

hey guys i need help with a trigger.
I want to spawn a unit as soon as a building finishes construction
I want that created to be assign to the player that built the building, then i want it to be uncontrollable (made unit a ward)
Finally i want that created unit to die when the building that spawned it is destroyed.

How do i do this?

As sadathy said, make your own thread about it. Do not comment on other threads and go off-topic.

 

[sadathy]

Fortunately thanks to the nature of the map this isn't possible

In the few cases where a spell has a shorter cooldown than it's effect duration I have "Sub" indexed it by checking for a specific buff, if they have the buff then I have a little trigger that correctly stacks the effects, if they don't then it just runs like normal

And yeah no worries about the non response, you helped out a lot anyway - I was up at 5am trying to fix this (I live in the UK) so yeah I kind of fell asleep shortly after fixing it anyway ^^


This thread is completely solved

 

[Bribe]

In case it hadn't been mentioned yet, never loop from 1 to 8191. In most cases you'll never have more than a few hundred units - in extreme cases maybe 1-2000...

Instead of the loop, use Pick units in group straight off the bat. This way, you don't iterate over instances which don't exist in the unit group and force everything to be relevant.

 

[sadathy]

In case it hadn't been mentioned yet, never loop from 1 to 8191. In most cases you'll never have more than a few hundred units - in extreme cases maybe 1-2000...

Instead of the loop, use Pick units in group straight off the bat. This way, you don't iterate over instances which don't exist in the unit group and force everything to be relevant.

Okay I will try that

The reason I used a for loop here is because I am nesting a unit group inside of it, I just assumed the way you refer to "(Picked Unit)" would be kind of buggy if I had two "(Picked Unit)s" to refer to, since it seems that it's a variable with global scope, it doesn't refer to some kind of local created for the loop/trigger

Is it possible to nest pick every unit loops then? Or would I have to add them to a group then let another trigger handle the damage/pushback seperately

 

[Bribe]

Just use a different set of variables in the nested group pick, so they don't overwrite the containing one.

 

[sadathy]

Just use a different set of variables in the nested group pick, so they don't overwrite the containing one.

So to clarify, I can use (Picked Unit) in both loops, and it will refer to the unit in that particular loop?

 

[Bribe]

Yes.