Animation not playing correctly

[PrisonLove]

Hi, I have a trigger that creates a bomb omb and makes it walk in a targte direction. When the bomb either comes into contact with another unit, or when its fuse runs out, it will explode damaging the area. All of that works just dandy, but there is one problem that I find so annoying: The bomb's walk animation won't play. It takes one step and then the animation freezes and I don't know why. The user defined functions that don't appear in the trigger are in the map header and work properly. Can anyone see why the animation isn't playing properly?

Here is the code:

function LaunchBombOmbConditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A000'
endfunction


function LaunchBombOmbGroupFilter takes nothing returns boolean
    local unit target = GetFilterUnit()
    local boolean flag = false

    if (IsUnitEnemy(target, udg_tempPlayer) and GetUnitState(target, UNIT_STATE_LIFE) > 0) then
        set flag = true
    endif
    
    set target = null
    return flag
endfunction


function LaunchBombOmbCallback takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local integer id = GetHandleId(t)
    local unit bomb = LoadUnitHandle(udg_hash, id, 0)
    local player p = LoadPlayerHandle(udg_hash, id, 1)
    local real angle = LoadReal(udg_hash, id, 2)
    local real fuseTime = LoadReal(udg_hash, id, 3)
    local real slideDist = 10
    local real x1 = GetUnitX(bomb)
    local real y1 = GetUnitY(bomb)
    local real x2 = PolarProjectX(x1, angle, slideDist)
    local real y2 = PolarProjectY(y1, angle, slideDist)
    local group g = CreateGroup()
    local integer numInGroup = 0

    set udg_tempPlayer = p
    call GroupEnumUnitsInRange(g, x2, y2, 50, Filter(function LaunchBombOmbGroupFilter))
    set numInGroup = CountUnitsInGroup(g)

    if (numInGroup == 0 and fuseTime > 0) then
        call SlideWithCollision(bomb, slideDist, angle, "")
        set fuseTime = fuseTime - .03
        call SaveReal(udg_hash, id, 3, fuseTime)
    else
        call DestroyEffect(AddSpecialEffect("war3mapImported\\NewGroundEX.mdx", x1, y1))
        call UnitDamagePoint(bomb, 0, 50, x1, y1, 25, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, WEAPON_TYPE_WHOKNOWS)
        call UnitDamagePoint(bomb, 0, 150, x1, y1, 15, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, WEAPON_TYPE_WHOKNOWS)
        call UnitDamagePoint(bomb, 0, 200, x1, y1, 10, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, WEAPON_TYPE_WHOKNOWS)
        call KillUnit(bomb)
        call RemoveUnit(bomb)
        
        call PauseTimer(t)
        call DestroyTimer(t)
        call FlushChildHashtable(udg_hash, id)
    endif

    set udg_tempPlayer = null
    set bomb = null
    set p = null
    set t = null
endfunction


function LaunchBombOmbActions takes nothing returns nothing
    local unit koopa = GetTriggerUnit()
    local player p = GetOwningPlayer(koopa)
    local integer bombID = 'o001'
    local real kx = GetUnitX(koopa)
    local real ky = GetUnitY(koopa)
    local real tx = GetSpellTargetX()
    local real ty = GetSpellTargetY()
    local real angle = GetAngle(kx, ky, tx, ty)
    local real dx = PolarProjectX(kx, 50, angle)
    local real dy = PolarProjectY(ky, 50, angle)
    local unit bomb = CreateUnit(p, bombID, dx, dy, angle)
    local timer t = CreateTimer()
    local integer id = GetHandleId(t)
    local real fuseTime = 3

    call SaveUnitHandle(udg_hash, id, 0, bomb)
    call SavePlayerHandle(udg_hash, id, 1, p)
    call SaveReal(udg_hash, id, 2, angle)
    call SaveReal(udg_hash, id, 3, fuseTime)

    call SetUnitAnimation(bomb, "walk")
    call TimerStart(t, .03, true, function LaunchBombOmbCallback)

    set koopa = null
    set bomb = null
    set p = null
    set t = null
endfunction

//===========================================================================
function InitTrig_LaunchBombOmb takes nothing returns nothing
    local trigger t = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( t, Condition( function LaunchBombOmbConditions ) )
    call TriggerAddAction( t, function LaunchBombOmbActions )
    set t = null
endfunction

 

[PurgeandFire]

You can always try pausing the unit, or setting its animation by its index. However, some animations have a specified default loop. That is where they will loop for a certain duration before pausing.

Thus, you might have to resort to a timer based on the animation's duration to keep resetting it.

 

[PrisonLove]

Actually I just discovered how to resolve the issue. By using SetUnitAnimationByIndex() I was able to solve the issue. Thanks for the help anyway.

 

[Bribe]

Efficiency:

1) put your "actions" within the conditions.
2) CountUnitsInGroup -- useless, you only require that any unit was added at all. You could easily use FirstOfGroup!=null to perform the exact same check.
3) You don't need a local group, it should be a global group that never gets units added to it.
4) "P" is kind of a waste to save into the hashtable. It's a bit faster just to use GetOwningPlayer over and over.
5) Do you have NewGen?
6) Nulling players - never do that. What a waste.
7) Nulling trigger that never gets destroyed- same as above.

 

[Kajik]

...
6) Nulling players - never do that. What a waste.
7) Nulling trigger that never gets destroyed- same as above.

How do you know that? I thought every variable (except integer, real, string and boolean) should be nulled. I am asking because I'm also using

local trigger trig = CreateTrigger()
...Register
...AddAction
set trig = null

 

[Bribe]

There is only one reason you do [ljass]set var = null[/ljass], and that's for handle recycling. Things that never get destroyed (spell triggers, players) have no point in being set to null, is even if all variables pointing to them are nulled you are not recycling their handle because the object itself still exists.

Dummy units, filter units, now those you always want to set to null.

 

[Jesus4Lyf]

There is only one reason you do [ljass]set var = null[/ljass], and that's for handle recycling.

On a philosophical level, I like to maintain a true reference count on all agents.

>Nulling players - never do that. What a waste.
>Nulling trigger that never gets destroyed- same as above.
It's not a waste, so to speak - it's one intangible benefit over another.
But technically speaking, you do not need to null vars pointing to agents you will never remove/destroy (players or permanent other things). Because the only thing we know the reference count is used for is handle recycling, and those handles should never be freed anyway.

 

[Kajik]

function B takes nothing returns nothing
    local player pl2 = GetHandlePlayer(GetExpiredTimer(), "pl1")
    call DoSomethingWithPlayer(pl2)
endfunction
    
function A takes nothing returns nothing
    local player pl1 = GetTriggerPlayer()
    call SetHandlePlayer(exampleTimer, "pl1", pl1)
    call TimerStart(exampleTimer, 0.0, false, function B)
endfunction

So I create 2 diferent var pointers and I dont have to null them because they are both pointing to permanent object (player)? Well, but this can generate variables I can't reach when function ends.
I think I am missing some important informations...

 

[Bribe]

>> So I create 2 diferent var pointers and I dont have to null them because they are both pointing to permanent object (player)?
> Yes.

>> Well, but this can generate variables I can't reach when function ends.
> Yes, "local variables" cannot be referenced past "endfunction". Hence the use of globals/hashtables.

>> I think I am missing some important informations
> There's an ocean of informations, you have to ask specific questions otherwise we have no idea what to tell you.

 

[saw792]

As you said, players are permanent objects. This means that they aren't created or destroyed, so while you can't reference the particular local pointer you used within a function you still return the same pointer for that player when you use any of the player natives later (i.e. GetTriggerPlayer() will return the same player if you call it multiple times for the same event response; it doesn't create a new player each time).

 

[Kajik]

Sure, I know that pointed object is still the same, but let's say I create 3 pointers in 3 diferent functions.

P1----> Object <----P2
    P3----^

Each of this 3 pointers uses one memory cell where is stored the adress of pointed object.

--------------
| ObjectData | - ObjectAdress
--------------
|   null     |
--------------
|ObjectAdress| - Adress of P1
--------------
|ObjectAdress| - Adress of P3
--------------
|ObjectAdress| - Adress of P2
--------------

If I null the pointer P3, it erase the pointer data (Object adress) so this memory cell can be used for another pointer. But memory cells of P1 and P2 will be still occupied till game ends.

P1----> Object <----P2

    P3---->null

Each of this 3 pointers uses one memory cell where is stored the adress of pointed object.

--------------
| ObjectData | - ObjectAdress
--------------
|   null     |
--------------
|ObjectAdress| - Adress of P1
--------------
|   null     | - Adress of P3
--------------
|ObjectAdress| - Adress of P2
--------------

So not-nulling pointers to objects which I am "not going to destroy anyway" is slowly eating the memory. Am I wrong?

 

[saw792]

Well I can't be sure exactly how the program cleans up memory usage. However, I suspect you would be wrong in assuming that nulling the pointer is the trigger for it to be cleaned up. I would think that once a pointer drifts out of scope it's memory is then available for reuse (being a completely separate issue to the reference count of the object itself).

I am not an expert on this however, and I don't think we can really be sure without seeing the source code.

 

[Bribe]

Being able to still use locals even after a TriggerSleepAction was what brought me to learn JASS. Unfortunately, I realized TriggerSleepAction was a bad idea for everything I wanted to use it for, anyway

But now, I wonder if I should continue to use locals, for the same reasons listed above. Past few days I've been trimming out some locals for globals. Especially when so much in this game involves group enumerating and it's easier to use globals than to keep nulling locals.

 

[saw792]

Test memory usage before and after:

scope Mem initializer Init

globals
  private player P = Player(0)
endglobals

function MemTest takes nothing returns nothing
  local player p0 = P
  local player p1 = P
  local player p2 = P
  local player p3 = P
  local player p4 = P
  local player p5 = P
  local player p6 = P
  local player p7 = P
  local player p8 = P
  local player p9 = P
endfunction

private function Ay takes nothing returns nothing
  local integer i = 0
  loop
    exitwhen i == 100
    call MemTest()
    set i = i + 1
  endloop
endfunction

private function Ax takes nothing returns nothing
  local integer i = 0
  loop
    exitwhen i == 100
    call Ay.execute()
    set i =  i + 1
  endloop
endfunction

private function Init takes nothing returns nothing
  //on esc press run Ax
endfunction

endscope

That should test that theory. Right? Should be 10x100x100 = 100000 pointers declared, i.e. 100000 x 4 bytes = 400000 bytes = 400KB. Hmm might need more iterations...

 

[Bribe]

Ran this 10 times; RAM remained a steady 89,548 kb each and every test.

scope Mem initializer Init

globals
  private player P = Player(0)
endglobals

function MemTest takes nothing returns nothing
  local player p0 = P
  local player p1 = P
  local player p2 = P
  local player p3 = P
  local player p4 = P
  local player p5 = P
  local player p6 = P
  local player p7 = P
  local player p8 = P
  local player p9 = P
endfunction

private function Ay takes nothing returns nothing
  local integer i = 0
  loop
    exitwhen i == 1000
    call MemTest()
    set i = i + 1
  endloop
endfunction

private function Ax takes nothing returns nothing
  local integer i = 0
  loop
    exitwhen i == 1000
    call Ay.execute()
    set i =  i + 1
  endloop
endfunction

private function Init takes nothing returns nothing
    local trigger t=CreateTrigger()
    call TriggerRegisterPlayerEvent(t,Player(0),EVENT_PLAYER_END_CINEMATIC)
    call TriggerAddAction(t, function Ax)
endfunction

endscope

 

[saw792]

I suspect the thread would have hit the op limit before 1000 iterations. Still, it backs up my theory (which doesn't say it's not still flawed).

 

[Bribe]

Each time I pressed Esc, it lagged for about 6-7 seconds before I could check its RAM.

 

[Kajik]

Each time I pressed Esc, it lagged for about 6-7 seconds before I could check its RAM.

Hmm, maybe because procesor saves data which won't be needed in a near future from RAM to disk space. It's called "content switch".

I suspect the thread would have hit the op limit before 1000 iterations.

Thanks to virtual memory, process can adress far more than RAM space (the process believes he owns whole disk space and he is only process in whole system). There is hard limit on 4GB I think and reasonable application shouldn't allow it's process to ask for such a big amount of memory.

Anyways, all of this backs up my theory, but as I said - I could be wrong...

 

[saw792]

It doesn't back up your theory at all. You're talking about an increase in page file usage instead of physical memory usage, which you have no evidence to support. It's also ridiculously difficult to track the page file usage of a particular process so you're going to have trouble testing this as well.

Your response to my op limit comment is completely unrelated...

 

[Bribe]

It's not much, but it's relevant because it shows the code is running for 6-7 seconds instead of crashing the thread immediately.