Need help in patching a small leak

terranempire · Mar 25, 2010

So, I need help in patching this trigger, because I am fairly certain that it leaks.
It might be easy, but I'm pretty lousy at spotting leaks.

Limit check
- Events
  - Time - Every 1.00 seconds of game time
- Conditions
- Actions
  - Player Group - Pick every player in (All players) and do (Actions)
    - Loop - Actions
      - Custom script: set bj_wantDestroyGroup = true
      - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        ((Picked player) Current gold) Greater than (MoneyLimit + ((Number of living Storage units owned by (Picked player)) x 50))
        
        Then - Actions
        
        Player - Set (Picked player) Current gold to (MoneyLimit + ((Number of living Storage units owned by (Picked player)) x 50))
        
        Else - Actions
  - Player Group - Pick every player in (All players) and do (Actions)
    - Loop - Actions
      - Custom script: set bj_wantDestroyGroup = true
      - Set Limit = (((((Number of living City units owned by (Picked player)) + (Number of living R units owned by (Picked player))) + ((Number of living N units owned by (Picked player)) + (Number of living M (Finnish) units owned by (Picked player))
      - Custom script: set bj_wantDestroyGroup = true
      - Set Limit = (Limit + ((((Number of living B units owned by (Picked player)) + (Number of living E units owned by (Picked player))) + (Number of living H units owned by (Picked player))) x 8))
      - Custom script: set bj_wantDestroyGroup = true
      - Set Limit = (Limit + (((Number of living A units owned by (Picked player)) + (Number of living S units owned by (Picked player))) x 2))
      - Custom script: set bj_wantDestroyGroup = true
      - Set Limit = (Limit + (((Number of living C units owned by (Picked player)) + (Number of living F units owned by (Picked player))) x 3))
      - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        ((Picked player) Current lumber) Greater than (Limit + 4)
        
        Then - Actions
        
        Player - Set (Picked player) Current lumber to (Limit + 4)
        
        Else - Actions

Pharaoh_ · Mar 25, 2010

Nope, it doesn't leak.

Justify · Mar 25, 2010

Afaik "Number of living ..." is "CountUnitsInGroup" and the group Blizzard internaly creates leaks.

Pharaoh_ · Mar 25, 2010

Well, Justify, I thought of that myself, but it's not the Number of units in Unit Group, it's a preset function, that counts the living units, maybe there is also an internal garbage collector for that?
That function could also be displayed as:

Custom script: set bj_wantDestroyGroup = true
Unit Group - Pick every units in (Units owned by (Picked player)) and do (Actions)
- Loop - Actions
  - If (All conditions are true) then do (Actions) else do (Actions)
    - If - Conditions
      - ((Picked unit) is alive) Equal to True
    - Then - Actions
      - ...
    - Else - Actions

which is replaced with the single line:

(Number of living Resource Storage units owned by (Picked player))

I don't know why they did implement such a function, when it can also be done other way. Maybe it cleans the leak itself?

Bribe · Mar 25, 2010

Actually, it only leaks a unit handle, a boolean handle and a group handle that aren't set to null.

JASS:

//===========================================================================
function CountUnitsInGroupEnum takes nothing returns nothing
    set bj_groupCountUnits = bj_groupCountUnits + 1
endfunction
//===========================================================================
function CountUnitsInGroup takes group g returns integer
    // If the user wants the group destroyed, remember that fact and clear
    // the flag, in case it is used again in the callback.
    local boolean wantDestroy = bj_wantDestroyGroup
    set bj_wantDestroyGroup = false
    set bj_groupCountUnits = 0
    call ForGroup(g, function CountUnitsInGroupEnum)
    // If the user wants the group destroyed, do so now.
    if (wantDestroy) then
        call DestroyGroup(g)
    endif
    return bj_groupCountUnits
endfunction
//===========================================================================
function LivingPlayerUnitsOfTypeIdFilter takes nothing returns boolean
    local unit filterUnit = GetFilterUnit()
    return IsUnitAliveBJ(filterUnit) and GetUnitTypeId(filterUnit) == bj_livingPlayerUnitsTypeId
endfunction
//===========================================================================
function CountLivingPlayerUnitsOfTypeId takes integer unitId, player whichPlayer returns integer
    local group g
    local integer matchedCount
    set g = CreateGroup()
    set bj_livingPlayerUnitsTypeId = unitId
    call GroupEnumUnitsOfPlayer(g, whichPlayer, filterLivingPlayerUnitsOfTypeId)
    set matchedCount = CountUnitsInGroup(g)
    call DestroyGroup(g)
    return matchedCount
endfunction

PurgeandFire · Mar 25, 2010

Bribe said:
Actually, it only leaks a unit handle, a boolean handle and a group handle that aren't set to null.

Booleans aren't handles, they don't need nulling. =)

lelyanra · Mar 27, 2010

Bribe said:

Actually, it only leaks a unit handle, a boolean handle and a group handle that aren't set to null.

JASS:

//===========================================================================
function CountUnitsInGroupEnum takes nothing returns nothing
    set bj_groupCountUnits = bj_groupCountUnits + 1
endfunction
//===========================================================================
function CountUnitsInGroup takes group g returns integer
    // If the user wants the group destroyed, remember that fact and clear
    // the flag, in case it is used again in the callback.
    local boolean wantDestroy = bj_wantDestroyGroup
    set bj_wantDestroyGroup = false
    set bj_groupCountUnits = 0
    call ForGroup(g, function CountUnitsInGroupEnum)
    // If the user wants the group destroyed, do so now.
    if (wantDestroy) then
        call DestroyGroup(g)
    endif
    return bj_groupCountUnits
endfunction
//===========================================================================
function LivingPlayerUnitsOfTypeIdFilter takes nothing returns boolean
    local unit filterUnit = GetFilterUnit()
    return IsUnitAliveBJ(filterUnit) and GetUnitTypeId(filterUnit) == bj_livingPlayerUnitsTypeId
endfunction
//===========================================================================
function CountLivingPlayerUnitsOfTypeId takes integer unitId, player whichPlayer returns integer
    local group g
    local integer matchedCount
    set g = CreateGroup()
    set bj_livingPlayerUnitsTypeId = unitId
    call GroupEnumUnitsOfPlayer(g, whichPlayer, filterLivingPlayerUnitsOfTypeId)
    set matchedCount = CountUnitsInGroup(g)
    call DestroyGroup(g)
    return matchedCount
endfunction

wow, where do i get that information?

Bribe · Mar 27, 2010

it's one of the three key archives that blizzard uses for map functions.

That one is an excerpt from blizzard.j

The most reliable (because it's the best and the fastest) is common.j

The AI functions are found in common.ai

You can download an MPQ extractor (I use RMPQ) and open up one of the mpq files in the War3 main folder to see the commands listed inside of them. It is really useful to have a copy of those in a text file for quick reference, or just download Jass NewGen Pack which has an auto-referencing library that holds the commands listed in those files. Pretty sweet.

terranempire · Mar 28, 2010

Uhm, I edited the first post with an updated version of the trigger, using some of the suggestions given (and added some other functions) but, does it still leak? Oh and is there any way to make it more effective without making it one long row of text?

Bribe · Mar 28, 2010

Well, you are re-writing Limit each time you do that, so you're not counting the total of all of that, you're only going to get the last group count + group count result.

I would recommend just adding +1 to Limit each time one of those units is created, so you don't have to count over and over.

Edit: and, of course, minus -1 when one of them dies.

terranempire · Mar 28, 2010

Bribe said:
Well, you are re-writing Limit each time you do that, so you're not counting the total of all of that, you're only going to get the last group count + group count result.

I would recommend just adding +1 to Limit each time one of those units is created, so you don't have to count over and over.

Edit: and, of course, minus -1 when one of them dies.

Er, doesn't it add Limit (which is the already stored number, in the action before it) + what it counts? It should.

Problem is, it's making the game laggy, is it because of leaks or because of it being run too often or something else?

terranempire · Apr 17, 2010

Okay, so this is the version that I'm currently using in one of my maps:

Limit check
- Events
  - Time - Every 4.00 seconds of game time
- Conditions
- Actions
  - Player Group - Pick every player in (All players) and do (Actions)
    - Loop - Actions
      - Custom script: set bj_wantDestroyGroup = true
      - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        ((Picked player) Current gold) Greater than (MoneyLimit + ((Number of living Storage units owned by (Picked player)) x 500))
        
        Then - Actions
        
        Player - Set (Picked player) Current gold to (MoneyLimit + ((Number of living Storage units owned by (Picked player)) x 500))
        
        Else - Actions
  - Player Group - Pick every player in (All players) and do (Actions)
    - Loop - Actions
      - Custom script: set bj_wantDestroyGroup = true
      - Set Limit = (((((Number of living City units owned by (Picked player)) + (Number of living R units owned by (Picked player))) + ((Number of living N units owned by (Picked player)) + (Number of living M (Finnish) units owned by (Picked player))
      - Custom script: set bj_wantDestroyGroup = true
      - Set Limit = (Limit + ((((Number of living B units owned by (Picked player)) + (Number of living E units owned by (Picked player))) + (Number of living H units owned by (Picked player))) x 8))
      - Custom script: set bj_wantDestroyGroup = true
      - Set Limit = (Limit + (((Number of living A units owned by (Picked player)) + (Number of living S units owned by (Picked player))) x 2))
      - Custom script: set bj_wantDestroyGroup = true
      - Set Limit = (Limit + (((Number of living C units owned by (Picked player)) + (Number of living F units owned by (Picked player))) x 3))
      - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        ((Picked player) Current lumber) Greater than (Limit + 4)
        
        Then - Actions
        
        Player - Set (Picked player) Current lumber to (Limit + 4)
        
        Else - Actions

There's just one problem: it causes loads of lag every time it fires.
How do I fix this? Or is it impossible?

Pharaoh_ · Apr 17, 2010

Meh, that looks ugly in the first place; plus, you don't need to pick player groups twice.

Trigger
Events
- Map Initialization
Conditions
Actions
- Set UnitTypes[1] = A
- Set UnitTypes[2] = B
- Set UnitTypes[3] = C
- Set UnitTypes[4] = E
- Set UnitTypes[5] = F
- Set UnitTypes[6] = H
- Set UnitTypes[7] = M
- Set UnitTypes[8] = N
- Set UnitTypes[9] = R
- Set UnitTypes[10] = S

Trigger2
Events
- Time - Every 4.00 seconds of game-time
Conditions
Actions
- Player Group - Pick every player in (All players) and do (Actions)
  - Loop - Actions
    - For each (Integer A) from 1 to 10, do (Actions)
      - Loop - Actions
        
        Set Limit[Player Number of (Picked player)] = (Number of living (UnitTypes[(IntegerA)]) units owned by (Picked player)

Try this. UnitTypes is a Unit-type variable.

terranempire · Apr 17, 2010

Pharaoh_ said:
Meh, that looks ugly in the first place; plus, you don't need to pick player groups twice.

Trigger

Events

Map Initialization

Conditions

Actions

Set UnitTypes[1] = A

Set UnitTypes[2] = B

Set UnitTypes[3] = C

Set UnitTypes[4] = E

Set UnitTypes[5] = F

Set UnitTypes[6] = H

Set UnitTypes[7] = M

Set UnitTypes[8] = N

Set UnitTypes[9] = R

Set UnitTypes[10] = S

Trigger2

Events

Time - Every 4.00 seconds of game-time

Conditions

Actions

Player Group - Pick every player in (All players) and do (Actions)

Loop - Actions

For each (Integer A) from 1 to 10, do (Actions)

Loop - Actions

Set Limit[Player Number of (Picked player)] = (Number of living (UnitTypes[(IntegerA)]) units owned by (Picked player)

Try this. UnitTypes is a Unit-type variable.

So, only the second part is wrong then?
Oh and does the leak-removers contribute anything to the lag?

Edit: but what about the multipliers? they are specific to certain types.

Pharaoh_ · Apr 17, 2010

I honestly don't know about the custom scripts if they have to be there. They make sense to leak, but I wouldn't use them anyway. People in here claimed they leak, so, to be sure and safe, use them.

You didn't tell me about multipliers

Either way, you can use

(Unit-type of (UnitsTypes[(IntegerA)]) Equal to N //Then do Actions - This is a Unit-type comparison

I just replied to something similar here, might wanna take a look at it.
https://www.hiveworkshop.com/threads/gui-click-equip-system.164217/post-1546583

You just have to make 2 lists, like a table in jass (but way less efficient of course) to retrieve the corresponding values.

terranempire · Apr 17, 2010

Uh, I'm sorry but I didn't fully understand your answer.

So, I should check if it's a particular unit type or? Or should I just check if it's, for example: UnitType[1] or for UnitType[3]?

Edit: uh, like this?

Limit check
- Events
  - Time - Every 4.00 seconds of game time
- Conditions
- Actions
  - Player Group - Pick every player in (All players) and do (Actions)
    - Loop - Actions
      - Custom script: set bj_wantDestroyGroup = true
      - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        ((Picked player) Current gold) Greater than (MoneyLimit + ((Number of living Resource Storage units owned by (Picked player)) x 500))
        
        Then - Actions
        
        Player - Set (Picked player) Current gold to (MoneyLimit + ((Number of living Resource Storage units owned by (Picked player)) x 500))
        
        Else - Actions
      - For each (Integer B) from 1 to 11, do (Actions)
        
        Loop - Actions
        
        Custom script: set bj_wantDestroyGroup = true
        
        If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        Or - Any (Conditions) are true
        
        Conditions
        
        UnitTypeFood[(Integer B)] Equal to Airport
        
        UnitTypeFood[(Integer B)] Equal to Tradeport
        
        Then - Actions
        
        Set FoodLimit = (((Number of living UnitTypeFood[(Integer B)] units owned by (Picked player)) x 2) + FoodLimit)
        
        Else - Actions
        
        If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        Or - Any (Conditions) are true
        
        Conditions
        
        UnitTypeFood[(Integer B)] Equal to City
        
        UnitTypeFood[(Integer B)] Equal to City (Finnish)
        
        UnitTypeFood[(Integer B)] Equal to Expanded City
        
        UnitTypeFood[(Integer B)] Equal to Expanded City (Finnish)
        
        Then - Actions
        
        Set FoodLimit = (((Number of living UnitTypeFood[(Integer B)] units owned by (Picked player)) x 5) + FoodLimit)
        
        Else - Actions
        
        If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        Or - Any (Conditions) are true
        
        Conditions
        
        UnitTypeFood[(Integer B)] Equal to Britannia!
        
        UnitTypeFood[(Integer B)] Equal to Capital
        
        UnitTypeFood[(Integer B)] Equal to Expanded Capital
        
        Then - Actions
        
        Set FoodLimit = (((Number of living UnitTypeFood[(Integer B)] units owned by (Picked player)) x 8) + FoodLimit)
        
        Else - Actions
        
        If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        Or - Any (Conditions) are true
        
        Conditions
        
        UnitTypeFood[(Integer B)] Equal to Farm
        
        UnitTypeFood[(Integer B)] Equal to Farm Base
        
        Then - Actions
        
        Set FoodLimit = (((Number of living UnitTypeFood[(Integer B)] units owned by (Picked player)) x 3) + FoodLimit)
        
        Else - Actions
  - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    - If - Conditions
      - ((Picked player) Current lumber) Greater than (FoodLimit + 4)
    - Then - Actions
      - Player - Set (Picked player) Current lumber to (FoodLimit + 4)
      - Game - Display to (All players) for 4.00 seconds the text: YOU HAVE EXCEEDED Y...
    - Else - Actions

Pharaoh_ · Apr 17, 2010

Ok, for your case, you need two types of variables with arrays. One will be Unit-type and the second will be Integer. The unit-types will be the Unit-types you used (A/B/C...) and the Integer variables will contain the "multipliers". So, up to which unit-type is called, you can make your multipliers. The example I posted in the other thread might orient you.

terranempire · Apr 18, 2010

Okay, I should notify the fact that the trigger didn't work.

Edit: This is the current version of the trigger, it doesn't work and it still causes loads of lag.

Limit check
- Events
  - Time - Every 4.00 seconds of game time
- Conditions
- Actions
  - Player Group - Pick every player in (All players) and do (Actions)
    - Loop - Actions
      - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        ((Picked player) Current gold) Greater than (MoneyLimit + ((Number of living Resource Storage units owned by (Picked player)) x 500))
        
        Then - Actions
        
        Custom script: set bj_wantDestroyGroup = true
        
        Player - Set (Picked player) Current gold to (MoneyLimit + ((Number of living Resource Storage units owned by (Picked player)) x 500))
        
        Game - Display to (All players) for 4.00 seconds the text: YOU HAVE EXCEEDED Y...
        
        Else - Actions
      - For each (Integer B) from 1 to 11, do (Actions)
        
        Loop - Actions
        
        If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        Or - Any (Conditions) are true
        
        Conditions
        
        (Integer B) Equal to 3
        
        (Integer B) Equal to 11
        
        Then - Actions
        
        Custom script: set bj_wantDestroyGroup = true
        
        Set FoodLimit = (((Number of living UnitTypeFood[(Integer B)] units owned by (Picked player)) x 2) + FoodLimit)
        
        Else - Actions
        
        If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        Or - Any (Conditions) are true
        
        Conditions
        
        (Integer B) Equal to 4
        
        (Integer B) Equal to 5
        
        (Integer B) Equal to 7
        
        (Integer B) Equal to 8
        
        Then - Actions
        
        Custom script: set bj_wantDestroyGroup = true
        
        Set FoodLimit = (((Number of living UnitTypeFood[(Integer B)] units owned by (Picked player)) x 5) + FoodLimit)
        
        Else - Actions
        
        If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        Or - Any (Conditions) are true
        
        Conditions
        
        (Integer B) Equal to 1
        
        (Integer B) Equal to 2
        
        (Integer B) Equal to 6
        
        Then - Actions
        
        Custom script: set bj_wantDestroyGroup = true
        
        Set FoodLimit = (((Number of living UnitTypeFood[(Integer B)] units owned by (Picked player)) x 8) + FoodLimit)
        
        Else - Actions
        
        If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        Or - Any (Conditions) are true
        
        Conditions
        
        (Integer B) Equal to 9
        
        (Integer B) Equal to 10
        
        Then - Actions
        
        Custom script: set bj_wantDestroyGroup = true
        
        Set FoodLimit = (((Number of living UnitTypeFood[(Integer B)] units owned by (Picked player)) x 3) + FoodLimit)
        
        Else - Actions
  - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    - If - Conditions
      - ((Picked player) Current lumber) Greater than (FoodLimit + 4)
    - Then - Actions
      - Player - Set (Picked player) Current lumber to (FoodLimit + 4)
    - Else - Actions

Edit2: oh and for some reason, it loves stating the message 'YOU HAVE EXCEEDED Y...' when it shouldn't (i.e. when I don't have more gold than the limit).

Edit3: simply removing the other select all players from the original version seems to have fixed the lag problem.

Edit4: oh wait, that was because the food part was outside of the pick all players part.

Need help in patching a small leak

Similar threads