• 🏆 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] Does this leak?

Status
Not open for further replies.
Level 8
Joined
Oct 2, 2013
Messages
288
  • Trigger
    • Events
      • Unit - A unit Dies
    • Conditions
    • Actions
      • Set TempPG = (All allies of (Owner of (Killing unit)))
      • Player Group - Pick every player in KillingBlowAllies and do (Actions)
        • Loop - Actions
          • Set TempUG = (Units owned by (Picked player) matching (((Matching unit) is A Hero) Equal to True))
          • Unit Group - Pick every unit in U1GTemp and do (Actions)
            • Loop - Actions
              • Hero - Add 25 experience to (Picked unit), Show level-up graphics
          • Custom script: call DestroyGroup(udg_TempUG)
      • Custom script: call DestroyForce(udg_TempPG)
 

Dr Super Good

Spell Reviewer
Level 64
Joined
Jan 18, 2005
Messages
27,201
Why not just pick every (unit owned by (allies of KillingBlowAllies))? This will save you running a loop inside a loop.
Because the group returned takes a player not a force.

The largest performance improvement would come from caching the hero results of the players in a group and using their player id as an index in a group array to retrieve it. This would eliminate searching for heroes each time. However unless performance is noticeably a problem then the above is fine.

Does this leak?
No it does not leak. At least it looks that way.
 
Level 8
Joined
Oct 2, 2013
Messages
288
Ups! I made a mistake in rewritting the variables before posting. This is how the trigger really looks:

  • Trigger
    • Events
      • Unit - A unit Dies
    • Conditions
    • Actions
      • Set TempPG = (All allies of (Owner of (Killing unit)))
      • Player Group - Pick every player in TempPG and do (Actions)
        • Loop - Actions
          • Set TempUG = (Units owned by (Picked player) matching (((Matching unit) is A Hero) Equal to True))
          • Unit Group - Pick every unit in U1GTemp and do (Actions)
            • Loop - Actions
              • Hero - Add 25 experience to (Picked unit), Show level-up graphics
          • Custom script: call DestroyGroup(udg_TempUG)
      • Custom script: call DestroyForce(udg_TempPG)
Well I'm happy it's not leaking. Thank you all for checking :)
 
Status
Not open for further replies.
Top