[Trigger] Lag Buildup overtime

Status
Not open for further replies.

sentrywiz

S

sentrywiz

:vw_sad:

This is the third time I make a thread about this. Its basically the same problem. My two other threads:

http://www.hiveworkshop.com/forums/triggers-scripts-269/leaks-i-cant-see-find-253539/

And

http://www.hiveworkshop.com/forums/triggers-scripts-269/map-huge-lag-cant-find-issue-253481/#post2542720

So I couldn't fix the issue in the last map. So I was like okay, fck it.
Then at some later date, I started a new map and remade the trigger, but I split it into multiple triggers, thinking that was the problem.

NOPE!


The same thing is back to haunt my dreams again.


  • Fire Shot
    • Events
      • Unit - A unit Starts the effect of an ability
    • Conditions
      • (Ability being cast) Equal to Fire Shot
    • Actions
      • Set Total_Shots = (Total_Shots + 1)
      • Set Shot_Loc = (Position of (Casting unit))
      • Set Shot_Loc2 = (Shot_Loc offset by 50.00 towards (Facing of (Casting unit)) degrees)
      • Set Shot_Target_Loc[(Player number of (Owner of (Casting unit)))] = (Target point of ability being cast)
      • Set Shot_Owner[(Player number of (Owner of (Casting unit)))] = (Casting unit)
      • Unit - Create 1 Basic Shot for (Owner of (Casting unit)) at Shot_Loc2 facing (Facing of (Casting unit)) degrees
      • Unit Group - Add (Last created unit) to Shots_Group
      • Trigger - Turn on Move Shot <gen>
      • Custom script: call RemoveLocation ( udg_Shot_Loc )
      • Custom script: call RemoveLocation ( udg_Shot_Loc2 )
      • Custom script: call RemoveLocation (udg_Shot_Target_Loc[GetConvertedPlayerId(GetOwningPlayer(GetTriggerUnit()))])
  • Move Shot
    • Events
      • Time - Every 0.04 seconds of game time
    • Conditions
      • Total_Shots Greater than 0
    • Actions
      • Set Shots_Damage_Group = (Units within PLAYER_SHOTAREA[(Player number of (Owner of (Picked unit)))] of Shots_Move_Loc matching ((((Matching unit) is alive) Equal to True) and (((Matching unit) belongs to an enemy of (Owner of (Picked unit))) Equal to True)))
      • Unit Group - Pick every unit in Shots_Group and do (Actions)
        • Loop - Actions
          • Set Shots_Move_Loc = (Position of (Picked unit))
          • Set Shots_Move_Loc2 = (Shots_Move_Loc offset by PLAYER_SHOTSPEED[(Player number of (Owner of (Picked unit)))] towards (Facing of (Picked unit)) degrees)
          • Set Shots_Hit_Loc = (Units within PLAYER_SHOTAREA[(Player number of (Owner of (Picked unit)))] of Shots_Move_Loc matching ((((Matching unit) belongs to an enemy of (Owner of (Picked unit))) Equal to True) and ((((Matching unit) is alive) Equal to True) and (((Matching unit) is A
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • (Number of units in Shots_Hit_Loc) Greater than 0
            • Then - Actions
              • Trigger - Run Damage Target <gen> (ignoring conditions)
            • Else - Actions
              • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                • If - Conditions
                  • Shot_Travel_Range[(Player number of (Owner of (Picked unit)))] Less than PLAYER_SHOTRANGE[(Player number of (Owner of (Picked unit)))]
                • Then - Actions
                  • Set Shot_Travel_Range[(Player number of (Owner of (Picked unit)))] = (Shot_Travel_Range[(Player number of (Owner of (Picked unit)))] + PLAYER_SHOTSPEED[(Player number of (Owner of (Picked unit)))])
                  • Unit - Move (Picked unit) instantly to Shots_Move_Loc2, facing (Facing of (Picked unit)) degrees
                • Else - Actions
                  • Unit - Remove (Picked unit) from the game
                  • Set Shot_Travel_Range[(Player number of (Owner of (Picked unit)))] = 0.00
                  • Set Total_Shots = (Total_Shots - 1)
          • Custom script: call DestroyGroup ( udg_Shots_Hit_Loc )
          • Custom script: call RemoveLocation ( udg_Shots_Move_Loc )
          • Custom script: call RemoveLocation ( udg_Shots_Move_Loc2 )
      • Custom script: call DestroyGroup ( udg_Shots_Damage_Group )
  • Damage Target
    • Events
    • Conditions
    • Actions
      • Set Shot_Player = (Random unit from Shots_Hit_Loc)
      • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        • If - Conditions
          • Or - Any (Conditions) are true
            • Conditions
              • (Shot_Player has buff Evade ) Not equal to True
              • PLAYER_RESISTSHOT[(Player number of (Owner of Shot_Player))] Less than 3
        • Then - Actions
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • PLAYER_SHOTCRIT[(Player number of (Owner of (Picked unit)))] Greater than 0.00
              • (Random integer number between 1 and 10) Less than or equal to 2
            • Then - Actions
              • Set Calc_Damage = (PLAYER_SHOTDAMAGE[(Player number of (Owner of (Picked unit)))] - (PLAYER_DAMAGERED[(Player number of (Owner of Shot_Player))] x PLAYER_SHOTDAMAGE[(Player number of (Owner of (Picked unit)))]))
              • Set Calc_Damage = (Calc_Damage x 2.00)
              • Unit - Cause (Picked unit) to damage (Random unit from Shots_Hit_Loc), dealing Calc_Damage damage of attack type Chaos and damage type Universal
              • Floating Text - Create floating text that reads (String((Integer(Calc_Damage)))) above Shot_Player with Z offset 0.00, using font size 14.00, color (100.00%, 0.00%, 0.00%), and 0.00% transparency
              • Floating Text - Set the velocity of (Last created floating text) to 90.00 towards 90.00 degrees
              • Floating Text - Change (Last created floating text): Disable permanence
              • Floating Text - Change the lifespan of (Last created floating text) to 1.50 seconds
              • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                • If - Conditions
                  • PLAYER_LIFESTEAL[(Player number of (Owner of (Picked unit)))] Greater than 0.00
                • Then - Actions
                  • Set Calc_Lifesteal = (Calc_Damage x PLAYER_LIFESTEAL[(Player number of (Owner of (Picked unit)))])
                  • Unit - Set life of Shot_Owner[(Player number of (Owner of (Picked unit)))] to ((Life of Shot_Owner[(Player number of (Owner of (Picked unit)))]) + Calc_Lifesteal)
                • Else - Actions
            • Else - Actions
              • Set Calc_Damage = (PLAYER_SHOTDAMAGE[(Player number of (Owner of (Picked unit)))] - (PLAYER_DAMAGERED[(Player number of (Owner of Shot_Player))] x PLAYER_SHOTDAMAGE[(Player number of (Owner of (Picked unit)))]))
              • Unit - Cause (Picked unit) to damage (Random unit from Shots_Hit_Loc), dealing Calc_Damage damage of attack type Chaos and damage type Universal
              • Floating Text - Create floating text that reads (String((Integer(Calc_Damage)))) above Shot_Player with Z offset 0.00, using font size 10.00, color (100.00%, 10.00%, 10.00%), and 0.00% transparency
              • Floating Text - Set the velocity of (Last created floating text) to 90.00 towards 90.00 degrees
              • Floating Text - Change (Last created floating text): Disable permanence
              • Floating Text - Change the lifespan of (Last created floating text) to 1.50 seconds
              • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                • If - Conditions
                  • PLAYER_LIFESTEAL[(Player number of (Owner of (Picked unit)))] Greater than 0.00
                • Then - Actions
                  • Set Calc_Lifesteal = (Calc_Damage x PLAYER_LIFESTEAL[(Player number of (Owner of (Picked unit)))])
                  • Unit - Set life of Shot_Owner[(Player number of (Owner of (Picked unit)))] to ((Life of Shot_Owner[(Player number of (Owner of (Picked unit)))]) + Calc_Lifesteal)
                • Else - Actions
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • PLAYER_AREALVL[(Player number of (Owner of (Picked unit)))] Greater than or equal to 1
            • Then - Actions
              • Set burn_loc = (Position of (Picked unit))
              • Unit - Create 1 Burning Area for (Owner of (Picked unit)) at burn_loc facing Default building facing degrees
              • Unit - Add a 7.00 second Generic expiration timer to (Last created unit)
              • Animation - Play (Last created unit)'s Stand Medium animation
              • Special Effect - Create a special effect at burn_loc using Objects\Spawnmodels\Human\FragmentationShards\FragBoomSpawn.mdl
              • Special Effect - Destroy (Last created special effect)
              • Custom script: call RemoveLocation ( udg_burn_loc )
            • Else - Actions
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • PLAYER_RESISTSHOT[(Player number of (Owner of Shot_Player))] Less than 3
            • Then - Actions
              • Set PLAYER_RESISTSHOT[(Player number of (Owner of Shot_Player))] = (PLAYER_RESISTSHOT[(Player number of (Owner of Shot_Player))] + 1)
            • Else - Actions
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • PLAYER_BURNLVL[(Player number of (Owner of (Picked unit)))] Greater than 0
            • Then - Actions
              • Set Cast_Loc = (Position of (Picked unit))
              • Unit - Create 1 Dummy Cast for (Owner of (Picked unit)) at Cast_Loc facing Default building facing degrees
              • Set Dummy_Unit = (Last created unit)
              • Unit - Add a 3.00 second Generic expiration timer to Dummy_Unit
              • Unit - Add Bottle of Napalm to Dummy_Unit
              • Unit - Set level of Bottle of Napalm for Dummy_Unit to PLAYER_BURNLVL[(Player number of (Owner of (Picked unit)))]
              • Unit - Order Dummy_Unit to Undead Necromancer - Unholy Frenzy Shot_Player
              • Custom script: call RemoveLocation ( udg_Cast_Loc )
            • Else - Actions
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • PLAYER_REVEALTARG[(Player number of (Owner of (Picked unit)))] Greater than 0
            • Then - Actions
              • Set Cast_Loc = (Position of (Picked unit))
              • Unit - Create 1 Dummy Cast for (Owner of (Picked unit)) at Cast_Loc facing Default building facing degrees
              • Set Dummy_Unit = (Last created unit)
              • Unit - Add a 3.00 second Generic expiration timer to Dummy_Unit
              • Unit - Add Sniper Scope Reveal to Dummy_Unit
              • Unit - Order Dummy_Unit to Night Elf Druid Of The Talon - Faerie Fire Shot_Player
              • Custom script: call RemoveLocation ( udg_Cast_Loc )
            • Else - Actions
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • PLAYER_FREEZELVL[(Player number of (Owner of (Picked unit)))] Greater than 0
            • Then - Actions
              • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                • If - Conditions
                  • PLAYER_FREEZELVL[(Player number of (Owner of (Picked unit)))] Equal to 5
                  • (Random integer number between 1 and 10) Less than or equal to 3
                • Then - Actions
                  • Set Cast_Loc = (Position of (Picked unit))
                  • Unit - Create 1 Dummy Cast for (Owner of (Picked unit)) at Cast_Loc facing Default building facing degrees
                  • Set Dummy_Unit = (Last created unit)
                  • Unit - Add a 3.00 second Generic expiration timer to Dummy_Unit
                  • Unit - Add Freeze Gun (Freeze) to Dummy_Unit
                  • Unit - Order Dummy_Unit to Human Mountain King - Storm Bolt Shot_Player
                  • Custom script: call RemoveLocation ( udg_Cast_Loc )
                • Else - Actions
                  • Set Cast_Loc = (Position of (Picked unit))
                  • Unit - Create 1 Dummy Cast for (Owner of (Picked unit)) at Cast_Loc facing Default building facing degrees
                  • Set Dummy_Unit = (Last created unit)
                  • Unit - Add a 3.00 second Generic expiration timer to Dummy_Unit
                  • Unit - Add Freeze Gun to Dummy_Unit
                  • Unit - Set level of Freeze Gun for Dummy_Unit to PLAYER_FREEZELVL[(Player number of (Owner of (Picked unit)))]
                  • Unit - Order Dummy_Unit to Human Sorceress - Slow Shot_Player
                  • Custom script: call RemoveLocation ( udg_Cast_Loc )
            • Else - Actions
          • Unit - Remove (Picked unit) from the game
          • Set Shot_Travel_Range[(Player number of (Owner of (Picked unit)))] = 0.00
          • Set Total_Shots = (Total_Shots - 1)
        • Else - Actions
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • (Shot_Player has buff Evade ) Equal to True
            • Then - Actions
              • Floating Text - Create floating text that reads Evade above Shot_Player with Z offset 0.00, using font size 12.00, color (100.00%, 100.00%, 100.00%), and 25.00% transparency
              • Floating Text - Set the velocity of (Last created floating text) to 90.00 towards 90.00 degrees
              • Floating Text - Change (Last created floating text): Disable permanence
              • Floating Text - Change the lifespan of (Last created floating text) to 1.50 seconds
              • Unit - Remove (Picked unit) from the game
              • Set Shot_Travel_Range[(Player number of (Owner of (Picked unit)))] = 0.00
              • Set Total_Shots = (Total_Shots - 1)
            • Else - Actions
              • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                • If - Conditions
                  • PLAYER_RESISTSHOT[(Player number of (Owner of Shot_Player))] Greater than or equal to 3
                • Then - Actions
                  • Set PLAYER_RESISTSHOT[(Player number of (Owner of Shot_Player))] = 0
                  • Floating Text - Create floating text that reads Resist above Shot_Player with Z offset 0.00, using font size 12.00, color (100.00%, 100.00%, 100.00%), and 25.00% transparency
                  • Floating Text - Set the velocity of (Last created floating text) to 90.00 towards 90.00 degrees
                  • Floating Text - Change (Last created floating text): Disable permanence
                  • Floating Text - Change the lifespan of (Last created floating text) to 1.50 seconds
                  • Unit - Remove (Picked unit) from the game
                  • Set Shot_Travel_Range[(Player number of (Owner of (Picked unit)))] = 0.00
                  • Set Total_Shots = (Total_Shots - 1)
                • Else - Actions


In short the trigger is about firing skillshot missiles that cause damage and effects on hit. I thought that GUI was messed up about this, until I saw other GUI spells that are FAR FAR more complex than this that don't suffer from any lag. So its definitely me and the way I trigger.

Tested it for about 10-15 times with /fps command. It starts at 64.4 and it drops to 50.3 in about 2-3 minutes of firing the spell.

There are no units stacking up. I monitored the unit generation through in-game messages and its not that.

I'd REALLY love if anyone could point out what am I doing so wrong that this problem appears in two completely separate maps ( I didn't copy any triggers from either map, made both from scratch ) and the exactly same thing is happening.

:vw_wtf:
 
Along with what was said above you need to make you triggers more efficient.
Anything used twice or more should be stored in a variable. The variable should then be used.

You should try to condense your actions as much as possible. There are several instances that you use the same actions in several ITEs. Remove all of the same actions and put them before the ITEs

Never use casting unit use triggering unit.
Also in the unit cast trigger use Triggering Player rather than owner of casting unit.
 

sentrywiz

S

sentrywiz

Thanks for all of your replies.

@jonhysone - Here is the map

View attachment Tag Team DM.w3x

@deathismyfriend - I do put everything I use multiple times in a variable.

@Daffa the Mage - Doubtful. The previous map was like that, and it still lagged like nuts.
 
Level 27
Joined
Sep 26, 2009
Messages
2,474
...sigh...
the solution to your problem is really simple. I just saw your thread, downloaded map, checked what I thought may be the problem and voala - indeed did the problem lie there.

Apart from improving your triggers a bit, the problem never was with the trigger, but with the ability itself.
NEVER use shockwave spell as the base for any ability! Not to mention the way you had it set up.
The reason for that is because of its terrain deformation (which Shockwave does). Even if you set it to 0, it will still calculate terrain deformation and deform terrain. You basically set speed of the missile that deforms terrain to 0 and its duration to 0 as well, which in most cases (like this) means "infinite duration". So the problem was with your PC having to repeatedly and endlessly calculate many terrain deformations.

How to fix? Easily - base your Fire Shot ability off Channel skill, which is actually intended for triggered spells.

My own tests:
1) I disabled the missile system, since there was no need for it.
2) Shockwave-based spell caused my FPS to permanently drop below 60 after 12 uses, then it went down by additional 2-3 fps per use
3) Channel-based spell never caused my FPS to drop below 60 (and if it did, it eventually returned back). Even after 30 uses, my FPS remained at 60.
4) Enabled missile system and used the channel-based ability instead. After firing 15 shots fps was still at 60.

(I would seriously kick anyone who promotes Shockwave as base spell for anything in balls and decapitate them... even saw a few tutorials where it was written to base spells off it :< Then such problems come for the mappers)
 

sentrywiz

S

sentrywiz

...sigh...
the solution to your problem is really simple. I just saw your thread, downloaded map, checked what I thought may be the problem and voala - indeed did the problem lie there.

Apart from improving your triggers a bit, the problem never was with the trigger, but with the ability itself.
NEVER use shockwave spell as the base for any ability! Not to mention the way you had it set up.
The reason for that is because of its terrain deformation (which Shockwave does). Even if you set it to 0, it will still calculate terrain deformation and deform terrain. You basically set speed of the missile that deforms terrain to 0 and its duration to 0 as well, which in most cases (like this) means "infinite duration". So the problem was with your PC having to repeatedly and endlessly calculate many terrain deformations.

How to fix? Easily - base your Fire Shot ability off Channel skill, which is actually intended for triggered spells.

My own tests:
1) I disabled the missile system, since there was no need for it.
2) Shockwave-based spell caused my FPS to permanently drop below 60 after 12 uses, then it went down by additional 2-3 fps per use
3) Channel-based spell never caused my FPS to drop below 60 (and if it did, it eventually returned back). Even after 30 uses, my FPS remained at 60.
4) Enabled missile system and used the channel-based ability instead. After firing 15 shots fps was still at 60.

(I would seriously kick anyone who promotes Shockwave as base spell for anything in balls and decapitate them... even saw a few tutorials where it was written to base spells off it :< Then such problems come for the mappers)

SERIOUSLY? IT WAS F**KING SHOCKWAVE?

oh man... I'll credit you everywhere if this is really the problem. Ill test both maps. But what you say makes a ton of sense with the terrain deformation

EDIT: ...sigh... it really was shockwave. Both maps work flawlessly now.. well except for my crappy triggering I've been testing them for about half an hour each and both never left 63.3 fps and always go back to 64.0 fps.

Thanks man. You may not realize it, but I was literally gonna quit map making because I thought my triggering sucked so much that every map I make is gonna lag out.

+REP! EVERYWHERE

But TBH, it was my fault anyway. If I didn't set shockwave speed and duration, then maybe it wouldn't have to go into infinite loop.
 
Last edited by a moderator:

Dr Super Good

Spell Reviewer
Level 64
Joined
Jan 18, 2005
Messages
27,241
Floating Text - Create floating text that reads (String((Integer(Calc_Damage)))) above Shot_Player with Z offset 0.00, using font size 10.00, color (100.00%, 10.00%, 10.00%), and 0.00% transparency
Generates a string that depending on damage ranges might be highly unique.

Each unique string leaks, there is no way to remove strings from WC3 memory however if the same string is generated again it will not make a new string (it recycles existing strings).

With enough string leaks it is possible for the game to perform so badly that any string creation will cause many frames to be dropped.
 

sentrywiz

S

sentrywiz

Generates a string that depending on damage ranges might be highly unique.

Each unique string leaks, there is no way to remove strings from WC3 memory however if the same string is generated again it will not make a new string (it recycles existing strings).

With enough string leaks it is possible for the game to perform so badly that any string creation will cause many frames to be dropped.

I've been using strings for a long time and I've never seen them lag as badly as using this Shockwave based missile spell did.

While your tip is really good, its not the bad guy here.

Still what do you suggest doing IF there is a case where strings might lag?
 

Dr Super Good

Spell Reviewer
Level 64
Joined
Jan 18, 2005
Messages
27,241
Still what do you suggest doing IF there is a case where strings might lag?
Use fewer unique strings so the issue is less of a problem. Or migrate to SC2 where this is not a problem as there is no fix for WC3.

If strings are not the cause, my only suggestion is to migrate to pure JASS (which allows more optimized code) and to disable parts of the ability until you find the part that is demanding.
 

sentrywiz

S

sentrywiz

Use fewer unique strings so the issue is less of a problem. Or migrate to SC2 where this is not a problem as there is no fix for WC3.

If strings are not the cause, my only suggestion is to migrate to pure JASS (which allows more optimized code) and to disable parts of the ability until you find the part that is demanding.

I wanted to learn JASS and I have started. But I got lazy and started to skip it again.

SC2 isn't my cup of tea, its kind of dead even though it has a superior editor which allows much more. And I am more familiar with WC3 editor
 

Dr Super Good

Spell Reviewer
Level 64
Joined
Jan 18, 2005
Messages
27,241
Just to make sure. Each player can only have 1 shot at any given time (the trigger points towards that being the case)?

I would suggest dummy recycling since personal tests showed a possible leak with units due to a WC3 internal bug (they allocate memory that never gets properly freed) however some people claim it gets garbage collected from time to time. In any case it would be more efficient.
 

sentrywiz

S

sentrywiz

Just to make sure. Each player can only have 1 shot at any given time (the trigger points towards that being the case)?

I would suggest dummy recycling since personal tests showed a possible leak with units due to a WC3 internal bug (they allocate memory that never gets properly freed) however some people claim it gets garbage collected from time to time. In any case it would be more efficient.

Well, you could have up to 2 knifes since it has 0.75 cooldown and with range increase it would travel long enough for you to fire another but that's the best case scenario.

I'm not familiar with dummy recycling, except putting dummies into an array, then cleaning that array once dummies are destroyed. Is that what you are implying here?
 

Dr Super Good

Spell Reviewer
Level 64
Joined
Jan 18, 2005
Messages
27,241
Well, you could have up to 2 knifes since it has 0.75 cooldown and with range increase it would travel long enough for you to fire another but that's the best case scenario.
Then the code will not cope with that since and will probably bug badly. I advise migrating to a full instance based system (many players can have many instances) as opposed to per player instance (many players have 1 instance).

I'm not familiar with dummy recycling, except putting dummies into an array, then cleaning that array once dummies are destroyed. Is that what you are implying here?
No, much like timer and group recycling the idea is you store them in a stack (array) and then re-use them between instances (pop top of stack, only make new if stack is empty). You then move the dummy, change ownership etc. The result is a dummy unit which has not leaked and will be re-used between ability casts.

I took another look through your code and do not see you removing the projectile from the persistent group before you remove the unit. As such you have a group contents leak which is why performance is degrading over time. Each unit in the group decreases group operation efficiency and also adds an extra iteration for "for group" operations in the form of picked unit returning null. This obviously will cause unexpected behaviour in your code which may also result in leaks as well as causing the ability execution time to depend on number of times cast. Since the group still keeps a reference to the unit there is also a handle leak.

The solution is to simply remove the projectiles from the persistent group with the appropriate group operator before you remove the unit. I am fully confident that you will see a considerable performance improvement after this.

An alternative is to add code to catch the null picked units (doing nothing if null) and then periodically flush the group with the appropriate clear group operator. This is less efficient than the above solution so I would advise against it.
 

sentrywiz

S

sentrywiz

Then the code will not cope with that since and will probably bug badly. I advise migrating to a full instance based system (many players can have many instances) as opposed to per player instance (many players have 1 instance).


No, much like timer and group recycling the idea is you store them in a stack (array) and then re-use them between instances (pop top of stack, only make new if stack is empty). You then move the dummy, change ownership etc. The result is a dummy unit which has not leaked and will be re-used between ability casts.

I took another look through your code and do not see you removing the projectile from the persistent group before you remove the unit. As such you have a group contents leak which is why performance is degrading over time. Each unit in the group decreases group operation efficiency and also adds an extra iteration for "for group" operations in the form of picked unit returning null. This obviously will cause unexpected behaviour in your code which may also result in leaks as well as causing the ability execution time to depend on number of times cast. Since the group still keeps a reference to the unit there is also a handle leak.

The solution is to simply remove the projectiles from the persistent group with the appropriate group operator before you remove the unit. I am fully confident that you will see a considerable performance improvement after this.

An alternative is to add code to catch the null picked units (doing nothing if null) and then periodically flush the group with the appropriate clear group operator. This is less efficient than the above solution so I would advise against it.

The projectile system is MUI. I don't see this being a problem.

I see, so dummy recycling is basically a smart way of using dummies. Pretty cool. I've used something like this before for casting spells, like making one invisible dummy per player that will cast spells for that player like hit chance item effects, spells etc.

But your solution involves a stack, I didn't make that. But I believe its easy to integrate with a few if/else and an array.

Can you point out to specifics in the triggers? I understand what you're saying but I'm flying blind because I re-use most of the same groups. Do you mean remove the projectile from the group before removing it from the game?
 

Dr Super Good

Spell Reviewer
Level 64
Joined
Jan 18, 2005
Messages
27,241
The projectile system is MUI. I don't see this being a problem.
Not according to the code in the main post. Specifically on unit removal...

Set Shot_Travel_Range[(Player number of (Owner of (Picked unit)))] = 0.00
Set Total_Shots = (Total_Shots - 1)
This will mean that any other projectiles owned by the player which are currently flying will now be known as just launched so it will travel further than intended. Also since MPI variables are used for target point, casting again will deflect existing missiles mid-flight. That is if the code in the starting post has not been altered (apologies if you did change it to some instance based system since then).

But your solution involves a stack, I didn't make that. But I believe its easy to integrate with a few if/else and an array.
Yes that is what is nice about stacks. Even better is you can bundle it as a single function call to avoid procedural coupling. Basically increment pointer (array index) and store to push onto the stack. When popping from stack get and then decrement pointer. If pointer is already 0 or -1 (depending on implementation) then make a new unit. You may need to move the units out of bounds when pushing to avoid an appearance in the mini-map.

Do you mean remove the projectile from the group before removing it from the game?
Yes, I mean that exactly. Removing the unit will not remove it from all groups it was in. Instead an invalid handle remains in those groups that will return "null" during "for group" loops. This prevents the unit's handle index from being recycled (handle leak) and also degrades the performance of the group (groups use a lot of operations with execution complexity not order 1). Since there is no valid handle to reference the unit the only way to remove it from the group is to clear the group completely with appropriate native call. For this reason it is best to remove the unit from the group before you remove it from the game while it still exists.
 

sentrywiz

S

sentrywiz

Yes, I mean that exactly. Removing the unit will not remove it from all groups it was in. Instead an invalid handle remains in those groups that will return "null" during "for group" loops. This prevents the unit's handle index from being recycled (handle leak) and also degrades the performance of the group (groups use a lot of operations with execution complexity not order 1). Since there is no valid handle to reference the unit the only way to remove it from the group is to clear the group completely with appropriate native call. For this reason it is best to remove the unit from the group before you remove it from the game while it still exists.

Here is an example from the code as it is right now, from the Move Ball trigger for the staff:

  • Move Ball
    • Set staff_range_ctr[index_staff] = 0
    • Set total_staff_balls = (total_staff_balls - 1)
    • Unit - Remove moving_ball from the game
    • Unit Group - Remove moving_ball from staff_group
You were right, I remove the unit from the game before I remove it from the group. And from what you're saying it leaves a null pointer that can only be removed via clearing the group, which I never do.

Many thanks I will remember it from now on.

Not according to the code in the main post. Specifically on unit removal...


This will mean that any other projectiles owned by the player which are currently flying will now be known as just launched so it will travel further than intended. Also since MPI variables are used for target point, casting again will deflect existing missiles mid-flight. That is if the code in the starting post has not been altered (apologies if you did change it to some instance based system since then).

I see your point on the projectiles. I do use range variable per weapon for players that determine how far projectile goes. But I still think this isn't a problem, because the cooldown of knife is long enough that even a knife with extended range cannot top it.

I know its bad practice, but only in this case its okay. But say I want to change that, how would I do that? Would I have to add a variable to a stack for every unit made?

Yes that is what is nice about stacks. Even better is you can bundle it as a single function call to avoid procedural coupling. Basically increment pointer (array index) and store to push onto the stack. When popping from stack get and then decrement pointer. If pointer is already 0 or -1 (depending on implementation) then make a new unit. You may need to move the units out of bounds when pushing to avoid an appearance in the mini-map.

I'd love to see an example. I think I can do it on my own, but if you can show me how its done "properly" then even better for me.
 
Status
Not open for further replies.
Top