[AI] Does this code leak memory or not?

[Tommi Gustafsson]

Hi!

I am not sure if the following code (from one of my AIs) leaks memory or not:

function AttackAssignment takes nothing returns nothing
    local boolean AttackSuccess = false
    local integer WaitTime1 = 240 + ((3 - difficulty) * 60) //Wait 6/5/4 minutes before the first attack on EASY/NORMAL/HARD
    local integer WaitTime2 = 600 + ((3 - difficulty) * 60) //Wait 12/11/10 minutes before other attacks on EASY/NORMAL/HARD
    local integer currentWaitLimit = 0
    local integer wait_seconds = 0
    local boolean LaunchAttack = false
    local unit base = null
    local integer greatDenCount = 0
    local group g = null
    local integer id
    local unit u = null

    loop
        loop

            //Wait until smg_custom_integer (in common.ai) is 1 and Player 1 has built Great Den
            loop
                set greatDenCount = GetPlayerUnitTypeCount(Player(0), GREAT_DEN)
                exitwhen smg_custom_integer == 1 and greatDenCount > 0
                call Sleep(5)
            endloop

            //Let's find the Great Den
            set g =  CreateGroup()
            call GroupEnumUnitsOfPlayer(g, Player(0), null)
            set u = FirstOfGroup(g)
            loop
                exitwhen u == null
                set id = GetUnitTypeId(u)
                if(id == GREAT_DEN) then
                    set base = u
                endif
                call GroupRemoveUnit(g, u)
                set u = FirstOfGroup(g)
            endloop
            call DestroyGroup(g)

            exitwhen base != null
           
            call DebugText("Base is null")

            call Sleep(5)
        endloop

        call DebugText("Attacking against base!")

        //Let's wait WaitTime seconds before launching an attack
        //We'll do this also before the first attack
        set wait_seconds = 0
        if attack_wave == 1 then
            set currentWaitLimit = WaitTime1
        else
            set currentWaitLimit = WaitTime2
        endif

       
        loop
            if ( HaveMinimumAttackers() and not CaptainRetreating() and not CaptainInCombat(true) and not TownThreatened()) then
                if wait_seconds >= currentWaitLimit then
                    set LaunchAttack = true
                else
                    //Wait WaitTime seconds before launching the attack
                    call Sleep( 3 )
                    set wait_seconds = wait_seconds + 3
                    call DebugText("Waited " + Int2Str(wait_seconds) + " seconds before attack. Needs to wait " + Int2Str(currentWaitLimit) + " seconds.")
                endif
            else
                //Wait until the conditions have been satisfied
                call Sleep( 3 )
                call DebugText("Waited 3 seconds to have minimum attackers etc.")
            endif

            exitwhen LaunchAttack == true
        endloop
       
        //Reset the variable
        set LaunchAttack = false

        if base != null and UnitAlive(base) then
            //Launch Attack
            call PrepareAttackGroup( )

            //Start building the next wave
            set attack_wave = attack_wave + 1
            call InitBuildArray()
            call InitAssaultGroup()
            call BuildPriorities()

            //Attack target
            set AttackSuccess = AttackPlayerBase(base)
        endif

        call Sleep(3)
    endloop
endfunction

I am just curious about unit pointers. The created group is at least destroyed properly.

 

[IcemanBo]

Only a small potential leak, but you can put..

set g = null
set base = null

at the end of the function, to avoid reference leak. (explanation)

 

[Tommi Gustafsson]

Yes, but the function is an endless loop that never exits...

 

[IcemanBo]

Not sure what exactly happens for example inside:

• GetPlayerUnitTypeCount(player, integer) // this might have a leak
  
• HaveMinimumAttacker()
  
• CaptainRetreating()

but what is the reason at first place then for the specific unit leak question?

Btw, if it's anways infinite, the group does not need to be re-created/destroyed each run.

 

[Tommi Gustafsson]

I needed to review all my AI code, since sometimes (very rarely) the AIs stopped functioning. But the problem might not be in my scripts but somewhere else.

 

[Pyrogasm]

A reference leak is never going to cause an AI to stop executing properly. For that matter neither would a memory leak. In this case, if you never actually leave the loop then the reference is 'permanently' allocated; things like that aren't 'leaks' they're just... necessary.

 

[IcemanBo]

Was is possible to break it down, the part which was not functioning anymore?

Some curious points:

• It doesn't seem possible to me further action can be performed after exitwhen base != null inside the loop, as at this point base should always exist?
• Hope it's aware it's technically possible that the picked base is currently dead.
• Following check decides if an attack should be launched:
  ( HaveMinimumAttackers() and not CaptainRetreating() and not CaptainInCombat(true) and not TownThreatened()).
  But between the point the check gets varified, and the point the actual attack gets launched, there is a WaitTime time gap, which makes it possible the check varification would not be "true" anymore after that delay. Is that wanted?

 

[Tommi Gustafsson]

Was is possible to break it down, the part which was not functioning anymore?

Actually, the thing was the we saw the AI not working in a random playthrough video in YouTube. The AI had completely stopped working so that it did not do anything, like building units etc.

Some curious points:

• It doesn't seem possible to me further action can be performed after exitwhen base != null inside the loop, as at this point base should always exist?
• Hope it's aware it's technically possible that the picked base is currently dead.

Thanks for pointing that out. However, it seems that I had redeemed the thing later when it checks if base != null and UnitAlive(base) then.
I don't know why it is checked so late, though.

• Following check decides if an attack should be launched:
  ( HaveMinimumAttackers() and not CaptainRetreating() and not CaptainInCombat(true) and not TownThreatened()).
  But between the point the check gets varified, and the point the actual attack gets launched, there is a WaitTime time gap, which makes it possible the check varification would not be "true" anymore after that delay. Is that wanted?

If I read the code right, there is no call Sleep between set LaunchAttack = true and set AttackSuccess = AttackPlayerBase(base), which actually executes the attack.

 

[IcemanBo]

If I read the code right,

Yes, sorry. ;P

Dunno then, I'm not really familiar with AI. ^^ Maybe @Nowow can see something, I believe he is pretty familiar with it.

 

[Nowow]

As far as reference leaks go, I don't see any problem - you're using u in an inner loop, which exits when u itself is null, so no problem there. The variable base is also not leaking. I think nulling base explicitly once you know it's dead is better practice, but not a major problem.

I think your problem might actually be the waiting time: you have a delay of at least 10 minutes before currentWaitLimit is reached. Pad that by the amount of time it takes to actually create the attack wave so that HaveMinimumAttackers() evaluates to true, add to that any additional attacks to the town created by the player (TownThreatened() can evaluate to true even when you're not attacked and enemy units are close to a building you own). All in all, you're looking at about 15 minutes, and to top it all off, you might pick up a dead great den as base, and base might be dead by the time currentWaitLimit expires. So you might take double that time to attack if something happens to your base.

I suggest the following:
1. Remove HaveMinimumAttackers() from the condition to increment currentWaitLimit. For reference, you can look at the inner workings of blizzard's attack wave function (including CreateGroup). I even have a post about it somewhere. Also, consider creating a function which times how long TownThreatened() evaluates to true, in order to distinguish a real threat from just a bunch of units moving around.

2. Call BuildPriorities() outside of the if/else block at the end, possibly in a different thread altogether. This is probably not what makes your AI stop building, but updating your build priorities should be independent of executing an attack or not.

3. Call InitBuildArray() before calling BuildPriorities(). I assume you're using the BuildPriorities() to update your buildings, and not only your attackers. Those should be in different functions, for better control over what you build. But disregarding that, not calling InitBuildArray() could cause your building arrays to overflow eventually.

4. Post the video you're talking about. The AI not doing anything can, unfortunately be caused by a lot of different factors, and very minor ones at that. I can already tell you that if your units are not performing any animation at all (i.e. unit is completely static, not even a normal stand animation, looks weird) there might be a clash between orders sent to the users by triggers and the AI. Other reasons could be caused by something as little as a variable instead of the rawID integer for an upgrade (UPG_BLK_SPHINX seems to be funky every time I use it).