• 🏆 Texturing Contest #33 is OPEN! Contestants must re-texture a SD unit model found in-game (Warcraft 3 Classic), recreating the unit into a peaceful NPC version. 🔗Click here to enter!
  • It's time for the first HD Modeling Contest of 2024. Join the theme discussion for Hive's HD Modeling Contest #6! Click here to post your idea!

[Trigger] How Do I Make This More Efficient?

Status
Not open for further replies.
Level 22
Joined
Jul 25, 2009
Messages
3,091
Once you get enough units fielded (maybe 20-30 for one player) this starts lagging like ass, it works exactly as I want it to, it's just not scaleable. How can I make it more efficient. Or completely rework it so it doesn't need to work with a loop at all, (I know I can use a dds to detect hp loss but not hp gain, and I have another loop just like this one that detects mana loss and gain which I need as well.)

  • HP Check
    • Events
      • Time - Every 0.03 seconds of game time
    • Conditions
    • Actions
      • Set TempGroup = (Units in (Playable map area) matching (((Level of Squad Leader Attachment for (Matching unit)) Greater than 0) and (((Matching unit) is dead) Equal to False)))
      • Unit Group - Pick every unit in TempGroup and do (Actions)
        • Loop - Actions
          • Set TempGroup2 = (Units in (Playable map area) matching (((Custom value of (Picked unit)) Equal to (Custom value of (Matching unit))) and ((((Matching unit) is dead) Equal to False) and ((Picked unit) Not equal to (Matching unit)))))
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • (Percentage life of GLeader[(Custom value of (Picked unit))]) Less than or equal to (GPercentage[(Custom value of (Picked unit))] x (GCurrentSquadSize[(Custom value of (Picked unit))] - 1.00))
            • Then - Actions
              • Set TempUnit = (Random unit from TempGroup2)
              • Unit - Kill TempUnit
              • Set GCurrentSquadSize[(Custom value of (Picked unit))] = (GCurrentSquadSize[(Custom value of (Picked unit))] - 1.00)
              • Unit - Set mana of GLeader[(Custom value of (Picked unit))] to ((Percentage mana of GLeader[(Custom value of (Picked unit))]) - GPercentage[(Custom value of (Picked unit))])%
            • Else - Actions
      • Unit Group - Pick every unit in TempGroup and do (Actions)
        • Loop - Actions
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • (Percentage life of GLeader[(Custom value of (Picked unit))]) Greater than (GPercentage[(Custom value of (Picked unit))] x (GCurrentSquadSize[(Custom value of (Picked unit))] - 0.00))
              • GCurrentSquadSize[(Custom value of (Picked unit))] Less than (Real((Point-value of (Unit-type of (Picked unit)))))
            • Then - Actions
              • Set GPoint[1] = (Position of (Picked unit))
              • Set tempLoc2 = (GPoint[1] offset by 160.00 towards (72.00 x (Real(GInt))) degrees)
              • Unit - Create 1 Locust Dummy for (Owner of (Picked unit)) at tempLoc2 facing (Facing of (Picked unit)) degrees
              • Unit - Add Locust Switcher to (Last created unit)
              • Custom script: call UnitAddAbility(GetLastCreatedUnit(),'Aloc')
              • Custom script: call UnitRemoveAbility(GetLastCreatedUnit(),'Aloc')
              • Unit - Set level of Locust Switcher for (Last created unit) to (Level of (Picked unit))
              • AI - Ignore (Last created unit)'s guard position
              • Unit - Set the custom value of (Last created unit) to (Custom value of (Picked unit))
              • Special Effect - Create a special effect at tempLoc2 using Abilities\Spells\Items\AIlm\AIlmTarget.mdl
              • Special Effect - Destroy (Last created special effect)
              • Unit - Disable supply usage for (Last created unit)
              • Set GCurrentSquadSize[(Custom value of (Picked unit))] = (GCurrentSquadSize[(Custom value of (Picked unit))] + 1.00)
              • Custom script: call RemoveLocation(udg_GPoint[1])
              • Custom script: call RemoveLocation(udg_GPoint[2])
            • Else - Actions
      • Custom script: call DestroyGroup(udg_TempGroup)
      • Custom script: call DestroyGroup(udg_TempGroup2)
 
Level 22
Joined
Aug 27, 2013
Messages
3,973
"Set TempGroup = (Units in (Playable map area) matching (((Level of Squad Leader Attachment for (Matching unit)) Greater than 0) and (((Matching unit) is dead) Equal to False)))"
-This line leaks, you should store the location into variable first and remove it in the end.

"Set TempGroup2 = (Units in (Playable map area) matching (((Custom value of (Picked unit)) Equal to (Custom value of (Matching unit))) and ((((Matching unit) is dead) Equal to False) and ((Picked unit) Not equal to (Matching unit)))))"
-Same goes with the first

-Combine the pick every unit in TempGroup
-Remove the temploc2
 
Level 37
Joined
Jul 22, 2015
Messages
3,485
For RiotZ:
  • Don't use (Matching unit) to filter out units; just find a radius and then filter them out using If/Then/Else Multiple Actions inside the loop
  • Store things into variables (ex: Picked unit, last created unit)
  • You use RemoveLocation(udg_GPoint[2]), but I never see you use the actual point
  • tempLoc2 leaks a location
  • I'm pretty sure you can combine the two unit group loops together, but I just did a quick scan of the triggers

Are you saying "Playable Map Area" leaks?
you should store the location into variable first and remove it in the end.
(Playable map area) is a rect, not a location.
 
Level 22
Joined
Jul 25, 2009
Messages
3,091
For RiotZ:
  • Don't use (Matching unit) to filter out units; just find a radius and then filter them out using If/Then/Else Multiple Actions inside the loop
  • Store things into variables (ex: Picked unit, last created unit)
  • You use RemoveLocation(udg_GPoint[2]), but I never see you use the actual point
  • tempLoc2 leaks a location
  • I'm pretty sure you can combine the two unit group loops together, but I just did a quick scan of the triggers



(Playable map area) is a rect, not a location.

1. Elaborate further on this one.
2. Why? Unit triggers don't leak?
3. You're right, I fucked up here and never noticed it.
4. Same as 3.
5. I don't see how, since one references ONLY squad leaders (TempGroup) while the other one references ONLY squad members, if you can think of a way to do this off the top of your head hmu fam.
 
Level 37
Joined
Jul 22, 2015
Messages
3,485
1. Elaborate further on this one.
(Matching unit) is inefficient since it constantly has to call for the function (Matching unit). It also makes it tedious to read, and add or remove filters when you need to. This is how I prefer doing them:
  • Unit Group - Pick every unit in (Units in (Playable map area)) and do (Actions)
    • Loop - Actions
      • Set TempUnit = (Picked unit)
      • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        • If - Conditions
          • (TempUnit is A structure) Equal to False
          • (TempUnit is Magic Immune) Equal to False
          • (TempUnit is alive) Equal to True
          • -------- easily add/remove conditions --------
        • Then - Actions
          • -------- TempUnit passed filters; do actions --------
        • Else - Actions
          • -------- TempUnit did not pass filters; do nothing --------

Why? Unit triggers don't leak?
Huh...? I don't know what that means. The reason I suggest doing it is that you don't have to keep calling the function every single time you need to refer to either of those units, you just go to the variable. The less parentheses your trigger has, the better.

I don't see how, since one references ONLY squad leaders (TempGroup) while the other one references ONLY squad members, if you can think of a way to do this off the top of your head hmu fam.
I'll take a look at it after I'm done eating my noodles :3
 
Level 22
Joined
Jul 25, 2009
Messages
3,091
(Matching unit) is inefficient since it constantly has to call for the function (Matching unit). It also makes it tedious to read, and add or remove filters when you need to. This is how I prefer doing them:
  • Unit Group - Pick every unit in (Units in (Playable map area)) and do (Actions)
    • Loop - Actions
      • Set TempUnit = (Picked unit)
      • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        • If - Conditions
          • (TempUnit is A structure) Equal to False
          • (TempUnit is Magic Immune) Equal to False
          • (TempUnit is alive) Equal to True
          • -------- easily add/remove conditions --------
        • Then - Actions
          • -------- TempUnit passed filters; do actions --------
        • Else - Actions
          • -------- TempUnit did not pass filters; do nothing --------


Huh...? I don't know what that means. The reason I suggest doing it is that you don't have to keep calling the function every single time you need to refer to either of those units, you just go to the variable. The less parentheses your trigger has, the better.


I'll take a look at it after I'm done eating my noodles :3

I'm going to change both of these right now, I appreciate your help and I'll look for you response after you're done eating your noodles. :p
 
Level 22
Joined
Jul 25, 2009
Messages
3,091
You're creating a new TempGroup2 for every iteration of the first TempGroup. You need to move the DestroyGroup(udg_TempGroup2) inside the loop.

You're absolutely right.

Edit: It looks like after all of your recommended changes the frame rate is MUCH higher under stress, but it's still a bit choppy considering this map is designed to be 4 player, I'm sure there's room for improvement yet.

ALSO: I have a loop that checks the mana of a squad to decide whether or not to add or remove negative buffs. This is going to hit the framerate as well, having 2 loops running at once. So I'm wondering if I can combine them somehow after this one is optimized.
 
Level 22
Joined
Jul 25, 2009
Messages
3,091
  • HP Check
    • Events
      • Time - Every 0.03 seconds of game time
    • Conditions
    • Actions
      • Unit Group - Pick every unit in (Units in (Playable map area)) and do (Actions)
        • Loop - Actions
          • Set TempUnit = (Picked unit)
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • (TempUnit is alive) Equal to True
              • (Level of Squad Leader Attachment for TempUnit) Greater than 0
              • (Percentage life of GLeader[(Custom value of TempUnit)]) Less than or equal to (GPercentage[(Custom value of TempUnit)] x (GCurrentSquadSize[(Custom value of TempUnit)] - 1.00))
            • Then - Actions
              • Set TempGroup2 = (Units in (Playable map area) matching (((Custom value of TempUnit) Equal to (Custom value of (Matching unit))) and ((((Matching unit) is dead) Equal to False) and (TempUnit Not equal to (Matching unit)))))
              • Set TempUnit2 = (Random unit from TempGroup2)
              • Unit - Kill TempUnit2
              • Set GCurrentSquadSize[(Custom value of (Picked unit))] = (GCurrentSquadSize[(Custom value of (Picked unit))] - 1.00)
              • Unit - Set mana of GLeader[(Custom value of (Picked unit))] to ((Percentage mana of GLeader[(Custom value of (Picked unit))]) - GPercentage[(Custom value of (Picked unit))])%
              • Custom script: call DestroyGroup(udg_TempGroup2)
            • Else - Actions
              • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                • If - Conditions
                  • (Percentage life of GLeader[(Custom value of TempUnit)]) Greater than (GPercentage[(Custom value of TempUnit)] x (GCurrentSquadSize[(Custom value of TempUnit)] - 0.00))
                  • GCurrentSquadSize[(Custom value of TempUnit)] Less than (Real((Point-value of (Unit-type of TempUnit))))
                • Then - Actions
                  • Set GPoint[1] = (Position of TempUnit)
                  • Set tempLoc2 = (GPoint[1] offset by 160.00 towards (72.00 x (Real(GInt))) degrees)
                  • Unit - Create 1 Locust Dummy for (Owner of TempUnit) at tempLoc2 facing (Facing of TempUnit) degrees
                  • Unit - Add Locust Switcher to (Last created unit)
                  • Custom script: call UnitAddAbility(GetLastCreatedUnit(),'Aloc')
                  • Custom script: call UnitRemoveAbility(GetLastCreatedUnit(),'Aloc')
                  • Unit - Set level of Locust Switcher for (Last created unit) to (Level of TempUnit)
                  • AI - Ignore (Last created unit)'s guard position
                  • Unit - Set the custom value of (Last created unit) to (Custom value of TempUnit)
                  • Special Effect - Create a special effect at tempLoc2 using Abilities\Spells\Items\AIlm\AIlmTarget.mdl
                  • Special Effect - Destroy (Last created special effect)
                  • Unit - Disable supply usage for (Last created unit)
                  • Set GCurrentSquadSize[(Custom value of TempUnit)] = (GCurrentSquadSize[(Custom value of TempUnit)] + 1.00)
                  • Custom script: call RemoveLocation(udg_GPoint[1])
                  • Custom script: call RemoveLocation(udg_tempLoc2)
                • Else - Actions
      • Custom script: call DestroyGroup(udg_TempGroup)
 
Level 22
Joined
Jul 25, 2009
Messages
3,091
To give you a better grasp of what the system does, when a leader unit, ie TempUnit, gets below a predetermined % of hp, it will remove a member of the squad ie, TempUnit2, until all squad members are dead except the leader.

And on the flipside, it checks if the hp surpasses the predetermined % in order to add units back to the squad.
 
Level 12
Joined
Jan 2, 2016
Messages
973
1) You could make the trigger run every 0.05 seconds instead of every 0.03. You wouldn't notice the difference in performance (of the trigger), but it will become lighter for executing.
2) Instead of re-making unit groups all the time, you can just have a group array, which is constant:
Group[0] contains the "leaders"
Group[1/2/3/4] countains the "soldiers"
So just pick every unit in Group[0] (instead of all units in the playable map area), and then check the custom value of the leader (hopefully it will be 1/2/3/4, otherwise your array will get quite messy), and pick all units in the representive group to do your actions.
When you create units for a leader - you put them in his group, and when you kill them - you remove them from there.
This way you wouldn't need to create and destroy groups all the time.

-----------------------------------------------------------------------------------------------------------------------------------------------------
However, if you don't wanna listen to my 2-nd suggestion, what you can fix:
You aren't setting TempGroup in your trigger, you are just picking all the units in the map ( the 1-st row of your trigger)
And you are still using the "matching" for TempGroup2.
 
Level 22
Joined
Jul 25, 2009
Messages
3,091
1) You could make the trigger run every 0.05 seconds instead of every 0.03. You wouldn't notice the difference in performance (of the trigger), but it will become lighter for executing.
2) Instead of re-making unit groups all the time, you can just have a group array, which is constant:
Group[0] contains the "leaders"
Group[1/2/3/4] countains the "soldiers"
So just pick every unit in Group[0] (instead of all units in the playable map area), and then check the custom value of the leader (hopefully it will be 1/2/3/4, otherwise your array will get quite messy), and pick all units in the representive group to do your actions.
When you create units for a leader - you put them in his group, and when you kill them - you remove them from there.
This way you wouldn't need to create and destroy groups all the time.

-----------------------------------------------------------------------------------------------------------------------------------------------------
However, if you don't wanna listen to my 2-nd suggestion, what you can fix:
You aren't setting TempGroup in your trigger, you are just picking all the units in the map ( the 1-st row of your trigger)
And you are still using the "matching" for TempGroup2.

The problem with this is that, some strange bug in warcraft's editor makes it so that you cannot add locust units to a unit group, which is what the /soldiers/ are, they're locusted, so you can only select the squad leader.

Making the loop run on 0.05 is something I considered but the trigger originally ran on 0.33 and wouldn't fire soon enough to remove units from the group accurately, so I don't want to push my luck.
 
Level 12
Joined
Jan 2, 2016
Messages
973
What can I say....
"Pick every unit in (Playable Map Area)" is also a unit group, you just don't set a variable to it, thus it leaks (if you don't use set bj_wantDestroyGroup = true).
So aparently - you can pick locust(ed) units.

On my map I have a trigger that picks a locusted unit too, and it works fine.

However, you can still give them the Locust later:
Create unit
Add unit to the group
Add locust to the unit

And after that - they will be in the unit group for sure, and you can pick them.


0.03 seconds is 11 times faster than 0.33; 0.05 seconds is 6.6 times faster than 0.33.
I'd say that if 0.33 wasn't fast enough, making it 6.6 times faster would make it QUITE faster.
 
Level 24
Joined
Aug 1, 2013
Messages
4,657
First of all, you should be realizing what you are doing...
Pick every unit in the map... ALL UNITS... EVERY 0.03 seconds...
So, for 30 units, you pick 1000 units per second. (Each unit 33.3... times.)

Then... for each unit that passes the first checks, you pick ALL UNITS... AGAIN.
It can get pretty heavy for no reason at all if you have proper data structure.


Another thing that is really overused in your trigger is stuff like this:
  • (Percentage life of GLeader[(Custom value of TempUnit)]) Less than or equal to (GPercentage[(Custom value of TempUnit)] x (GCurrentSquadSize[(Custom value of TempUnit)] - 1.00))
"(Custom value of TempUnit)" is used 3 times here.
This is a function call (which is pretty heavy in JASS).
What you do is ask the computer to go and search to find the custom value of the unit.
Then the computer goes to find the unit first, then asks the custom value of that unit.
Then he will return to you with the value.

Dont you think it is more efficient to ask it only once and then store the value in a variable which you can read out and change with minimal processing power?
At least I do, but now you will too.
(This is the same for every GUI action except variables and a few small exceptions...
But stuff like (Triggering unit), (Owner of <unit>), (Picked unit), (Life of <unit>), etc)


Then there is one small thing:
  • Unit - Kill TempUnit2
This leaks, so you should use "Unit - Add expiration timer".
Use "generic" and 0.01 just for simple use.
The unit wont be dead immediately, but he will still die without any visual difference.
If you want it to die immediately, you could damage it with tons of damage or set its health to 0.


For maybe a better data structure and not having to care about thousands of units that have nothing to do with the actual effect of the spell, can you explain what it has to do?
 
Level 22
Joined
Jul 25, 2009
Messages
3,091
First of all, you should be realizing what you are doing...
Pick every unit in the map... ALL UNITS... EVERY 0.03 seconds...
So, for 30 units, you pick 1000 units per second. (Each unit 33.3... times.)

Then... for each unit that passes the first checks, you pick ALL UNITS... AGAIN.
It can get pretty heavy for no reason at all if you have proper data structure.


Another thing that is really overused in your trigger is stuff like this:
  • (Percentage life of GLeader[(Custom value of TempUnit)]) Less than or equal to (GPercentage[(Custom value of TempUnit)] x (GCurrentSquadSize[(Custom value of TempUnit)] - 1.00))
"(Custom value of TempUnit)" is used 3 times here.
This is a function call (which is pretty heavy in JASS).
What you do is ask the computer to go and search to find the custom value of the unit.
Then the computer goes to find the unit first, then asks the custom value of that unit.
Then he will return to you with the value.

Dont you think it is more efficient to ask it only once and then store the value in a variable which you can read out and change with minimal processing power?
At least I do, but now you will too.
(This is the same for every GUI action except variables and a few small exceptions...
But stuff like (Triggering unit), (Owner of <unit>), (Picked unit), (Life of <unit>), etc)


Then there is one small thing:
  • Unit - Kill TempUnit2
This leaks, so you should use "Unit - Add expiration timer".
Use "generic" and 0.01 just for simple use.
The unit wont be dead immediately, but he will still die without any visual difference.
If you want it to die immediately, you could damage it with tons of damage or set its health to 0.


For maybe a better data structure and not having to care about thousands of units that have nothing to do with the actual effect of the spell, can you explain what it has to do?

I have explained in depth throughout the thread...

It finds squads where the health is lower or higher than a predetermined threshold which is based on a percentage.

Ex.

Squad size = 6

HP of leader = 900 (900 HP accounts for the health of every member, so to determine each member's individual HP we have to use some basic arithmetic, 900/6 = 150, this is how we determine the threshold at which squad members are added or removed)

HP Threshold = 150 * Squad size - 1 (this is updated every time a unit is added or removed)

A full squad will lose OR gain its first member when its leader's HP drops OR rises to 750.

750 - 150 = your next threshold = 600, and so on
 
Level 24
Joined
Aug 1, 2013
Messages
4,657
What I find a more realistic method is to let the leader display the average health of each unit of its squad. That way, you still have proper health evaluation by the leader and not have one unlucky unit take all the damage until he dies and passes the black spot on to the next unlucky unit.

But apart from realistical features, the approach could be optimized by listing all squads and listing all units in a squad (can be done in a simple way by a unit group).

What you want to end up with is a list of squads.
A squad is an object similar to the following:
Code:
Squad: {
    leader: (unit)
    members: (unit group)
    membersCount: (integer)
    hpThreshold: (real)
}
Every squad has a leader, a list of members, a number that tells you the number of members and for efficiency reasons a hp threshold.

Because none of the attributes of the squad is an array, you are a lucky person.
You can make 4 variables in your map:
- Squad_Leader (unit array)
- Squad_Members (unit group array)
- Squad_MemberCount (integer array)
- Squad_HpThreshold (real array)

You can use dynamic indexing to create a list of all squads and keep their data in sync.

Then, you only have to loop over all squads and check only the units in each squad.
This way, you wont need the custom value of the units (which is preferred to be reserved for unit indexer systems which give you infinite* custom values of any and all types).

* Infinite as in finite within the game's capacity... iIrc, ~25k variables.
 
Level 22
Joined
Jul 25, 2009
Messages
3,091
What I find a more realistic method is to let the leader display the average health of each unit of its squad. That way, you still have proper health evaluation by the leader and not have one unlucky unit take all the damage until he dies and passes the black spot on to the next unlucky unit.

But apart from realistical features, the approach could be optimized by listing all squads and listing all units in a squad (can be done in a simple way by a unit group).

What you want to end up with is a list of squads.
A squad is an object similar to the following:
Code:
Squad: {
    leader: (unit)
    members: (unit group)
    membersCount: (integer)
    hpThreshold: (real)
}
Every squad has a leader, a list of members, a number that tells you the number of members and for efficiency reasons a hp threshold.

Because none of the attributes of the squad is an array, you are a lucky person.
You can make 4 variables in your map:
- Squad_Leader (unit array)
- Squad_Members (unit group array)
- Squad_MemberCount (integer array)
- Squad_HpThreshold (real array)

You can use dynamic indexing to create a list of all squads and keep their data in sync.

Then, you only have to loop over all squads and check only the units in each squad.
This way, you wont need the custom value of the units (which is preferred to be reserved for unit indexer systems which give you infinite* custom values of any and all types).

* Infinite as in finite within the game's capacity... iIrc, ~25k variables.

I could simply make the health of the members equivalent to the total health of the leader to fix the problem of randomness and I may just do that.

It's better for me to use a % to check the hp threshold over a flat number like 600.00, 500.00 etc, because I can modify the % much easier at any time, it's a bit harder to understand but much more effective.
 
Status
Not open for further replies.
Top