Any way of making this trigger more effective?

[terranempire]

So, I am pretty sure that this trigger is free of leaks (but please say if it does actually leak), but, is there a way to make it easier, faster and less of a pain to make? Because It's supposed to be for all players and about 9 different objects in total for each player that is supposed to give money.

• income
  • Events
    • Time - Every 60.00 seconds of game time
  • Conditions
  • Actions
    • Sound - Play GoodJob <gen>
    • Set TempGroup = (Units owned by Player 1 (Red) matching ((((Matching unit) is alive) Equal to True) and ((Unit-type of (Matching unit)) Equal to Territory)))
    • Player - Add (Number of units in TempGroup) to Player 1 (Red) Current gold
    • Custom script: call DestroyGroup(udg_TempGroup)
    • Set TempGroup = (Units owned by Player 1 (Red) matching ((((Matching unit) is alive) Equal to True) and ((Unit-type of (Matching unit)) Equal to Factory)))
    • Player - Add (Number of units in TempGroup) to Player 1 (Red) Current gold
    • Custom script: call DestroyGroup(udg_TempGroup)

 

[Berb]

This is about as optimized as you can get it, in GUI. Of course this is relatively simple code in the first place.

 

[terranempire]

So, there is no way that I could change the add part to owner of units in tempgroup or something or?
Or would it work if I made it add to the owner of random unit from tempgroup?

 

[Berb]

Well you could do the counting yourself and save a function call. To you would have an integer variable set to 0 and then for each member in the group you would add 1 to that variable.

Come to think of it you could put an "or" condition in your group enumeration so that units that are alive, of type territory or of type factory. This would save you enumerating similar units twice. In combination with the manual counting that I showed you it would be as optimal as you can get.

• Untitled Trigger 001
  • Events
  • Conditions
  • Actions
    • Set Counter = 0
    • Set TempGroup = (Units owned by Player 1 (Red) matching ((((Matching unit) is alive) Equal to True) and (((Unit-type of (Matching unit)) Equal to Footman) or ((Unit-type of (Matching unit)) Equal to Rifleman))))
    • Unit Group - Pick every unit in TempGroup and do (Actions)
      • Loop - Actions
        • Set Counter = (Counter + 1)
    • Player - Set Player 1 (Red) Current gold to ((Player 1 (Red) Current gold) + Counter)
    • Custom script: call DestroyGroup(udg_TempGroup)

Also, are you doing this for every player? You could optimize that as well.

 

[terranempire]

Well you could do the counting yourself and save a function call. To you would have an integer variable set to 0 and then for each member in the group you would add 1 to that variable.

Come to think of it you could put an "or" condition in your group enumeration so that units that are alive, of type territory or of type factory. This would save you enumerating similar units twice. In combination with the manual counting that I showed you it would be as optimal as you can get.

Also, are you doing this for every player? You could optimize that as well.

Yeah, this is going to be for all players, well 11 then but almost the same thing.

Well, that would be quite a pain to do I think, and it may not work as some of my other triggers has a tendency to do, even though they should work.
Though could you please say if it would work if I did it like this?

• income
  • Events
    • Time - Every 60.00 seconds of game time
  • Conditions
  • Actions
    • Sound - Play GoodJob <gen>
    • Set TempGroup = (Units owned by Player 1 (Red) matching ((((Matching unit) is alive) Equal to True) and ((Unit-type of (Matching unit)) Equal to Territory)))
    • Player - Add (Number of units in TempGroup) to (Owner of (Random unit from TempGroup)) Current gold
    • Custom script: call DestroyGroup(udg_TempGroup)
    • Set TempGroup = (Units owned by Player 1 (Red) matching ((((Matching unit) is alive) Equal to True) and ((Unit-type of (Matching unit)) Equal to Factory)))
    • Player - Add (Number of units in TempGroup) to (Owner of (Random unit from TempGroup)) Current gold
    • Custom script: call DestroyGroup(udg_TempGroup)

 

[Berb]

I updated my post with some sample code. For the players, you could use something like this:

• Untitled Trigger 001
  • Events
  • Conditions
  • Actions
    • For each (Integer A) from 1 to 11, do (Actions)
      • Loop - Actions
        • Set Counter = 0
        • Set TempGroup = (Units owned by (Player((Integer A))) matching ((((Matching unit) is alive) Equal to True) and (((Unit-type of (Matching unit)) Equal to Footman) or ((Unit-type of (Matching unit)) Equal to Rifleman))))
        • Unit Group - Pick every unit in TempGroup and do (Actions)
          • Loop - Actions
            • Set Counter = (Counter + 1)
        • Player - Set (Player((Integer A))) Current gold to (((Player((Integer A))) Current gold) + Counter)
        • Custom script: call DestroyGroup(udg_TempGroup)

 

[terranempire]

I updated my post with some sample code. For the players, you could use something like this:

• Untitled Trigger 001
  • Events
  • Conditions
  • Actions
    • For each (Integer A) from 1 to 12, do (Actions)
      • Loop - Actions
        • Set Counter = 0
        • Set TempGroup = (Units owned by (Player((Integer A))) matching ((((Matching unit) is alive) Equal to True) and (((Unit-type of (Matching unit)) Equal to Footman) or ((Unit-type of (Matching unit)) Equal to Rifleman))))
        • Unit Group - Pick every unit in TempGroup and do (Actions)
          • Loop - Actions
            • Set Counter = (Counter + 1)
        • Player - Set (Player((Integer A))) Current gold to (((Player((Integer A))) Current gold) + Counter)
        • Custom script: call DestroyGroup(udg_TempGroup)

Kinda like this?

• income
  • Events
    • Time - Every 60.00 seconds of game time
  • Conditions
  • Actions
    • Sound - Play GoodJob <gen>
    • For each (Integer A) from 1 to 11, do (Actions)
      • Loop - Actions
        • Set TempGroup = (Units owned by (Player((Integer A))) matching ((((Matching unit) is alive) Equal to True) and ((Unit-type of (Matching unit)) Equal to Territory)))
        • Player - Add (Number of units in TempGroup) to (Player((Integer A))) Current gold
        • Custom script: call DestroyGroup(udg_TempGroup)
        • Set TempGroup = (Units owned by (Player((Integer A))) matching ((((Matching unit) is alive) Equal to True) and ((Unit-type of (Matching unit)) Equal to Factory)))
        • Player - Add (Number of units in TempGroup) to (Player((Integer A))) Current gold
        • Custom script: call DestroyGroup(udg_TempGroup)

I don't want the line of code to get too long so I think it's best to stick with making them separate, considering that there'll be 9 different objects.
But, I don't understand what that counter is for, I should mention that some objects will give more gold than others (for which I plan to use a units in tempgroup multiplied by a number).

 

[Berb]

Okay, well you simply asked if this code could be optimized. The counter is for counting the units in the group (instead of using a function-call to do it). In order to "weight" different unit types using my optimized method, do:

• Untitled Trigger 001
  • Events
  • Conditions
  • Actions
    • For each (Integer A) from 1 to 11, do (Actions)
      • Loop - Actions
        • Set UnitCounter[0] = 0
        • Set UnitCounter[1] = 0
        • Set TempGroup = (Units owned by (Player((Integer A))) matching ((((Matching unit) is alive) Equal to True) and (((Unit-type of (Matching unit)) Equal to Footman) or ((Unit-type of (Matching unit)) Equal to Rifleman))))
        • 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
                • (Unit-type of (Picked unit)) Equal to Footman
              • Then - Actions
                • Set UnitCounter[0] = (UnitCounter[0] + 1)
              • Else - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (Unit-type of (Picked unit)) Equal to Rifleman
              • Then - Actions
                • Set UnitCounter[1] = (UnitCounter[1] + 1)
              • Else - Actions
        • Player - Set (Player((Integer A))) Current gold to (((Player((Integer A))) Current gold) + ((UnitCounter[0] x 2) + (UnitCounter[1] x 3)))
        • Custom script: call DestroyGroup(udg_TempGroup)

 

[terranempire]

Okay, well you simply asked if this code could be optimized. The counter is for counting the units in the group (instead of using a function-call to do it). In order to "weight" different unit types using my optimized method, do:

• Untitled Trigger 001
  • Events
  • Conditions
  • Actions
    • For each (Integer A) from 1 to 11, do (Actions)
      • Loop - Actions
        • Set UnitCounter[0] = 0
        • Set UnitCounter[1] = 0
        • Set TempGroup = (Units owned by (Player((Integer A))) matching ((((Matching unit) is alive) Equal to True) and (((Unit-type of (Matching unit)) Equal to Footman) or ((Unit-type of (Matching unit)) Equal to Rifleman))))
        • 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
                • (Unit-type of (Picked unit)) Equal to Footman
              • Then - Actions
                • Set UnitCounter[0] = (UnitCounter[0] + 1)
              • Else - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (Unit-type of (Picked unit)) Equal to Rifleman
              • Then - Actions
                • Set UnitCounter[1] = (UnitCounter[1] + 1)
              • Else - Actions
        • Player - Set (Player((Integer A))) Current gold to (((Player((Integer A))) Current gold) + ((UnitCounter[0] x 2) + (UnitCounter[1] x 3)))
        • Custom script: call DestroyGroup(udg_TempGroup)

Well, I think I'll stick with the one I posted above yours, but thanks for the help (it made me actually realise what the integer A thing could be used for.