[JASS] Simple unit revival system not working

[Kroack]

Please ignore my code needing cleaning up - like rewriting CUnitType and preventing leaks. That is not my current concern.

I have a simple JASS code wherein a unit is created after the same unit type of the same owner dies after 5 seconds. Why is it not working?

function CUnitType takes nothing returns boolean
    if ( not ( GetUnitTypeId(GetDyingUnit()) == 'hfoo' ) ) then
        return false
    endif
    return true
endfunction

function ExpireTimer takes nothing returns nothing
    local timer t=GetExpiredTimer()
    local integer id=GetHandleId(t)
        local unit u = LoadUnitHandle(udg_Hashtable, id, 2)
    local player p=GetOwningPlayer(u)
    //local location pos=GetRandomLocInRect(GetPlayableMapRect()) remove useless
    local location loc
    local integer intx=GetRandomInt(-512,512)
    local integer inty=GetRandomInt(-512,512)

    //call DisplayTextToForce( GetPlayersAll(), I2S(id) )

    set loc=Location(intx,inty)    
    call DisplayTextToForce( GetPlayersAll(), "unit spawned" ) 
    //call CreateNUnitsAtLoc( 1, 'hfoo', p, loc, bj_UNIT_FACING )
    call CreateUnit(p,'hfoo',intx,inty,0)

    call DestroyTimer(t)
    set t=null
    set u=null        
endfunction

function CallTimer takes nothing returns nothing
    local unit u=GetDyingUnit()
    local timer t=CreateTimer()
    local integer id=GetHandleId(t)
    //call DisplayTextToForce( GetPlayersAll(), I2S(id) )
    call SaveUnitHandle(udg_Hashtable, id, 2, u)
    call TimerStart(t, 5,false,function ExpireTimer)
endfunction

//===========================================================================
function InitTrig_UnitDeath takes nothing returns nothing
    local trigger t = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_DEATH )
    call TriggerAddCondition(t,Condition (function CUnitType))
    call TriggerAddAction( t, function CallTimer)
endfunction

If I use, say, SaveUnitHandle(udg_Hashtable, 1, 1, u) and LoadUnitHandle(udg_Hashtable, 1, 1), it works fine with the corresponding player. But then all the other players are excluded.

 

[Uncle]

Perhaps the unit is being removed or something. I don't see the issue but here's a version that makes more sense to me, I'm saving the Unit-Type and it's Player Id as Integers instead of hoping that the Footman will still exist after 5.00 seconds:

function UnitDeath_TimerExpired takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local integer id = GetHandleId(t)
    local integer ut = LoadInteger(udg_Hashtable, id, 2) // load unit-type
    local integer pid = LoadInteger(udg_Hashtable, id, 3) // load player id
    local real x = GetRandomReal(-512.0, 512.0)
    local real y = GetRandomReal(-512.0, 512.0)
    local player p = Player(pid)

    call CreateUnit(p, ut, x, y, 270.0)
    call DisplayTextToForce(GetPlayersAll(), "unit spawned")

    call DestroyTimer(t)
    set u = null
    set p = null
    set t = null
endfunction

function UnitDeath_Condition takes nothing returns boolean
    if (GetUnitTypeId(GetTriggerUnit()) == 'hfoo') then
        return true
    endif
    return false
endfunction

function UnitDeath_Action takes nothing returns nothing
    local unit u = GetTriggerUnit()
    local timer t = CreateTimer()
    local integer id = GetHandleId(t)

    call SaveInteger(udg_Hashtable, id, 2, GetUnitTypeId(u)) // save unit-type
    call SaveInteger(udg_Hashtable, id, 3, GetPlayerId(GetOwningPlayer(u))) // save player id
    call TimerStart(t, 5.0, false, function UnitDeath_TimerExpired)

    set u = null
    set t = null
endfunction

//===========================================================================
function InitTrig_UnitDeath takes nothing returns nothing
    local trigger tr = CreateTrigger()

    call TriggerRegisterAnyUnitEventBJ(tr, EVENT_PLAYER_UNIT_DEATH)
    call TriggerAddCondition(tr, Condition(function UnitDeath_Condition))
    call TriggerAddAction(tr, function UnitDeath_Action)

    set tr = null
endfunction

 

[Rheiko]

It's because the game cannot recognize the owner after you load the unit handle for some reason. You should save the player id into the hashtable and load that instead, it will work.

add this to your CallTimer function
call SaveInteger(udg_Hashtable, id, 3, GetPlayerId(GetOwningPlayer(u)))

and change the line in your ExpireTimer function to this
local player p = Player(LoadInteger(udg_Hashtable, id, 3))

For some reason, SavePlayerHandle and LoadPlayerHandle do not work for me. I have no idea what the reason could be.

Edit:
It's better to use Uncle's code for a cleaner approach.

 

[Kroack]

Strange. It seems to work, unless I use the GetHandleId local in one of SaveUnitHandle's arguments.

I was planning, for the future, to use the dying unit's stats like its affected mana or inventory items by referencing the dead unit. That is why I was saving the unit itself into a handle. I suppose I can save an integer/real as the dying unit's mana instead. Never coded inventories with JASS, so I have no idea how to work with that.


Anyway, a leak question.

If I have a loop, do I null a local within the loop or can they be safely overridden unlike via GUI?

Option 1:

    loop
        exitwhen i>x
        set locran =GetRandomLocInRect(GetPlayableMapRect())
        set realx=GetLocationX(locran)
        set realy=GetLocationY(locran)

        call IssueBuildOrderById(u, 'hhou', realx, realy)
        set i=i+1
       
        call RemoveLocation(locran)
        set locran=null
    endloop

Option 2:

    loop
        exitwhen i>x
        set locran =GetRandomLocInRect(GetPlayableMapRect())
        set realx=GetLocationX(locran)
        set realy=GetLocationY(locran)

        call IssueBuildOrderById(u, 'hhou', realx, realy)
        set i=i+1
    endloop

    call RemoveLocation(locran)
    set locran=null

And thanks guys, I got the system to work as intended by using unit-type and player id handles.

 

[Rheiko]

Go for Option 1. In Option 2 you will leak x-1 number of location since only the last one will be cleaned up.

 

[Uncle]

Minor detail, in Option 1 you can move the "set locran=null" to outside of the loop, like where you've placed it in Option 2.

Removing the Location needs to occur for each new Location object that you create. This function creates a new Location object:

exitwhen i>x
set locran =GetRandomLocInRect(GetPlayableMapRect())

^ You're creating "x" Location objects in the Loop, so all "x" of them need to be cleaned up.

However, there's only one "locran" variable, so nulling it only needs to happen once. In this case you'd null it once you're completely done using it.

    loop
        exitwhen i>x
        set locran =GetRandomLocInRect(GetPlayableMapRect())
        set realx=GetLocationX(locran)
        set realy=GetLocationY(locran)

        call IssueBuildOrderById(u, 'hhou', realx, realy)
        call RemoveLocation(locran) // Done with the object? Remove or destroy it
        set i=i+1
    endloop

    set locran=null // Done with the variable? Null it

Lastly, I recommend working in coordinates (x/y/z) directly and avoiding these Location based functions. You'll often be creating unnecessary work since the coordinates are more direct, more efficient and don't leak.

    loop
        exitwhen i>x
        set realx=GetRandomMapX()
        set realy=GetRandomMapY()

        call IssueBuildOrderById(u, 'hhou', realx, realy)
        set i=i+1
    endloop

^ Those are not real functions but you could easily create them. Then to further clean your code, you don't even need variables for them:

    loop
        exitwhen i>x
        call IssueBuildOrderById(u, 'hhou', GetRandomMapX(), GetRandomMapY())
        set i=i+1
    endloop

 

[Kroack]

Incredibly handy. It did not occur to me GetRandomMapX/Y was an option.

 

[PurgeandFire]

^just to clarify, you'll still need to code those functions yourself (as Uncle mentioned). They don't exist by default

It would look like this:

function GetRandomMapX takes nothing returns real
    return GetRandomReal(GetRectMinX(bj_mapInitialPlayableArea), GetRectMaxX(bj_mapInitialPlayableArea))
endfunction

function GetRandomMapY takes nothing returns real
    return GetRandomReal(GetRectMinY(bj_mapInitialPlayableArea), GetRectMaxY(bj_mapInitialPlayableArea))
endfunction