[JASS] Why does this leak (floatingtext)?

[looking_for_help]

Hello again,

my function I am sitting on since a few days is finally doing what I want, but it causes heavy memory leaks. I really dont understand why, I tried to make everything local and destroy it afterwards, but still it leaks my memory completley within minutes. Here is the code:

function Trig_text_Actions takes nothing returns nothing

    local location myLoc = GetUnitLoc(gg_unit_Obla_0006)
    local texttag myText = null
    local string myString = null
    local force AllPlayers = GetPlayersAll()

    local player Player_0 = Player(0)
    local player Player_1 = Player(1)
    local player Player_11 = Player(11)


    if ( udg_counter < 7) then
        set myString = I2S(udg_counter)
    else
        set myString = "test"
    endif

    if udg_destroyedText == 0 then
        call DestroyTextTagBJ( myText )
        set udg_destroyedText = 1
    endif
    

    if (IsUnitDeadBJ(gg_unit_Obla_0006) == false) and ( not ( gg_unit_Obla_0006 == null ) ) and ( ( GetUnitAbilityLevelSwapped('A001', gg_unit_Obla_0006) >= 1 ) or ( GetUnitAbilityLevelSwapped('A000', gg_unit_Obla_0006) >= 1 ) ) then
        set udg_destroyedText = 0

        if ( udg_counter < 7) then

            if (IsUnitVisible(gg_unit_Obla_0006, Player_11) == true) then
                set myText = CreateTextTagLocBJ( myString, myLoc, 200, 12, 100, 0, 0, 0)
                call ShowTextTagForceBJ( true, myText, AllPlayers )
            else

                if GetLocalPlayer() == Player_0 then
                    set myText = CreateTextTagLocBJ( myString, myLoc, 200, 12, 100, 35, 30, 0)
                    call ShowTextTagForceBJ( true, myText, GetForceOfPlayer(Player_0) )
                else
                    if (IsUnitVisible(gg_unit_Obla_0006, Player_1) == true) then
                        set myText = CreateTextTagLocBJ( myString, myLoc, 200, 12, 100, 35, 30, 0)
                        call ShowTextTagForceBJ( true, myText, AllPlayers )
                    endif
                endif
            endif

        else

            if (IsUnitVisible(gg_unit_Obla_0006, Player_11) == true) then
                set myText = CreateTextTagLocBJ( myString, myLoc, 200, 12, 100, 0, 0, 0 )
                call ShowTextTagForceBJ( true, myText, AllPlayers )
            else
                if GetLocalPlayer() == Player_0 then
                    set myText = CreateTextTagLocBJ( myString, myLoc, 200, 12, 100, 35, 30, 0 )
                    call ShowTextTagForceBJ( true, myText, GetForceOfPlayer(Player_0) )
                else
                    if (IsUnitVisible(gg_unit_Obla_0006, Player_1) == true) then
                        set myText = CreateTextTagLocBJ( myString, myLoc, 200, 12, 100, 35, 30, 0)
                        call ShowTextTagForceBJ( true, myText, AllPlayers )
                    endif
                endif
            endif 
        endif
    endif
    


    call RemoveLocation(myLoc)
    call DestroyForce(AllPlayers)


    set AllPlayers = null
    set Player_0 = null
    set Player_1 = null
    set Player_11 = null
    set myText = null
    set myLoc = null
    set myString = null

endfunction

//===========================================================================
function InitTrig_text takes nothing returns nothing
    set gg_trg_text = CreateTrigger(  )
    call TriggerRegisterTimerEventPeriodic( gg_trg_text, 0.00 )
    call TriggerAddAction( gg_trg_text, function Trig_text_Actions )
endfunction

The function basically displays a variable (counter) as a floatingtext over a unit. If the Unit gets invisible the text changes its color and gets invisible to the enemy player etc.

I really can't figure out where the memory leak comes from, but it has to be in this trigger, as it disappears if I deactivate it...

 

[SeriousEnemy]

I think the main problem comes from the text tag and your repeating time.

You need to destroy text tags, they still leak if you null the variable.
call DestroyTextTag(variable name)

You can also try
call SetTextTagFadepoint(variable name, 0.1)
call SetTextTagLifespan(variable name, 0.1)
call SetTextTagPermanent(variable name, false)

And starting the same trigger every 0.00 seconds is a little bit to much.



Oh and try to avoid BJ functions if you repeat stuff very often.
Each bj starts another function (warcraft standart functions)!
Here's a good list of almost anything.. you should check it out.
http://jass.sourceforge.net/doc/api/Blizzard_j-functions.shtml

 

[WaterKnight]

You do not destroy the texttag. The variable you use is a local one. Local variables are not function specific but function call specific, so the next time the function runs, it commands other variables than the previous instance.

Also you should not destroy the force you get from GetPlayersAll() because it returns a single globally saved force, does not create a new one.

 

[looking_for_help]

Ok, thanks for the replies.

But how do I have to destroy the texttag properly? I changed my code to:

function Trig_text_Actions takes nothing returns nothing

    local location myLoc = GetUnitLoc(gg_unit_Obla_0006)
    local texttag myText = null
    local string myString = null
    local force AllPlayers = GetPlayersAll()

    local player Player_0 = Player(0)
    local player Player_1 = Player(1)
    local player Player_11 = Player(11)

    local force Player_0_force = GetForceOfPlayer( Player(0))


    if ( udg_counter< 7) then
        set myString = I2S(udg_counter)
    else
        set myString = "test"
    endif


    set myText = GetLastCreatedTextTag()
    call DestroyTextTagBJ(myText)
    set myText = null

    

    if (IsUnitLoaded(gg_unit_Obla_0006) == false) and (IsUnitDeadBJ(gg_unit_Obla_0006) == false) and ( not ( gg_unit_Obla_0006 == null ) ) and ( ( GetUnitAbilityLevelSwapped('A001', gg_unit_Obla_0006) >= 1 ) or ( GetUnitAbilityLevelSwapped('A000', gg_unit_Obla_0006) >= 1 ) ) then


        if ( udg_counter < 7) then

            if (IsUnitVisible(gg_unit_Obla_0006, Player_11) == true) then
                set myText = CreateTextTagLocBJ( myString, myLoc, 200, 12, 100, 0, 0, 0)
                call ShowTextTagForceBJ( true, myText, AllPlayers )
            else

                if GetLocalPlayer() == Player_0 then
                    set myText = CreateTextTagLocBJ( myString, myLoc, 200, 12, 100, 35, 30, 0)
                    call ShowTextTagForceBJ( true, myText, Player_0_force )
                else
                    if (IsUnitVisible(gg_unit_Obla_0006, Player_1) == true) then
                        set myText = CreateTextTagLocBJ( myString, myLoc, 200, 12, 100, 35, 30, 0)
                        call ShowTextTagForceBJ( true, myText, AllPlayers )
                    endif
                endif
            endif

        else

            if (IsUnitVisible(gg_unit_Obla_0006, Player_11) == true) then
                set myText = CreateTextTagLocBJ( myString, myLoc, 200, 12, 100, 0, 0, 0 )
                call ShowTextTagForceBJ( true, myText, AllPlayers )
            else
                if GetLocalPlayer() == Player_0 then
                    set myText = CreateTextTagLocBJ( myString, myLoc, 200, 12, 100, 35, 30, 0 )
                    call ShowTextTagForceBJ( true, myText, Player_0_force )
                else
                    if (IsUnitVisible(gg_unit_Obla_0006, Player_1) == true) then
                        set myText = CreateTextTagLocBJ( myString, myLoc, 200, 12, 100, 35, 30, 0)
                        call ShowTextTagForceBJ( true, myText, AllPlayers )
                    endif
                endif
            endif 
        endif
    endif
    


    call RemoveLocation(myLoc)
    call DestroyForce(AllPlayers)
    call DestroyForce(Player_0_force)

    set AllPlayers = null
    set Player_0_force = null
    set Player_0 = null
    set Player_1 = null
    set Player_11 = null
    set myText = null
    set myLoc = null
    set myString = null

endfunction

//===========================================================================
function InitTrig_text takes nothing returns nothing
    set gg_trg_text = CreateTrigger(  )
    call TriggerRegisterTimerEventPeriodic( gg_trg_text, 0.01 )
    call TriggerAddAction( gg_trg_text, function Trig_text_Actions )
endfunction

and it seems that the leak is fixed. At least I could play it for half an hour without any memory problems...

Is the texttag now destroyed properly? And what happens if I have multiple Units that have this ability? And wouldn't GetLastCreatedTextTag interfer with different units?

And GetForceOfPlayer is not like GetPlayersAll? So I have only to destroy the forces returned by GetForceOfPlayer?

I don't really understand this, I read in some tutorials that local variables only have a lifespan of its function runtime, so why do I have to destroy anything at all?

Thanks for all help!

Greetings,
lfh

 

[Arhowk]

GetForceOfPlayer leaks.

GetPlayersAll doesnt leak because it returns a global

 

[looking_for_help]

Ok, does "GetForceOfPlayer leaks" mean that it leaks allways/in my code or if you use it properly it doesn't leak also?

 

[WaterKnight]

Just take a look in the API. Both functions are bjs, meaning they are declared within the blizzard.j script file and consist of jass code you could have written yourself, stuff not being native.

function GetPlayersAll takes nothing returns force
    return bj_FORCE_ALL_PLAYERS
endfunction

function GetForceOfPlayer takes player whichPlayer returns force
    local force f = CreateForce()
    call ForceAddPlayer(f, whichPlayer)
    return f
endfunction

You may easily see that the first one returns the content of a global variable, second one creates a new force to return.

GetLastCreatedTextTag naturally just returns the value of a global variable as well. If you want to have it unit specific, you need to globally store the texttags but assign them to the unit they belong to. This can be done with hashtable and the GetHandleId function to clearly identify each unit for example.

Local variables are being killed with the termination of the function call but that does not mean the objects behind them would be. You have to distinguish between variables and objects. Multiple variables may point to the same object. Wc3 does not have a garbage collector that would release objects not longer referenced and this would not work well for interactive objects like units anyway because they might be handled by other ingame mechanics.

You are instructed to nullify variables because certain objects do not completely release their memory after destruction unless all references like variables are gone. Those objects hold a unique id and this id can only be given to a new object if it's sure nothing happens to the old object (via the id) anymore. Else the new object would be hit by false actions, it's a security method. While this is senceful, however, Blizzard included another bug. Local variables do not automatically free their assigned object reference when their lifespan expires. The reference counter is not decremented and therefore the object cannot return its id anymore. With global variables, it is not a big issue since those are static and often recycled but local variables are dynamically allocated throughout runtime.

Btw, you call it "leak" when there is memory reserved you cannot get rid of anymore, probably because you lost all references to it. So no, GetForceOfPlayer does not force a leak but allocates memory you should return when you do not need it anymore.

 

[looking_for_help]

Ok, thank you very much for this great explanation.

Then I will now start to replace all BJ-functions (I dont understand anyway why they exist, if you can replace them by faster native ones).

 

[SeriousEnemy]

To make the script more readable and not that long.
Good for beginners, because you mostly write exactly what you want without any additional lines of code.

Some of the bj have been removed over time (those which are almost the same, but they still exist to make sure old maps still work.

 

[-Kobas-]

Can you tell me what exactly your code is supposed to do?

 

[WaterKnight]

They merge native tasks and serve as wrappers. As you advance, build systems etc. you will also find need in declaring your own functions and structuring your tasks.

 

[looking_for_help]

Ok, learned something again.

Can you tell me what exactly your code is supposed to do?

Sure.

It displays the variable counter over a certain unit (the texttag follows the unit). If the unit gets invisible, the text gets also invisible to enemy players. And if the variable has a certain value (here: 7) it displays a pre-defined string instead of variable counter. Thats basically all.

 

[-Kobas-]

I can create library for you if you want that will do that easy, fast and efficient enough not to disturb fps you already have.

Just tell me how you want it, I can do something like:
call RegisterTextTag(id,unit,string,color)
then you will be able to remove it later or do whatever you want with it if needed.

 

[looking_for_help]

Hi,

thanks for this offer, but I first want to try it out on my own to learn and get a deeper understanding.

But maybe you can help me with another question I couldn't find a solution for untill now:

As one can see, I use gg_unit_Obla_0006 (Blademaster) to refere to the unit I need. But this only works if the Hero is already placed in the map, but I want to refer to it when the hero was build later during the game (like in normal melee).

How can I do this?

Best regards,
lfh

 

[WaterKnight]

Save the unit in an own unit variable when it appears. There are events like unit finishes training or unit enters entire map.

 

[looking_for_help]

Hm, ok, I did so and this works:

function Trig_bmCreated_Conditions takes nothing returns boolean
    if ( GetUnitTypeId(GetTriggerUnit()) == 'Obla' ) then
        return true
    else
        return false
    endif
endfunction

function Trig_bmCreated_Actions takes nothing returns nothing
    set udg_unitBM = GetTriggerUnit()
endfunction

//===========================================================================
function InitTrig_bmCreated takes nothing returns nothing
    set gg_trg_bmCreated = CreateTrigger(  )
    call TriggerRegisterEnterRectSimple( gg_trg_bmCreated, GetEntireMapRect() )
    call TriggerAddCondition( gg_trg_bmCreated, Condition( function Trig_bmCreated_Conditions ) )
    call TriggerAddAction( gg_trg_bmCreated, function Trig_bmCreated_Actions )
endfunction

BUT if I now want to trigger if that specific unit died, the trigger doesn't work:

function Trig_bmDies_Actions takes nothing returns nothing
    call BJDebugMsg("check")
endfunction

//===========================================================================
function InitTrig_bmDies takes nothing returns nothing
    set gg_trg_bmDies = CreateTrigger(  )
    call TriggerRegisterUnitEvent( gg_trg_bmDies, udg_unitBM, EVENT_UNIT_DEATH )
    call TriggerAddAction( gg_trg_bmDies, function Trig_bmDies_Actions )
endfunction

But if I use gg_unit_Obla_0006 (Hero already exists when starting the map in this case) instead of the global variable udg_unitBM, it works... why is that?

 

[Arhowk]

because the init function runs before udg_unitBM enters the map. try putting the TriggerRegisterUnitEvent in InitTrig_bmDies into Trig_bmCreated_Actions

 

[WaterKnight]

A common mistake that is probably thought this way because GUI users rarely make use of the "Trigger - Add New Event To Trigger" action. Instead, they enter events statically in that branch GUI presents them. The events there are created during map initialization, which is also the reason you are not allowed to select variable values for them because at this point, the variables will hardly hold a proper information other than the starting value set in variable editor.

When you change the color of a unit with SetUnitColor for example and write a variable unit within the brackets, the color modification would not shift to another unit either when the variable is reset. You do not directly pass the variable but rather just the current value behind it as the code is executed. Jass does not allow to pass variables (at least not directly) so you cannot write a function like one that swaps the contents of two variables.

 

[looking_for_help]

Ah, ok now that is good to know

I started the triggers now first when the BM entered the map and now it works fine, thank you guys!

Now I have only one remaining problem with my map (which might be quite simple to solve):

I used as a map a standard melee map (Terenas Stand) where I implemented all these triggers. Now I wanted to play the map, but it is in some kind different to the standard Terenas Stand, allthough I didn't change anything in the map itself (Only the hero and the triggers for the hero).

For example, in the orc shop there is no clarity potion available but a dust of apperance, some creeps drop different items than in the ladder map of TS and in the tavern not all heros are available (no Firelord, no Tinker, no Alchi).

I think this is an older version of the ladder TS, but I took the one from my map directory, so it should be up-to-date.

Do I have to tell the editor to use the most recent version?

Greetings,
lfh

 

[WaterKnight]

Yes, when your map is not classified as a melee map, it uses another data set on default. Go to >Scenario >Map Options and change the game data set at the bottom to Melee (Latest Patch).

 

[looking_for_help]

Works perfect!

I really have to say, this board is awesome. BIG thanks to you all.

Best regards,
lfh

 

[Dr Super Good]

String always leak. Once a unique string is created it can never be destroyed. If you do something simple like a counter that counts from 1 to 2^31-1 while generating strings of that number you will soon find that every itteration of the algorthim starts to become very demanding on the processor.

 

[looking_for_help]

Hm, ok. Would it be better then to make the string global, so that I always overwrite the older string instead of creating new ones?

Anyway, I finished with the first version of my map, so I attach you a copy of it if you are interested.

The main idea is to make the critical Strike of the Blademaster deterministic and chargeable. This means that the Blademaster, once critical Strike is skilled, has to hit his target six times to "charge" the Spell. Once its charged, he crits the next hit.

It works very well if only one player is orc. Right now i'm trying to implement this for two orc-players (or more).

Feel free to try it out

Greetings,
lfh

 

[WaterKnight]

Hm, ok. Would it be better then to make the string global, so that I always overwrite the older string instead of creating new ones?

You cannot overwrite it. Again, you have to differ between variables and objects. A string object is created whenever there is a "" expression or functions like I2S. However, wc3 wants to recycle them, so the same variation of characters results in presenting you the same string. When you reset the variable, this does not influence the object it had pointed to. The object stays in memory and for strings no destroy function is available. Your only chance is to avoid allocating too many different character variations.

 

[looking_for_help]

Hm ok, if its recycled its ok as I use only very few different strings.

One really has to be carefull with this memory-leak stuff

 

[looking_for_help]

A common mistake that is probably thought this way because GUI users rarely make use of the "Trigger - Add New Event To Trigger" action. Instead, they enter events statically in that branch GUI presents them. The events there are created during map initialization, which is also the reason you are not allowed to select variable values for them because at this point, the variables will hardly hold a proper information other than the starting value set in variable editor.

Hi again, I have one litte remaining question with this.

I used TriggerAddAction and it works as I like, but I must have the double code. Here as an example:

function Trig_bmDies_Actions takes nothing returns nothing    
    call BJDebugMsg("test")
endfunction

function Trig_bmCreated_Conditions takes nothing returns boolean
    if ( GetUnitTypeId(GetTriggerUnit()) == 'Obla' ) and ( GetOwningPlayer(GetTriggerUnit()) == Player(0) ) then
        return true
    else
        return false
    endif
endfunction

function Trig_bmCreated_Actions takes nothing returns nothing
   
    set udg_unitBM = GetTriggerUnit()

    set gg_trg_bmDies = CreateTrigger(  )
    call TriggerRegisterUnitEvent( gg_trg_bmDies, udg_unitBM, EVENT_UNIT_DEATH )
    call TriggerAddAction( gg_trg_bmDies, function Trig_bmDies_Actions )

endfunction

This is the code of my first trigger. My second trigger is just:

function Trig_bmDies_Actions takes nothing returns nothing
    call BJDebugMsg("test")
endfunction

//===========================================================================
function InitTrig_bmDies takes nothing returns nothing
    set gg_trg_bmDies = CreateTrigger(  )
    call TriggerRegisterUnitEvent( gg_trg_bmDies, udg_unitBM, EVENT_UNIT_DEATH )
    call TriggerAddAction( gg_trg_bmDies, function Trig_bmDies_Actions )
endfunction

Which is basically exactly the same as in the trigger above. But if I delete the second trigger (I would prefer having all in one Trigger), then the WE gives me the error message "name of variable expected".

Why is that?

 

[WaterKnight]

The only variable expression in your second code is gg_trg_bmDies, which is a, by the editor, generated variable identifying the trigger. It mirrors the name of the trigger page in editor, prefixed by gg_trg_. Special symbols, whitespace etc. get replaced by an underscore. So would guess you renamed your trigger/copied code around.

 

[looking_for_help]

Hi, thanks for the answer.

I didn't change the name of the trigger, I have at the moment two trigger pages in the editor (bmCreated and bmDies). As all the code is allready in bmCreated, I want to get rid of the whole trigger page of bmDies. But if I delete it, I get the error message.

I don't really get why this is, as the same code is allready in bmCreated?


EDIT:

I uploaded a minimal-example testmap where one can see the problem. Just build a blademaster and kill him with the towers, so you will get the Debug-Message "test".

I used the two trigger pages bmCreated and bmDies. Now I want to get rid of the page bmDies, as it doesn't contain any "new" code.