Blizzard on crack? (MB)

[ZugZugZealot]

Or am I missing something...?

function MultiboardSetItemValueBJ takes multiboard mb, integer col, integer row, string val returns nothing
    local integer curRow = 0
    local integer curCol = 0
    local integer numRows = MultiboardGetRowCount(mb)
    local integer numCols = MultiboardGetColumnCount(mb)
    local multiboarditem mbitem = null

    // Loop over rows, using 1-based index
    loop
        set curRow = curRow + 1
        exitwhen curRow > numRows

        // Apply setting to the requested row, or all rows (if row is 0)
        if (row == 0 or row == curRow) then
            // Loop over columns, using 1-based index
            set curCol = 0
            loop
                set curCol = curCol + 1
                exitwhen curCol > numCols

                // Apply setting to the requested column, or all columns (if col is 0)
                if (col == 0 or col == curCol) then
                    set mbitem = MultiboardGetItem(mb, curRow - 1, curCol - 1)
                    call MultiboardSetItemValue(mbitem, val)
                    call MultiboardReleaseItem(mbitem)
                endif
            endloop
        endif
    endloop
endfunction

Instead of...

MultiboardSetItemValueBJ(mb, 0, 0, "Some Text")

I'm using...

set mbI = MultiboardGetItem(mb, 0, 0)
call MultiboardSetItemValue(mbI, "Some Text")
call MultiboardReleaseItem(mbI)

...which seems to work just fine...

 

[Troll-Brain]

It's like GetPlayerIdBJ or whatever it's called in gui, expect here they use 0 for setting all lines/rows.
Just remember it's a gui function.
When you use a multiboard in gui, rows and lines begin with 1 and not 0.

 

[ZugZugZealot]

And what you're going on...
It could just be...

set mbI = MultiboardGetItem(mb, udg_Row - 1,udg_Col - 1)
call MultiboardSetItemValue(mbI, "Some Text")
call MultiboardReleaseItem(mbI)

Anyways, the part that sticks out to me as odd... Which is the loops... Or does that not strike you as odd? >_>

 

[Troll-Brain]

So the loops don't strike you even as the littlest bit odd?

No.
In fact this function is not that bad, it does the job fine.
If you care about speed you can inline it each time you need it, but it should be irrelevant, unless your do it really often like each 0.01 s.

EDIT :
You don't get it "0" is used to set all rows/columns of the mb.

 

[ZugZugZealot]

"it does the job just fine" just means "it's okay to use," but it doesn't mean "what they're doing is rational."

I want to know if there is method to this madness.

 

[Troll-Brain]

"it does the job just fine" just means "it's okay to use," but it doesn't mean "what they're doing is rational."

I want to know if there is method to this madness.

Seriously, wake up, the gui functions weren't made in the most efficient or even rational way.
I thought it was pretty obvious and known.

 

[ZugZugZealot]

PolledWait() is a pretty rational GUI function, so are most of the Cam functions...

I was looking for information, not "Blizzard can is justified for everything" fanboy attitude.

 

[Troll-Brain]

PolledWait leak an handle id ...

 

[ZugZugZealot]

Cite.

 

[Troll-Brain]

Cite.

No need, a timer is destroyed an the local variable associated isn't set to null.

 

[Xorifelse]

MultiboardSetItemValueBJ is probably one of the worst BJ's there is, not only does it create a leak ( mbitem not being set to null ) it is coded inefficiently.

But the reason why it's looping is pretty simple, it's to prevent out of bounds indexes. Yes, I agree it could just have used simple if else statements.

It is a bad function, just bullet proof for GUI users. (except for the leak)

 

[Artificial]

The loop is, afaik, there so the GUI users could do e.g. this:

• Multiboard - Set value in row 0 column 0 to Hello

instead of this:

• For (Integer A) from 1 to MultiBoardLength do (Actions)
  • Loop - Actions
    • For (Integer B) from 1 to MultiBoardWidth do (Actions)
      • Loop - Actions
        • Multiboard - Set value in row (Integer A) column (Integer B) to Hello

(The GUI syntax might be a bit off, but you should get the idea.)

Ofc. they could've made a couple of changes to that function to make it not loop through all rows and columns when it isn't needed, but then again, will looping harm anyone? I mean a multiboard will never have tens of thousands of rows and columns, so the loop won't be too slow, and it's rather unlikely there is an actual need to update a multiboard's items' values so frequently it would actually cause lag. Optimizing where optimization isn't needed seems useless, don't you think?

 

[ZugZugZealot]

No need, a timer is destroyed an the local variable associated isn't set to null.

You're going to start needing to. I ran a trigger to execute a trigger 50 times with a PolledWait(yes, more than 0 duration), everytime the memory usage would shoot up just for the 50 timers and trigger instances in process, but dropped right back down to where it was once they finished. I ran this test 50 times, making it 2500 times I've fired off PolledWait(again value greater than 0) and no noticeable hit to the memory usage.

You could consider the local variable bug doesn't apply to Blizzard functions.

 

[Xorifelse]

function PolledWait takes real duration returns nothing
    local timer t
    local real  timeRemaining

    if (duration > 0) then
        set t = CreateTimer()
        call TimerStart(t, duration, false, null)
        loop
            set timeRemaining = TimerGetRemaining(t)
            exitwhen timeRemaining <= 0

            // If we have a bit of time left, skip past 10% of the remaining
            // duration instead of checking every interval, to minimize the
            // polling on long waits.
            if (timeRemaining > bj_POLLED_WAIT_SKIP_THRESHOLD) then
                call TriggerSleepAction(0.1 * timeRemaining)
            else
                call TriggerSleepAction(bj_POLLED_WAIT_INTERVAL)
            endif
        endloop
        call DestroyTimer(t)
    endif
endfunction

Could be converted to this:

function test takes nothing returns nothing
    local timer t = CreateTimer()
    call DestroyTimer(t)
endfunction

Now for the leaktest:

function init takes nothing returns nothing
    call TimerStart( CreateTimer(), 0.01, true, function test )
endfunction

Now for the complete thing:

scope meh initializer init
    function test takes nothing returns nothing
        local timer t = CreateTimer()
        call DestroyTimer(t)
    endfunction
    function init takes nothing returns nothing
        call TimerStart( CreateTimer(), 0.01, true, function test )
    endfunction
endscope

Run this code and dare say it doesn't leak, because it does. Perhaps only 1 byte per time, going very slowly up.
Perhaps you don't notice it because of the TriggerSleepAction() which takes even with a value of 0.00, at least 0.02 - 0.04 milliseconds to execute.

 

[ZugZugZealot]

That leaks because it's "user code"

library LeakTest initializer Init
    globals
        trigger test
        integer callCount = 0
    endglobals
    
    private function TestIt takes nothing returns nothing
        call PolledWait(0.01)
    endfunction
    
    private function Something takes nothing returns nothing
        local integer i
        
        set callCount = callCount + 1
        call BJDebugMsg( "Call: " + I2S(callCount))
        
        set i = 0
        loop
            exitwhen i > 500
            call TriggerExecute(test)
            set i = i + 1
        endloop
    endfunction
    
    private function Init takes nothing returns nothing
        local trigger t
        
        set t = CreateTrigger()
        call TriggerRegisterPlayerEvent( t, Player(0), EVENT_PLAYER_END_CINEMATIC )
        call TriggerAddAction( t, function Something )
        
        set test = CreateTrigger()
        call TriggerAddAction(test, function TestIt )
        
        set t = null
    endfunction
endlibrary

You could consider the local variable bug doesn't apply to Blizzard functions.

I've already ran the whole...

function TestLeak takes nothing returns nothing
    local timer t = CreateTimer()
    call DestroyTimer(t)
endfunction

Which did leak... However, using PolledWait() which is a Blizzard function, did not leak.

 

[Troll-Brain]

Expect the shortest TriggerSleepAction possible is about 0.3 s ?
That's why you don't notice the leak in your test ...

 

[ZugZugZealot]

If you look at the function, it creates a timer and destroys a timer regardless of that.

function PolledWait takes real duration returns nothing
    local timer t //Timer local variable
    local real  timeRemaining

    if (duration > 0) then //0.01 IS greater than 0
        set t = CreateTimer() //So a timer get created
        call TimerStart(t, duration, false, null) //Timer starts and let's just go
        loop                                              //it can't do a less than .03
                                                           //sleep. However, a timer is
                                                          //STILL created.
            set timeRemaining = TimerGetRemaining(t)
            exitwhen timeRemaining <= 0

            // If we have a bit of time left, skip past 10% of the remaining
            // duration instead of checking every interval, to minimize the
            // polling on long waits.
            if (timeRemaining > bj_POLLED_WAIT_SKIP_THRESHOLD) then
                call TriggerSleepAction(0.1 * timeRemaining)
            else
                call TriggerSleepAction(bj_POLLED_WAIT_INTERVAL)
            endif
        endloop
        call DestroyTimer(t) //Timer is destroyed
    endif
endfunction //t is never set to null, however no leak.

Also take notice, that I pointed out a temporary spike in the memory usage that went back down.

 

[Xorifelse]

Hopeless, manually pressing enter.. whaha. After this not another word!

library LeakTest initializer Init
    globals
        integer callCount = 0
    endglobals
    
    public function Something takes nothing returns nothing
        set callCount = callCount + 1
        
        if callCount - (callCount / 100) * 100 == 0 then
            call DisplayTextToPlayer( GetLocalPlayer(), 0, 0, I2S( callCount ) )
        endif
        call PolledWait( .01 )
    endfunction
    
    private function Something2 takes nothing returns nothing
    endfunction
    
    private function Init takes nothing returns nothing
        call TimerStart( CreateTimer(), 0.00001, true, function Something )
    endfunction
endlibrary

Using timer callbacks, TriggerSleepAction gets ignored, while the rest of the code still executes.

 

[ZugZugZealot]

It's pressing ESC to run 50 instances, and it allows me to run as many tests as I like.

 

[Troll-Brain]

@ Themis:
You can't use a TriggerSleepAction and ofc neither a PolledWait in a timer callback, and in fact in any callback, call, only inside a trigger action or a function called with ExecuteFunc.

@ZugZugZealot :
Use an handle counter and you will see.
Anyway i'm bored saying facts, don't believe it, i don't care, i don't use myself PolledWait neither TriggerSleepAction anyway.
And the leak handle reference thing is not the main reason, it's just because it's random and inaccurate.