[vJass] how does this happen?

03-07-2011, 02:20 PM

I've tried to make a library to fade in and out units. It works so far, but if the first instance is done, all other instances are also stopped. Why is that?

JASS:

library fade

globals
    private constant real INTERVAL = 0.03
    private Fade array Inst
    private integer i = 0
endglobals

struct Fade
    unit u
    real visibility
    real step
    real togo

    static method create takes unit whichunit, real time, real start, real end returns Fade
    local Fade herp = Fade.allocate()
        set herp.step = -(start-end)/(time/INTERVAL)
        set herp.visibility = start
        set herp.u = whichunit
        set herp.togo = time/INTERVAL
        return herp
    endmethod
    
    method onDestroy takes nothing returns nothing
        set .step = 0.
        set .visibility = 0
        set .u = null
        set .togo = 0.
    endmethod
endstruct

function update takes nothing returns nothing
    local integer s = 1
    local Fade durp
    local unit u
    local real st
    local timer ti = GetExpiredTimer()
    loop
        set durp = Inst[s]
        set durp.togo = durp.togo - 1.
        if durp.togo < 0. then
            if i == 1 then
                call DisplayTextToPlayer(Player(0), 0., 0., "Exiting.")
                call PauseTimer(ti)
            else
                set Inst[s] = Inst[i]
                call Inst[i].destroy()
                set s = s - 1
                set i = i - 1
            endif
        else
            set u = durp.u
            set st = durp.step + durp.visibility
            call SetUnitVertexColor(u, 100, 100, 100, R2I(st))
            set durp.visibility = st
        endif
        set s = s + 1
        exitwhen s > i
    endloop
endfunction

function IFade takes unit u, real t, real s, real e returns nothing
    local timer tim = CreateTimer()
    set i = i + 1
    set Inst[i] = Fade.create(u, t, s, e)
    if i == 1 then
        call TimerStart(tim, INTERVAL, true, function update)
    endif
endfunction

endlibrary

Also, I should mention that I am still a newb when it comes to vJass.

03-08-2011, 09:55 AM

Fireeye

It have been quite some time since i last checked code.
So i may make some errors too.
Let's start with IFade(...)
If you do it this way, you waste the slot [0], which cause the bug you described, but i'm gonna get there later.
Just switch the lines and you'll use slot[0] too.

JASS:

    set i = i + 1
    set Inst[i] = Fade.create(u, t, s, e)

Let's head to the real "problems" now.

JASS:

function update takes nothing returns nothing
    local integer s = 1

As we're using slot[0] now, change it "local integer s = 0

JASS:

    local Fade durp
    local unit u
    local real st
    local timer ti = GetExpiredTimer()
    loop
        set durp = Inst[s]
        set durp.togo = durp.togo - 1.
        if durp.togo < 0. then
            if i == 1 then
                call Inst[i].destroy()
                call DisplayTextToPlayer(Player(0), 0., 0., "Exiting.")
                call PauseTimer(ti)
            else
                set Inst[s] = Inst[i]
                call Inst[i].destroy()
                set s = s - 1
                set i = i - 1

Alright, this is quite the trouble maker atm, basically what happens.
You reduce the counter for the loop AND instances. Furthermore you set the current instance equal a later one and destroy it, which if i remeber correct also destroy the struct for the loopslot as both are refering to the same struct.
Thus, first we destroy the current instance, reduce the instance the counter and set the current instance equal to the n-th instance.
Also we don't need to mess around with the loop counter.

JASS:

            endif
        else
            set u = durp.u
            set st = durp.step + durp.visibility
            call SetUnitVertexColor(u, 100, 100, 100, R2I(st))
            set durp.visibility = st
        endif
        set s = s + 1
        exitwhen s > i

As the previous loop counter decrease got removed, move the loop counter increase into the the else branch.
You also want to change the exitwhen condition to s>=i, as it is the slot number of the first un-used slot.

JASS:

    endloop
endfunction

Long explanation, short reason. I'm gonna make some "drawings".

Code:

INSTANCE COUNTER = 4
LOOP COUNTER = 1
[NOT USED][USED] [USED] [USED]

First instance gets destroyed
INSTANCE COUNTER = 3
LOOP COUNTER = 1
Slot 4 gets copied to slot 1, loop counter stays the same.
[NOT USED] [NOT USED] [USED] [NOT USED]
The loop counter points to a not used slot now. It will keep destroying the structs till the instance counter hits 1 and pause the timer then.

Here's a version which should work. But as said, i didn't write any code for some time already.

JASS:

library fade

globals
    private constant real INTERVAL = 0.03
    private Fade array Inst
    private integer i = 0
endglobals

struct Fade
    unit u
    real visibility
    real step
    real togo

    static method create takes unit whichunit, real time, real start, real end returns Fade
    local Fade herp = Fade.allocate()
        set herp.step = -(start-end)/(time/INTERVAL)
        set herp.visibility = start
        set herp.u = whichunit
        set herp.togo = time/INTERVAL
        return herp
    endmethod
    
    method onDestroy takes nothing returns nothing
        set .step = 0.
        set .visibility = 0
        set .u = null
        set .togo = 0.
    endmethod
endstruct

function update takes nothing returns nothing
    local integer s = 0
    local Fade durp
    local unit u
    local real st
    local timer ti = GetExpiredTimer()
    loop
        set durp = Inst[s]
        set durp.togo = durp.togo - 1.
        if durp.togo < 0. then
            if i == 0 then
                call DisplayTextToPlayer(Player(0), 0., 0., "Exiting.")
                call PauseTimer(ti)
            else
                call Inst[s].destroy()
                set i = i - 1
                set Inst[s] = Inst[i]
            endif
        else
            set u = durp.u
            set st = durp.step + durp.visibility
            call SetUnitVertexColor(u, 100, 100, 100, R2I(st))
            set durp.visibility = st
            set s = s + 1
        endif
        exitwhen s >= i
    endloop
endfunction

function IFade takes unit u, real t, real s, real e returns nothing
    local timer tim = CreateTimer()
    set Inst[i] = Fade.create(u, t, s, e)
    set i = i + 1
    if i == 1 then
        call TimerStart(tim, INTERVAL, true, function update)
    endif
endfunction

endlibrary

Meh, wrote some crap with slot 0, fixed.

03-08-2011, 02:55 PM	#3
Idontneedaname	wow... +Rep given!

03-10-2011, 11:44 AM	#4
Anitarf	You should just use a global timer which you create at map initialization instead of creating a new timer each time you need one (right now, you even create new timers when you don't need them).

03-26-2011, 01:09 PM	#5
Bribe	Some things I notice even in the "fixed" version (to add to what has already been mentioned): 1. Timer isn't nulled. In either function. 2. GetExpiredTimer() is stored into a local variable...rather uselessly. 3. Timer never gets destroyed. Only paused. 4. No timer recycling either. All these things support what Anitarf said: use a single global timer.