[vjass] Can this be improved?

[Xorifelse]

I'm still trying to improve the struct found in this topic, but I'm not sure if the code is efficient.

library Effects requires HandleVars

    struct sfx
    
        timer   t = CreateTimer()
        string  m
        real    x
        real    y
        real    d
        
        method onDestroy takes nothing returns nothing
            set .t = null
            set .m = null
            set .x = 0
            set .y = 0
            set .d = 0
        endmethod
        
        static method CallBack takes nothing returns nothing
          local sfx a = GetHandleInteger( GetExpiredTimer(), "i" )
            call DestroyEffect( AddSpecialEffect( a.m, GetRandomReal( a.x - a.d, a.x + a.d ), GetRandomReal( a.y - a.d, a.y + a.d ) ) )
        endmethod
        
        method Stop takes nothing returns nothing
            call PauseTimer( .t )
            call DestroyTimer( .t )
            call FlushHandleInteger( .t, "i" )
        endmethod
        
        method Pause takes nothing returns nothing
            call PauseTimer( .t )
        endmethod
        
        method Start takes real timeout, string m, real x, real y, real d returns nothing
          local sfx   a = sfx.create()
            
            set a.m = m
            set a.x = x
            set a.y = y
            set a.d = d
            
            call SetHandleInteger( .t, "i", a )
            call TimerStart( .t, timeout, true, function sfx.CallBack )
        endmethod
        
    endstruct

endlibrary

scope test initializer init

    function Actions takes nothing returns nothing
        local sfx a = sfx.create()
        local sfx b = sfx.create()
        
        call FogModifierStart( CreateFogModifierRect( Player( 0 ), FOG_OF_WAR_VISIBLE, GetWorldBounds(), true, true ) )
        
        call a.Start( 0.2, "Abilities\\Spells\\Human\\SpellSteal\\SpellStealMissile.mdl", -650, -900, 300 )
        call b.Start( 1, "Abilities\\Weapons\\Bolt\\BoltImpact.mdl", 650, 350, 500 )
    endfunction

    private function init takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerAddAction( t, function Actions )
        call TriggerExecute( t )
        
        set t = null
    endfunction

endscope

The thing is, I know jass very well and looking at this code it looks very efficient to me.
But I do not know the limitations or boundary's of vjass.
I've heard from a friend that i should use something called a timerObject library, but would it be more efficient?

 

[Romek]

Can't you put the entire thing into 1 library?

        method onDestroy takes nothing returns nothing
            set .t = null
            set .m = null
            set .x = 0
            set .y = 0
            set .d = 0
        endmethod

Globals don't need to be nulled.
And reals/integers don't need to be nulled, even when local.

private function init takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerAddAction( t, function Actions )
        call TriggerExecute( t )
        
        set t = null
    endfunction

Can't you just do:

private function init takes nothing returns nothing
        call Actions()
    endfunction

 

[Xorifelse]

Can't you put the entire thing into 1 library?
Globals don't need to be nulled.
And reals/integers don't need to be nulled, even when local.

Now I do understand that nothing has to be nulled, since it doesn't leak because you will always have the reference to access it again.
I do it for another reason, since I personally want to use as little memory as possible.

This struct may be called perhaps 50 times in my map, so.. it doesn't actually recycle the globals.
So the entire game, the globals will have there values and therefor useless memory is consumed.

Correct me if I'm wrong though.

>Can't you just do:

Yes I can, but that was just a testing trigger.
Don't worry about that

 

[Flare]

This struct may be called perhaps 50 times in my map, so.. it doesn't actually recycle the globals.
So the entire game, the globals will have there values and therefor useless memory is consumed.

Structs do recycle their indexes (when destroyed)

local MyStruct a = MyStruct.create ()
call BJDebugMsg (I2S (a))
call a.destroy ()
set a = MyStruct.create ()
call BJDebugMsg (I2S (a))
call a.destroy ()

That should display 1 twice (assuming no previous instances of MyStruct still exist)

And you don't really need to have the timer has a struct member (which have an array of timers that may never be used when you can just create them as you need them) - you can pass a timer argument to your Stop method, and do

call PauseTimer (whichTimer)
call DestroyTimer (whichTimer) //Although timer recycling would be preferable to creating/destroying, look for Vexorian's TimerUtils over at WC3Campaigns
call FlushHandleInteger (whichTimer, "i")

 

[Xorifelse]

local MyStruct a = MyStruct.create ()
call BJDebugMsg (I2S (a))
call a.destroy ()
set a = MyStruct.create ()
call BJDebugMsg (I2S (a))
call a.destroy ()

But for example, if I do..

local structure a = structure.create()
local structure b = structure.create()
local structure c = structure.create()

call b.destroy()
set b = structure.create()

 // Some code..

call c.destroy()
call a.destroy()

Wouldn't that disrupt the values containing of c?

 

[Flare]

No.

Let's say we start at 1 i.e.
a=1
b=2
c=3

123

b is destroyed
1#3

b is created again
123

c is destroyed
12#

a is destroyed
#2#

If you destroyed all 3, then recreated them in a different order, stuff might get mixed up, but you are setting the members to a definite value with your create method so it won't really matter what value it was before, since it's probably gonna be different in the .create (...) call

New structs use the first available index, as far as I know (I don't really know how the allocate method works, hopefully Vexorian will elaborate)

Just add a BJDebugMsg ("<Struct variable> is equal to " + I2S (structVar)) and see what is displayed

(NOTE: For all I know, I could be completely wrong )

 

[emjlr3]

as soon as a saw "requiers handle vars" i stopped reading, that should be your first improvement

 

[Romek]

as soon as a saw "requiers handle vars" i stopped reading, that should be your first improvement

They're not THAT bad are they?
They're just slightly slower than other systems, although in return, they have no limit.

 

[Forty]

why does everybody dislike handle vars? i mean.... dota uses gamecache, and does dota have any problems with lagg or performance?

 

[Flare]

It uses I2H which is unsafe

 

[Forty]

did you ever have problems with it. or in dota? and where exactly does HV use I2H(i)?

 

[Flare]

did you ever have problems with it. or in dota? and where exactly does HV use I2H(i)?

1) I've never used LHV before, and have no intention of ever doing so
2) DotA isn't really a very good example to go by as regards exceptional coding - from alot of what I've seen over at DotA forum, there are a number of leaks, and it's still using I2H-based stuff even when vJASS is available
3)

function GetHandleHandle takes handle subject, string name returns handle
    return GetStoredInteger(LocalVars(), I2S(H2I(subject)), name)
    return null
endfunction

That's one

function GetHandleUnit takes handle subject, string name returns unit
    return GetStoredInteger(LocalVars(), I2S(H2I(subject)), name)
    return null
endfunction
function GetHandleTimer takes handle subject, string name returns timer
    return GetStoredInteger(LocalVars(), I2S(H2I(subject)), name)
    return null
endfunction
function GetHandleTrigger takes handle subject, string name returns trigger
    return GetStoredInteger(LocalVars(), I2S(H2I(subject)), name)
    return null
endfunction
function GetHandleEffect takes handle subject, string name returns effect
    return GetStoredInteger(LocalVars(), I2S(H2I(subject)), name)
    return null
endfunction
function GetHandleGroup takes handle subject, string name returns group
    return GetStoredInteger(LocalVars(), I2S(H2I(subject)), name)
    return null
endfunction
function GetHandleLightning takes handle subject, string name returns lightning
    return GetStoredInteger(LocalVars(), I2S(H2I(subject)), name)
    return null
endfunction
function GetHandleWidget takes handle subject, string name returns widget
    return GetStoredInteger(LocalVars(), I2S(H2I(subject)), name)
    return null
endfunction

and a few more, just for good measure

There's a thread over at WC3C about how/why it's bad, if only I could find it...

EDIT: Here it is

 

[TheDamien]

did you ever have problems with it. or in dota?

It is enough that it is known to be buggy. There are circumstances under which I2H functions are probably safe, but why risk it? Although you are not using any of the unsafe functions in your code, HandleVars is still outdated and also inferior to the alternatives. Go with something like CSData instead.

and where exactly does HV use I2H(i)?

Here, for example:

function GetHandleUnit takes handle subject, string name returns unit
    return GetStoredInteger(LocalVars(), I2S(H2I(subject)), name)
    return null
endfunction

 

[Forty]

H2I is not equal to I2H?

I2S neither.

so where is your I2H? i dont get it

 

[Flare]

H2I is not equal to I2H?

There's only 1 kind of integer, so H2I won't return a real (for example) when called

But, there's many different kinds of handles (and all have different ID's) so what's stopping you from retrieving a timer instead of a unit? Both are handles, both have handle ID's, they are both valid returns. Then, what happens if you use the timer for a unit function e.g. KillUnit - how can you kill a timer with KillUnit? And that's where I2H falls down - there's no guarantee that you're going to retrieve the specific type (even if you be more specific and do "returns unit" or something)

so where is your I2H? i dont get it

//The function is designed to return a unit
function GetHandleUnit takes handle subject, string name returns unit
//But wait, what's this!? We are returning an integer, even though we are supposed to return a unit!
//And from here, you just hope that you have got the right type...
return GetStoredInteger(LocalVars(), I2S(H2I(subject)), name)
return null
endfunction

 

[Forty]

okay then just use GetHandleInteger which will not cause any problems and will use structs. and again, i dont see a problem

i mean, if you are not an idiot, your map will not crash.

 

[Flare]

okay then just use GetHandleInteger which will not cause any problems and will use structs

Well, if you were going to use a cache-based system, you'd be better off with CSSafeCache (since it removes all access to Get/SetHandleHandle-esque functions, to get rid of any temptations, and it's vJASS so you don't have to do anything other than CnP the code)

if you are not an idiot

We may not be idiots, but there are always people who will chance their arm, just for the hell of it...

 

[emjlr3]

1. its slower then anything else
2. it can lead to massive bugs
3. there are much better options available - which don't bug and which are much faster and easier to use

do you really need any more reasons to to use it?