Pinging the map & leaks

[Shooterman56]

Is this the correct way to select a point on the map and ping it without leaking?


If (All Conditions are True) then do (Then Actions) else do (Else Actions)
If - Conditions
MapRand Equal to 1
Then - Actions
Cinematic - Ping minimap for (All players matching ((Owner of (Triggering unit)) Equal to TriggeringPlayer)) at (Position of Sea Giant Behemoth 0358 <gen>) for 5.00 seconds
Game - Display to (All players matching ((Owner of (Triggering unit)) Equal to TriggeringPlayer)) for 10.00 seconds the text: Aye laddie we've se...
Else - Actions
If (All Conditions are True) then do (Then Actions) else do (Else Actions)
If - Conditions
MapRand Equal to 2
Then - Actions
Cinematic - Ping minimap for (All players matching ((Owner of (Triggering unit)) Equal to TriggeringPlayer)) at (Position of Cursed Oathbreaker 0441 <gen>) for 5.00 seconds
Game - Display to (All players matching ((Owner of (Triggering unit)) Equal to TriggeringPlayer)) for 10.00 seconds the text: Aye laddie we've se...
Else - Actions
If (All Conditions are True) then do (Then Actions) else do (Else Actions)
If - Conditions
MapRand Equal to 3
Then - Actions
Cinematic - Ping minimap for (All players matching ((Owner of (Triggering unit)) Equal to TriggeringPlayer)) at (Position of Enraged Druid 0547 <gen>) for 5.00 seconds
Game - Display to (All players matching ((Owner of (Triggering unit)) Equal to TriggeringPlayer)) for 10.00 seconds the text: Aye laddie we've se...
Else - Actions
If (All Conditions are True) then do (Then Actions) else do (Else Actions)
If - Conditions
MapRand Equal to 4
Then - Actions
Cinematic - Ping minimap for (All players matching ((Owner of (Triggering unit)) Equal to TriggeringPlayer)) at (Position of Overlord 0434 <gen>) for 5.00 seconds
Game - Display to (All players matching ((Owner of (Triggering unit)) Equal to TriggeringPlayer)) for 10.00 seconds the text: Aye laddie we've se...
Else - Actions
If (All Conditions are True) then do (Then Actions) else do (Else Actions)
If - Conditions
MapRand Equal to 5
Then - Actions
Cinematic - Ping minimap for (All players matching ((Owner of (Triggering unit)) Equal to TriggeringPlayer)) at (Position of Doom Guard 0452 <gen>) for 5.00 seconds
Game - Display to (All players matching ((Owner of (Triggering unit)) Equal to TriggeringPlayer)) for 10.00 seconds the text: Aye laddie we've se...
Else - Actions

 

[Dr Super Good]

It leaks player groups (force) which are created but not destroyed with the DestroyForce function. It also leaks point (location) which are created but not destroyed with the RemoveLocation function.

 

[Shooterman56]

Stables
Events
Player - Player 1 (Red) types a chat message containing -stables as An exact match
Conditions
Actions
Set tempPG = (Player group((Triggering player)))
Set pingP = (Position of Stable Master 0296 <gen>)
Cinematic - Ping minimap for tempPG at pingP for 1.00 seconds
Custom script: call RemoveLocation (udg_pingP)
Set pingP = (Position of Stable Master 0315 <gen>)
Cinematic - Ping minimap for tempPG at pingP for 1.00 seconds
Custom script: call RemoveLocation (udg_pingP)
Set pingP = (Position of Stable Master 0306 <gen>)
Cinematic - Ping minimap for tempPG at pingP for 1.00 seconds
Custom script: call RemoveLocation (udg_pingP)
Set pingP = (Position of Stable Master 0295 <gen>)
Cinematic - Ping minimap for tempPG at pingP for 1.00 seconds
Custom script: call RemoveLocation (udg_pingP)
Custom script: call DestroyForce ( udg_tempPG )



Does this format look up to par?

 

[apsyll]

first you can use the [*trigger] [*/trigger] tags without the stars so it is easier to read for us.
second you leak in point and forces.
I recommend doing something like:

Spoiler

• Actions
  • -------- Shadowing the global variable to a local one so it just exists in this trigger --------
  • Custom script: local location udg_TempPoint = null
  • -------- Store the triggering player in a playergroup variable to remove the leaks later on --------
  • Set TempForce = (Player group((Owner of (Triggering unit))))
  • -------- Check your conditions and set the variables properly --------
  • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    • If - Conditions
      • MapRand Equal to 1
    • Then - Actions
      • Set TempPoint = (Position of UnitA)
      • Game - Display to TempForce the text: Text A
    • Else - Actions
  • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    • If - Conditions
      • MapRand Equal to 2
    • Then - Actions
      • Set TempPoint = (Position of UnitB)
      • Game - Display to TempForce the text: Text B
    • Else - Actions
  • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    • If - Conditions
      • MapRand Equal to 3
    • Then - Actions
      • Set TempPoint = (Position of UnitC)
      • Game - Display to TempForce the text: Text C
    • Else - Actions
  • -------- you can move the pinging out because it is something you do in every case --------
  • Cinematic - Ping minimap for TempForce at TempPoint for 5.00 seconds
  • -------- Remove the leaks --------
  • Custom script: call RemoveLocation(udg_TempPoint)
  • Custom script: call DestroyForce(udg_TempForce)

alternative:

Spoiler

• Actions
  • -------- Shadowing the global variable to a local one so it just exists in this trigger --------
  • Custom script: local location udg_TempPoint = null
  • -------- Store the triggering player in a playergroup variable to remove the leaks later on --------
  • Set TempForce = (Player group((Owner of (Triggering unit))))
  • -------- Check your conditions and set the variables properly --------
  • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    • If - Conditions
      • MapRand Equal to 1
    • Then - Actions
      • Set TempPoint = (Position of UnitA)
      • Game - Display to TempForce the text: Text A
      • Cinematic - Ping minimap for TempForce at TempPoint for 5.00 seconds
      • -------- Remove the leaks --------
      • Custom script: call RemoveLocation(udg_TempPoint)
      • Custom script: call DestroyForce(udg_TempForce)
      • -------- you found already the right case, so there is no need to check further --------
      • -------- this line finishes the trigger so the other cases won't be checked --------
      • Skip remaining actions
    • Else - Actions

 

[Rahko]

Also, depends on your event, but I'd suggest to consider if there's a way to use loops. That way you don't have to make an action for every possible unit. Though, as I said, it depends on your event if this suitable. You could for example save the unit into unit variable with array, so you could refer to the unit array with the loop. Using loops has also the advantage that you simply add more units later just by increasing the loop max integer and adding the new units to the Unit[X] variable. Something like this:

• Actions
  • For each (Integer A) from 1 to [Your amount of MapRands], do (Actions)
    • Loop - Actions
      • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        • If - Conditions
          • MapRand Equal to (Integer A)
        • Then - Actions
          • Set TempForce = (Player group((Owner of (Triggering unit))))
          • Set TempPoint = (Position of Unit[(Integer A)])
          • Game - Display to TempForce the text: [Whatever you want]
          • Cinematic - Ping minimap for TempForce at TempPoint for 5.00 seconds
          • Custom script: call RemoveLocation(udg_TempPoint)
          • Custom script: call DestroyForce(udg_TempForce)
        • Else - Actions

And this way you have to set the unit variables in another trigger:

• Actions
  • Set Unit[1] = Sea Giant Behemoth 0358 <gen>
  • Set Unit[2] = Cursed Oathbreaker 0441 <gen>
  • Set Unit[3] = Enraged Druid 0547 <gen>
  • ... And so on ...

 

[Wrda]

• Actions
  • -------- Shadowing the global variable to a local one so it just exists in this trigger --------
  • Custom script: local location udg_TempPoint = null[/QUOTE]

What's even the point of shadowing the global variable here? The trigger is instantaneous, it is only benefitial to shadow global variables if the trigger has waits... so this line is pretty much doing nothing.
@Sabe , while that is a good advice, it shouldn't apply in this context since it's only 3 units.

 

[Rahko]

@Sabe , while that is a good advice, it shouldn't apply in this context since it's only 3 units.

Well, the OP had 5 units in it, and as I said above, I don't know the event, if it's suitable, but after all it's really a matter of opinion I guess. I'd use loop for even 3 units, but yeah. I think it just looks a bit more "professional" to use less code than to write 5 times the same thing with one or two things changed. But, whatever works.