Simple Trigger/Leak Question

[MorbazanRhot]

I have a pretty simple question regarding where to remove a variable within multiple instances of it in a single trigger.

Example:

Defenses
    Events
        Time - Elapsed game time is 0.50 seconds
    Conditions
    Actions
        Set unitgroup = (Units in Pink Army 1 <gen>)
        Unit Group - Pick every unit in unitgroup and do (Actions)
            Loop - Actions
                Unit - Order (Picked unit) to Hold Position
        Set unitgroup = (Units in Pink Army 2 <gen>)
        Unit Group - Pick every unit in unitgroup and do (Actions)
            Loop - Actions
                Unit - Order (Picked unit) to Hold Position
        Set unitgroup = (Units in Pink Army 3 <gen>)
        Unit Group - Pick every unit in unitgroup and do (Actions)
            Loop - Actions
                Unit - Order (Picked unit) to Hold Position
        Set unitgroup = (Units in Pink Army 4 <gen>)
        Unit Group - Pick every unit in unitgroup and do (Actions)
            Loop - Actions
                Unit - Order (Picked unit) to Hold Position
        Custom script: call DestroyGroup (udg_unitgroup)

Is that fine? Or would it be better like this:

Defenses
    Events
        Time - Elapsed game time is 0.50 seconds
    Conditions
    Actions
        Set unitgroup = (Units in Pink Army 1 <gen>)
        Unit Group - Pick every unit in unitgroup and do (Actions)
            Loop - Actions
                Unit - Order (Picked unit) to Hold Position
        Custom script: call DestroyGroup (udg_unitgroup)
        Set unitgroup = (Units in Pink Army 2 <gen>)
        Unit Group - Pick every unit in unitgroup and do (Actions)
            Loop - Actions
                Unit - Order (Picked unit) to Hold Position
        Custom script: call DestroyGroup (udg_unitgroup)
        Set unitgroup = (Units in Pink Army 3 <gen>)
        Unit Group - Pick every unit in unitgroup and do (Actions)
            Loop - Actions
                Unit - Order (Picked unit) to Hold Position
        Custom script: call DestroyGroup (udg_unitgroup)
        Set unitgroup = (Units in Pink Army 4 <gen>)
        Unit Group - Pick every unit in unitgroup and do (Actions)
            Loop - Actions
                Unit - Order (Picked unit) to Hold Position
        Custom script: call DestroyGroup (udg_unitgroup)

I'd just like to know which of these would be more efficient? I assumed the first just because it has less lines, but would it be worse for some reason because it's not removed before being assigned a new value? Or maybe neither of these are any good? And would the same go for points, like setting TempPoint1=center of rect, then spawning units there, setting TempPoint1=center of a different rect, then spawning units there, etc. and removing TempPoint1 at the end? Thanks, and +rep for help.

edit: And yes, I know that if there's a wait it needs to be removed before the wait. I'm asking for situations where there are no waits in between.

 

[GooseBoy]

imo, the first one should work just fine.

It wont leak as long as you are using it. I guess.

 

[Emu.Man00]

custom script : local unit group tempGroup
unit group- add pink army 1 to tempGroup
unit group- add pink army 2 to tempGroup
unit group- add pink army 3 to tempGroup
unit group- add pink army 4 to tempGroup
unit group - pick every unit in tempgroup and do (actions)
   loop actions
      unit - order (picked unit) to hold position
custom script: call DestroyGroup (tempGroup)

I think that would work best

 

[bOb666777]

custom script : local unit group tempGroup
unit group- add pink army 1 to tempGroup
unit group- add pink army 2 to tempGroup
unit group- add pink army 3 to tempGroup
unit group- add pink army 4 to tempGroup
unit group - pick every unit in tempgroup and do (actions)
   loop actions
      unit - order (picked unit) to hold position
custom script: call DestroyGroup (tempGroup)

I think that would work best

Yeah, just one thing: udg_tempGroup*

 

[Emu.Man00]

The variable was local. 'udg_' is only for globals.... I think :3

 

[TriggerHappy]

The variable was local. 'udg_' is only for globals.... I think :3

Yes, udg_ is only used for GUI generated globals, though you can define locals with that prefix also.

 

[MorbazanRhot]

The thing is, Pink Army x is a rect, and that I know of, there's not any way to add units to a unit group from a rect without making them into a unit group in the first place, which is what I'm already doing here.

I could add unitgroup to tempgroup after each rect group assignment, but I'd still need to remove unitgroup and tempgroup, which doesn't answer my initial question and doesn't really make the trigger any more efficient either.

 

[CaptDeath]

the latter is better
@emu you cant use locals in a GUI trigger so the loop would have to be a custom script as well

 

[Emu.Man00]

Oh. Well you can still just make it a global, I figured mine was a lot more clean of a code :O

 

[MorbazanRhot]

Thanks for the help guys, +rep to all.

 

[AceHart]

Leaks a unit group:
- Set unitgroup = (Units in Pink Army 1 <gen>)
- Set unitgroup = (Units in Pink Army 2 <gen>)
- Custom script: call DestroyGroup (udg_unitgroup)

No problem here:
- Set unitgroup = (Units in Pink Army 1 <gen>)
- Custom script: call DestroyGroup (udg_unitgroup)
- Set unitgroup = (Units in Pink Army 2 <gen>)
- Custom script: call DestroyGroup (udg_unitgroup)