Does this ability trigger leak/lag?

[Valtarian]

Top side
    Events
        Unit - A unit owned by Player 8 (Pink) Is attacked
        Unit - A unit owned by Player 12 (Brown) Is attacked
        Unit - A unit owned by Player 10 (Light Blue) Is attacked
    Conditions
    Actions
        Unit Group - Pick every unit in (Units in Snow <gen>) and do (Actions)
            Loop - Actions
                If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                    If - Conditions
                        ((Unit-type of (Picked unit)) Equal to Stalker (|cff00a800Unlimited|r) lvl 6) or ((Unit-type of (Picked unit)) Equal to Stalker lvl 6)
                    Then - Actions
                        If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                            If - Conditions
                                ((Picked unit) is in AntiPatrolGroup3) Equal to True
                            Then - Actions
                                Unit - Order (Picked unit) to (Name of Curse (Creeps)(Stalker)) (Attacking unit)
                                Set TempPoint = (Center of Top Left <gen>)
                                Unit - Order (Picked unit) to Move To TempPoint
                                Point - Remove TempPoint
                                Trigger - Turn off (This trigger)
                            Else - Actions
                                If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                                    If - Conditions
                                        ((Picked unit) is in AntiPatrolGroup4) Equal to True
                                    Then - Actions
                                        Unit - Order (Picked unit) to (Name of Curse (Creeps)(Stalker)) (Attacking unit)
                                        Set TempPoint = (Center of Top Right <gen>)
                                        Unit - Order (Picked unit) to Move To TempPoint
                                        Point - Remove TempPoint
                                        Trigger - Turn off (This trigger)
                                    Else - Actions
                                        If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                                            If - Conditions
                                                (((Picked unit) is in UnitsBottomLeft) Equal to True) or (((Picked unit) is in UnitsBottomRight) Equal to True)
                                            Then - Actions
                                                Unit - Order (Picked unit) to (Name of Curse (Creeps)(Stalker)) (Attacking unit)
                                                Set TempPoint = (Center of Top Center <gen>)
                                                Unit - Order (Picked unit) to Move To TempPoint
                                                Point - Remove TempPoint
                                                Trigger - Turn off (This trigger)
                                            Else - Actions
                                                If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                                                    If - Conditions
                                                        ((Picked unit) is in TopToBoat) Equal to True
                                                    Then - Actions
                                                        Unit - Order (Picked unit) to (Name of Curse (Creeps)(Stalker)) (Attacking unit)
                                                        Set TempPoint = (Center of Load Top <gen>)
                                                        Unit - Order (Picked unit) to Move To TempPoint
                                                        Point - Remove TempPoint
                                                        Trigger - Turn off (This trigger)
                                                    Else - Actions
                    Else - Actions

The trigger will be approximately 20x longer, that's just for one unit. I have about 30-40 more units to apply this to. The trigger will be turned on AGAIN by this trigger:

Creep Spells
    Events
        Time - Every 4.75 seconds of game time
    Conditions
    Actions
        Trigger - Turn on Top side <gen>

I've tried using a timer instead of a periodic event.. but I couldn't get it going.


Does this leak at all? Also, will this cause any lag? THANK YOU!!

 

[Solu9]

Next time will you please use the WC3 /WC3 tags?

Action:
Unit Group - Pick every unit.....

That unit group leaks. Set the group in a variable instead and destroy it in the end of the trigger.

And that: Point - Remove TempPoint
Im not sure if thats a custom script or not, but you need to use:
Custom script: call RemoveLocation (udg_TempPoint)

 

[Imp Midna]

Asuming the Point - Remove TempPoint is a GUI implimentation from RemoveLocation so i guess that one is fine. Apart from that, the only leaking thing is the unit group.

The trigger will be approximately 20x longer, that's just for one unit. I have about 30-40 more units to apply this to.

The whole trigger is quite inefficient then. Note that this is one of those situations where hash tables were made for. The millions of ifs and the tons of recreated locations can be eliminated then, resulting in a smaller trigger that is way faster.

 

[Valtarian]

Forgive me, I've been working with GUI for about 5 years now... I haven't learned the efficient way of doing things... I often times spend an abundant amount of time working with one trigger that's supposed to be really simple. I know my way around, just not efficiently. I apologize for not using WC3 tags--I forgot how to implement them on a thread.

As for the unit group: I'll fix that, I appreciate it.

Imp Midna: You're correct... Point-Remove is the same as the custom script Solu9 montioned, and as far as hash tables are concerned; I've honestly never even looked into them. I appreciate it.. I'll see what I can do.

Thanks!

 

[Valtarian]

Actually, Imp Midna... I'm kind of looking up hash tables online... It makes sense, but how could I make my trigger more effective by using a hash table when I need to list multiple units in different unit groups who cast different abilities? Could you possibly post an example for me, please?

 

[Juggernaut]

Im posting off-topic here but, how did you change that Point - Remove ? I want it too.

 

[Valtarian]

Im posting off-topic here but, how did you change that Point - Remove ? I want it too.

I use Jass NewGen... Point- Remove Point is an action right in-between the Leaderboard and Melee Game section.

 

[Imp Midna]

Im posting off-topic here but, how did you change that Point - Remove ? I want it too.

You can archive the same effect by copying and modifying the TriggerData, TriggerStrings and WorldEditStrings.txt from the main MPQ or downloading it from the internet and modify it. Tutorials can be found in the internet. You can add pratically every function availabe on this way (DestroyGroup, RemoveLocation, DestroyForce, some natives instead of the BJ versions etc etc). Note that people who did not do that will not be able to open your map unless they delete the GUI triggers though.

Actually, Imp Midna... I'm kind of looking up hash tables online... It makes sense, but how could I make my trigger more effective by using a hash table when I need to list multiple units in different unit groups who cast different abilities? Could you possibly post an example for me, please?

I guess with this piece of the trigger i might have gotten a wrong impression and i apologise if i did so. What i expected when you wrote "i have 30-40 more units to apply this to" is that you have this if construct a few more times under it, using different units with abilitys

So, the thing in your trigger though about is that you have 40 different units, which means 40 times this if, else construct which means, in the worst case, its gonna go through 40 ifs until he reaches the real one. Using a hashtable you can easily reduce the complexity from O(n) to O(1) which is a huge advantage.

Example for hashtable using:
(I hope you can still read it though part of it is german, i didnt figure out how to set my editor to english just yet though. I manually translated the mutch i can)

• init
  • Events
  • Map initialization
  • Conditions
  • Actions
  • -------- creating the hashtable --------
  • Hashtable - Create a hashtable
  • -------- Asuming A000 etc are the unit types. You have to get them using custom scripts because --------
  • -------- a GUI unit type is actualy an integer so there is no hashtable function for it --------
  • -------- but neither is there a GUI function to convert unit type into an integer. --------
  • -------- (though you could modify your UI to be able to do that i guess) --------
  • Custom script: set udg_TempInt = 'A000'
  • Hashtable - Save True as 0 of TempInt in (Last created hashtable)
  • Hashtable - Save abilityCommand1 as 1 of TempInt in (Last created hashtable)
  • Custom script: set udg_TempInt = 'A001'
  • Hashtable - Save True as 0 of TempInt in (Last created hashtable)
  • Hashtable - Save otherAbility as 1 of TempInt in (Last created hashtable)
  • Custom script: set udg_TempInt = 'A002'
  • Hashtable - Save True as 0 of TempInt in (Last created hashtable)
  • Hashtable - Save A002sAbility as 1 of TempInt in (Last created hashtable)
  • Custom script: set udg_TempInt = 'A003'
  • Hashtable - Save True as 0 of TempInt in (Last created hashtable)
  • Hashtable - Save imUnimaginative as 1 of TempInt in (Last created hashtable)
  • Set SomeHashtable = (Last created hashtable)

• attacked trigger
  • Events
  • Unit - A unit owned by Spieler 8 (Rosa) Is Attacked
  • Unit - A unit owned by Spieler 10 (Hellblau) Is Attacked
  • Unit - A unit owned by Spieler 12 (Braun) Is Attacked
  • Conditions
  • Actions
  • Set TempUnitgroup = (Units in YourArea <gen>)
  • Unit Group - Pick every unit in TempUnitgroup and do (Actions)
  • Loop - Actions
  • Custom script: set udg_TempInt = GetUnitTypeId(GetEnumUnit())
  • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
  • (Load 0 of TempInt from SomeHashtable) Gleich True
  • Then - Actions
  • -------- now you allready know its one of the 40 units this trigger applies to --------
  • -------- there is no need to have 40 ifs now --------
  • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
  • ((Picked unit) is in SomeGroup) Gleich True
  • Then - Actions
  • -------- Loading the correct Ability to cast here --------
  • Set TempString = (Load 1 of TempInt from SomeHashtable)
  • -------- again, a GUI limtation forces me to use a custom script at this point. --------
  • -------- As i can see, you CAN use variables for this instruction, so you can mostlikely --------
  • -------- use the gui function for this. --------
  • Custom script: call IssueTargetOrderBJ(GetEnumUnit(), udg_TempString, GetAttacker())
  • -------- [Do Move Stuff. Note that you might put a wait here since ] --------
  • -------- [Issuing the Skill cast and instantly giving a move order will] --------
  • -------- [usually not work. Also consider putting the turn off above the moves] --------
  • -------- [if you decide to use a wait.] --------
  • Trigger - Turn off (This trigger)
  • Skip remaining actions
  • Else - Actions
  • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
  • ((Picked unit) is in SomeOtherGroup) Gleich True
  • Then - Actions
  • Set TempString = (Load 1 of TempInt from SomeHashtable)
  • Custom script: call IssueTargetOrderBJ(GetEnumUnit(), udg_TempString, GetAttacker())
  • -------- [Do Move Stuff Again] --------
  • Trigger - Turn off (This trigger)
  • Skip remaining actions
  • Else - Actions
  • Else - Actions
  • Unit Group - Destroy TargetGroup

There is no need to go through 40 ifs on this way then. Adding new units to this trigger is way easier on that way either, you just need to put them into the hashtable in the init trigger.