[JASS] Help me improve in using structs

[KaiserG]

Well, I just made another trigger with use of structs, but I feel I'm not that good at it, so I was just wondering if someone could give me some advice if there is something wrong that I could improve with my code. Just to learn a little bit more.

struct WkUp
    integer i
endstruct

function WakeUp takes nothing returns nothing
    local WkUp data = GetCSData(GetExpiredTimer())
    local timer t = GetExpiredTimer()
    local integer i = data.i
    
    call PauseUnit( udg_Survivor[i], false )
    call UnitRemoveAbility( udg_Survivor[i], 'A011' )
    
    //unleak
    set t = null
    call data.destroy()
endfunction

function Trig_ESC_Actions takes nothing returns nothing
    local integer   i           = GetPlayerId(GetTriggerPlayer())+1
    local timer     t           = CreateTimer()
    local WkUp data = WkUp.create()
    
    if ( GetUnitAbilityLevel( udg_Survivor[i], 'A011' ) > 0 ) then
    //struct data
        set data.i = i
        call SetCSData(t, data)
        
        call DisplayTextToPlayer(Player(i-1), 0, 0, "Trying to wake up, please wait some seconds while you wake up from your dreams")
        call TimerStart( t, 10, false, function WakeUp )
    else
        call data.destroy()
    endif
    
    //unleak
    set t = null
endfunction

Also, I would like to do some random questions like if I have to destroy the timers, I mean, do they leak?
And can I use the same data name for more than 1 struct? For example, can I have WkUp data and Ress data without getting errors like the Double Free or something like that?

 

[Berb]

Also, I would like to do some random questions like if I have to destroy the timers, I mean, do they leak?

When you are completely done with a timer, you should be using PauseTimer to stop any problems with the timer iteration and then DestroyTimer to clean the memory.

And can I use the same data name for more than 1 struct?

Not sure what you mean by this.

 

[ProFeT]

The point to get when dealing with structs is to put everything inside the struct itself instead of writting functions that use them, because
structs (or classes in other langages) should be self-sufficient entities.

In your case, you should put all the initialization code into the struct constructor and get rid of the function. You should also store the timer as struct's member and use a static method as callback (i also recommend you to replace the CS library by Vexorian's TimerUtils).

If you need a concrete example, you can take a look at my JCast system's code, I made a frequent use of this pattern inside =)

 

[Berb]

because
structs (or classes in other langages) should be self-sufficient entities.

Not exactly, though you should definitely use struct methods to your advantage.

 

[ProFeT]

I meant "struct in warcraft3" because structs in other langages are only used as data containers, but there are classes to do the nasty work.

In warcraft we rarely need pure data structs because there are often related actions to do with, but in some cases, you right, we still do =)

However, in his example, he definitely needs to know how to use structs as classes, so I tried to put him on the right way

PS: I'll try to write you a little code example later.

 

[Berb]

Well they still don't need to be self-sufficient entities.

 

[ProFeT]

There is no "need" but they "have to" in a OO concept, kinda playing with words I guess..

 

[Berb]

It really depends on your definition of "self sufficient". When I code my structs and such I make sure that I will be able to implement it exactly as I want, and it tends to require several steps - each step is simplified by object oriented code.

For example, this is a very useful element of system that I use but it is not necessarily "self sufficient".

struct abilitydata
    readonly    unit        caster      = null      // the caster/target of the spell that is being
    readonly    unit        target      = null      // used; these are the only two important units.
    
    readonly    real        targetX                 // any important coordinates are saved in the
    readonly    real        targetY                 // following X/Y variables, such as the cast-point
    readonly    real        castpointX              // and the point of the target.
    readonly    real        castpointY
    
    readonly    real        castrange               // these are special components that include the
    readonly    real        castangle               // distance that the spell was cast and the angle
                                                    // at which it was cast.
    readonly    integer     level                 
    readonly    integer     abilityid                 

    // the abilitydata.generate() method will return an instance of abilitydata with automatically
    // defined components, using the event responses from the current trigger. 
    
    // if the .generate static method is not used in the proper event response, then it will not
    // function properly; so DO NOT DO THAT.
    static method generate takes boolean special returns thistype
        local thistype gen=allocate()
        
        set gen.caster = GetTriggerUnit()
        set gen.castpointX = GetUnitX(gen.caster)
        set gen.castpointY = GetUnitY(gen.caster)
        set gen.target = GetSpellTargetUnit()
        set gen.targetX = GetSpellTargetX()
        set gen.targetY = GetSpellTargetY()
        set gen.abilityid = GetSpellAbilityId()
        set gen.level = GetUnitAbilityLevel(gen.caster, gen.abilityid)
        
        // the "special" components are only allocated if the user has specified so with the flag
        // boolean passed to this method.
        if special then
            set gen.castrange = SquareRoot((gen.castpointX-gen.targetX)*(gen.castpointX-gen.targetX)/*
                    */+(gen.castpointY-gen.targetY)*(gen.castpointY-gen.targetY))
            set gen.castangle = Atan2(gen.targetY-gen.castpointY, gen.targetX-gen.castpointX)
        endif
        return gen
    endmethod
endstruct

 

[ProFeT]

Yes, obviously they are not self-sufficient in the way they could be alone and do all the stuff !
What I wanted to explain after looking at his code, is that if you create a timer intrinsically related to an "object", in term of OO programing, you should put in inside the object itself and not use functions that do that instead and bind them together.

Anyway, there is no ultimate-truth and every theory has exceptions or special cases


@KayserG: here is how I would have coded your system.

struct sWakeUp
    
    private timer ti
    private integer playerId
    
    private method onExpiration takes nothing returns nothing
        
        //We are now "inside" the struct's instance so we can use it's members again
        call PauseUnit(udg_Survivor[this.playerId], false)
        call UnitRemoveAbility(udg_Survivor[this.playerId], 'A011')
        
        //Don't forget to destroy the instance (else it will leak!!)
        call this.destroy()
        
    endmethod
    
    
    private static method timer_Callback takes nothing returns nothing
        //Each time a timer expires, this function will be called.
        //The "static" keyword means that the function is shared among the whole struct,
        // thus it doesn't belong specifically to one or another instance.
        //In concrete terms, that means we are not allowed to use struct's members here.
        //
        //Before all, we need to retrieve the instance id binded to the timer, and this
        //can be done with GetTimerData(GetExpiredTimer())
        //
        //Then, we need to convert the retrieved value (it is an integer) into our struct-type,
        //this is done with thistype(...)
        //It could also be done with <structname>(...) but thistype(...) is more convenient
        //since it works with all struct names (it's a shorthand for the current struct name).
        //
        //Finally, we just have to call our struct's expiration method.
        call thistype(GetTimerData(GetExpiredTimer())).onExpiration()
    endmethod
    
    
    static method create takes integer playerId returns thistype
    local thistype this
        
        //Check if we need to create an instance
        if (GetUnitAbilityLevel(udg_Survivor[playerId], 'A011') <= 0) then
            return 0
        endif
        
        //Now allocate a new instance of the struct and store the player id
        set this = thistype.allocate()
        set this.playerId = playerId
       
        //I use here the TimerUtils' method NewTimer() instead of CreateTimer() because it
        //allows timers recycling and avoid some strange bugs.
        set this.ti = NewTimer()
        
        //Then bind the struct instance to the timer (using a TimerUtils' method)
        call SetTimerData(this.ti, this)
        
        //Display the text to the player and start the timer
        call DisplayTextToPlayer(Player(playerId), 0, 0, "Trying to wake up, please wait some seconds while you wake up from your dreams")
        call TimerStart(this.ti, 10., false, function thistype.timer_Callback)

        return this
    endmethod
    
    
endstruct


//Thus, we can "activate" the wakeup for a player with the following line:
call sWakeUp.create( GetPlayerId(GetTriggerPlayer()) )

I hope it helped...