• 🏆 Texturing Contest #33 is OPEN! Contestants must re-texture a SD unit model found in-game (Warcraft 3 Classic), recreating the unit into a peaceful NPC version. 🔗Click here to enter!
  • It's time for the first HD Modeling Contest of 2024. Join the theme discussion for Hive's HD Modeling Contest #6! Click here to post your idea!

[Snippet] TemporaryHeroAttribute

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
TemporaryHeroAttribute

Preface:

I don't know if any system exists which does this, but if there is a public one, I doubt it's this simple/efficient.

I've created this to aid the creation of a spell which I'm helping create for a fellow hive user. If anyone has a request/bug report, I'd be happy to update it.

Pre-requisites:

The script requires no other libraries to function, but as it is written in vJass, it DOES require JassHelper (Though I recommend just compiling with JNGP).

API:


JASS:
integer AGILITY
integer STRENGTH
integer INTELLIGENCE
JASS:
TemporaryHeroAttribute_AGILITY
TemporaryHeroAttribute_STRENGTH
TemporaryHeroAttribute_INTELLIGENCE
These constants are word representations of ascii integers, 'A', 'S', and 'I', that correspond to the three stat types in warcraft 3. Strictly speaking, they let the client use the stat types within the system (including in the 'add' function) if they prefer to avoid this ascii codes.

real TIMER_FIDELITY
This is a constant that controls how often a global timer refreshes to update any outstanding stat modifications. This can be as accurate or blunt as you like it, but generally I suggest values in the range [1./60. , 1./2.]



function add takes unit hero, integer attribute, integer value, real duration returns nothing

TemporaryHeroAttribute_add(<hero>,('A' | 'S' | 'I'),<positve or negative stat modifier>,<real positive duration>)

This is the only function in the snippet. Use it to temporarily modify a hero stat. Put simply, it will receive <value> <attribute> for <duration> seconds. The ASCII codes 'A', 'S', and 'I' are interchangeable with "tempoaryHeroAttribute_AGILITY" as well as the same with strength and intelligence.


The script:


JASS:
library TemporaryHeroAttribute
    private struct attribDat
        integer attribute
        integer value
        real duration
        unit hero
    endstruct
    
    globals
        public constant integer AGILITY='A'
        public constant integer STRENGTH='S'
        public constant integer INTELLIGENCE='I'
        private constant real TIMER_FIDELITY=1./10.
        private attribDat array attribDB
        private integer dbIndex=-1
        private timer time=CreateTimer()
    endglobals
    
    private function p takes nothing returns nothing
        local attribDat tempDat
        local integer index=0
        loop
            exitwhen index>dbIndex
            set tempDat=attribDB[index]
            set tempDat.duration=tempDat.duration-TIMER_FIDELITY
            if tempDat.duration<=0. then
                if tempDat.attribute==AGILITY then
                    call SetHeroAgi(tempDat.hero,GetHeroAgi(tempDat.hero,false)-tempDat.value,true)
                elseif tempDat.attribute==STRENGTH then
                    call SetHeroStr(tempDat.hero,GetHeroStr(tempDat.hero,false)-tempDat.value,true)
                elseif tempDat.attribute==INTELLIGENCE then
                    call SetHeroInt(tempDat.hero,GetHeroInt(tempDat.hero,false)-tempDat.value,true)
                endif
                call tempDat.destroy()
                set attribDB[index]=attribDB[dbIndex]
                set dbIndex=dbIndex-1
                set index=index-1
                if dbIndex==-1 then
                    call PauseTimer(time)
                endif
            endif
            set index=index+1
        endloop
    endfunction
    
    public function add takes unit hero, integer attribute, integer value, real duration returns nothing
        local attribDat tempDat
        if attribute==AGILITY then
            call SetHeroAgi(hero,GetHeroAgi(hero,false)+value,true)
        elseif attribute==STRENGTH then
            call SetHeroStr(hero,GetHeroStr(hero,false)+value,true)
        elseif attribute==INTELLIGENCE then
            call SetHeroInt(hero,GetHeroInt(hero,false)+value,true)
        else
            debug call BJDebugMsg("|cff990000tempModifyHeroAttrib:|r Unrecognized attribute integer!")
            return
        endif
        set tempDat=attribDat.create()
        set tempDat.attribute=attribute
        set tempDat.value=value
        set tempDat.duration=duration
        set tempDat.hero=hero
        set dbIndex=dbIndex+1
        set attribDB[dbIndex]=tempDat
        if dbIndex==0 then
            call TimerStart(time,TIMER_FIDELITY,true,function p)
        endif
    endfunction
endlibrary


Change Log:

2012.06.09 The script no longer creates an instance if a non-attribute is specified.
2012.06.01 #2 Updated the duration check to <= just because.
2012.06.01 Updated CamelCase and fixed a decrement bug.
2012.05.31 Changed one line "debug else". Very minor edit.
2012.05.30 #2 - First update: Changed the name slightly and made the script also work with ASCII representations ('A', 'S', and 'I'). Additionally, debug messages use the debug keyword.
2012.05.30 - Initial submission to Hive: Jass Resources: Submissions

Special Thanks:

  • Bribe for suggesting the use of ASCII conversion.
  • Magtheridon96 for pointing out a decrement bug in my stack logic.
  • -Kobas- for creating a map submission template, which turned out useful for system submissions as well.
  • Vexorian for developing JassHelper. Without vJass, I almost certainly would not still be scripting for wc3.
  • The various developers of JNGP including PitzerMike and MindworX. Integrating JassHelper and TESH is a godsend.

Author's Notes:

I understand this is a simple system, but it should anyway be very useful, if not as a snippet, at least for a learning resource. This is an excellent introduction to a linear stack if you're at "that stage" of learning vJass.

Pastebin Mirror (Newest First):


http://pastebin.com/6BfRedJy
http://pastebin.com/GZ8QQ0tV
http://pastebin.com/iWB2kLAy
http://pastebin.com/gU5qTu2L
http://pastebin.com/XebbjbQG
http://pastebin.com/7sVTsvWS
 
Last edited:
Level 10
Joined
May 27, 2009
Messages
494
hmm a much cooler function name?
Like AddTemporaryAttribute
or ModifyAttributeTimed

or something like that o.o

perhaps use table? or extend the struct as an array.. just to remove unused methods (allocate/destroy)

oh and make the error message debug-only if possible...
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,456
Library name can use some work to be more intuitive as does your other submission. LibraryNamesAreUsuallyLikeThis notLikeThis.

I agree with jim7777 that the name could be shortened as well.

Instead of constant integers to give a face to those nameless integers, you could do the ASCII trick and pass 'A' 'S' 'I' for those respective attributes. That's intuitive enough, definitely more than tempModifyHeroAttrib_AGILITY/STRENGTH/INTELLIGENCE

JASS:
                if tempDat.attribute==@'A'@ then
                    call SetHeroAgi(tempDat.hero,GetHeroAgi(tempDat.hero,false)-tempDat.value,true)
                elseif tempDat.attribute==@'S'@ then
                    call SetHeroStr(tempDat.hero,GetHeroStr(tempDat.hero,false)-tempDat.value,true)
                elseif tempDat.attribute==@'I'@ then
                    call SetHeroInt(tempDat.hero,GetHeroInt(tempDat.hero,false)-tempDat.value,true)
                else
                    @debug @call BJDebugMsg(ERROR_MESSAGE)
                endif

Good idea for a resource. I can think of ways it will be useful.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
Thanks for the nice comments, guys! I like the idea for using the ASCII trick. I'll probably include that in the next version.

extend the struct as an array.. just to remove unused methods (allocate/destroy)

Can you explain this a bit more to me? I do use the .destroy() method.

debug call BJDebugMsg(ERROR_MESSAGE)

Also never used this. All I do is add the debug keyword and boom it's done? Or do I have to define some debug constant somewhere?

Regarding library name, how about:

temporaryHeroAttribute ? I like trueCamelCase because it matches all my other scripts =/

---

Edit: I've updated the script. "extends array" is still a big question mark for me though.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
The 'extends array' thing would just make you write the allocation/deallocation methods yourself because the way JassHelper does it is a bit uglier than the way we'd usually do it.

It's fine though, that change is totally optional.

Ah okay, I'll have to take a look at how some other people have done that. Perhaps for my next system or something.

I like it. Nice job, pretty useful for easy spell making.

Woot! Thanks mate ^^
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
yes, it's just optional but since the data are just being replaced overtime.. allocate/destroy aren't that much useful...

oh and
JASS:
@else@
            debug call BJDebugMsg(ERROR_MESSAGE)
to
JASS:
@debug else@
            debug call BJDebugMsg(ERROR_MESSAGE)

I've updated the script to reflect that "debug else" line.

If I use "extends array" I have to re-write .create() and .destroy() though, don't I? I need to look at some examples of how other people have done this. For now it stays the way it is I guess.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Not something directly related to the code itself (coz i'm too lazy to check it, and i suppose Bribe and Mag did it, so it should be ok) :

- a library name which doesn't use the CamelCase syntax is a bit weird imho.

Also what about a shorter name, something like TempHeroAttrib(ute).
Temp is pretty common, it obvioulsy means temporary.

I would prefer TempHeroAttrib.add rather than TempHeroAttrib_add.
Ofc it's exactly the same in the end.
And i won't argue with you about that, just saying my taste.

The reason is simple i reserve this_syntax for variables (constant ones already use it with full uppercase letters), and anyway i find a bit ugly for functions, i think Zinc uses the public attribute in the good way.
And it doesn't really require much effort for you, since you already use a struct.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
Not something directly related to the code itself (coz i'm too lazy to check it, and i suppose Bribe and Mag did it, so it should be ok) :

- a library name which doesn't use the CamelCase syntax is a bit weird imho.

Aghh it's going to take ages to get in that habit. I'll make sure my next submission follows suit.

Also what about a shorter name, something like TempHeroAttrib(ute).
Temp is pretty common, it obvioulsy means temporary.

I just changed the name =/ The issue is that there's no short way that's also descriptive enough to explain what it does. You can't remove "hero" from the name because "attribute" can mean a lot of things in wc3.

I would prefer TempHeroAttrib.add rather than TempHeroAttrib_add.
Ofc it's exactly the same in the end.
And i won't argue with you about that, just saying my taste.

I agree it looks better, but generally I actually don't use methods in my libraries. structuredDD was an exception because I made it while planning from the start to submit it.

The reason is simple i reserve this_syntax for variables (constant ones already use it with full uppercase letters), and anyway i find a bit ugly for functions, i think Zinc uses the public attribute in the good way.
And it doesn't really require much effort for you, since you already use a struct.

If I update the script to reflect something more important, I'll consider changing this as well.

Thanks for the comments.
 
Oh, I forgot to actually review the algorithm :p

Seems robust and fast since you're using a stack, there's just this one thing you could do:

if tempDat.duration<0. then
->
if tempDat.duration<=0. then

:p

edit
I wrote my own version:

JASS:
library AddHeroAttribute
    
    globals
        constant integer HERO_STAT_AGI = 'A'
        constant integer HERO_STAT_STR = 'S'
        constant integer HERO_STAT_INT = 'I'
        
        private constant real TIMEOUT = 0.03125
    endglobals
    
    private struct AttributeData extends array
        private static integer ic = 0
        private static integer array rn
        
        private static integer array next
        private static integer array prev
        
        private static timer time = CreateTimer()
        
        private static integer array stat
        private static integer array amount
        private static real array duration
        private static unit array hero
        
        private static method iterate takes nothing returns nothing
            local thistype this = next[0]
            loop
                exitwhen this == 0
                
                set duration[this] = duration[this] - TIMEOUT
                
                if duration[this] <= 0 then
                    if stat[this] == 'A' then
                        call SetHeroAgi(hero[this], GetHeroAgi(hero[this], false) - amount[this], true)
                    elseif stat[this] == 'I' then
                        call SetHeroInt(hero[this], GetHeroInt(hero[this], false) - amount[this], true)
                    elseif stat[this] == 'S' then
                        call SetHeroStr(hero[this], GetHeroStr(hero[this], false) - amount[this], true)
                    endif
                    
                    set hero[this] = null
            
                    set next[prev[this]] = next[this]
                    set prev[next[this]] = prev[this]
                    
                    set rn[this] = rn[0]
                    set rn[0] = this
                    
                    if next[0] == 0 then
                        call PauseTimer(time)
                    endif
                endif
                
                set this = next[this]
            endloop
        endmethod
        
        static method create takes unit u, integer typ, integer howMuch, real dur returns thistype
            local thistype this = rn[0]
            
            if this == 0 then
                set ic = ic + 1
                set this = ic
            else
                set rn[0] = rn[this]
            endif
            
            set next[this] = 0
            set prev[this] = prev[0]
            set next[prev[0]] = this
            set prev[0] = this
            
            set stat[this] = typ
            set amount[this] = howMuch
            set hero[this] = u
            set duration[this] = dur
            
            if typ == 'A' then
                call SetHeroAgi(u, GetHeroAgi(u, false) + howMuch, true)
            elseif typ == 'I' then
                call SetHeroInt(u, GetHeroInt(u, false) + howMuch, true)
            elseif typ == 'S' then
                call SetHeroStr(u, GetHeroStr(u, false) + howMuch, true)
            debug else
                debug call DisplayTimedTextToPlayer(GetLocalPlayer(), 0, 0, 60, "Please delete Warcraft III you noob. Lol, don't cry, go play Tetris.")
            endif
            
            if next[0] == this then
                call TimerStart(time, TIMEOUT, true, function thistype.iterate)
            endif
            
            return this
        endmethod
    endstruct
    
    function AddHeroAttribute takes unit hero, integer attribute, integer value, real duration returns nothing
        call AttributeData.create(hero, attribute, value, duration)
    endfunction
endlibrary

And compared it to yours.
They use the same algorithms, except yours uses a stack which is a bit faster.

I'm using a list here because I want to avoid that side effect that could cause us to skip the iteration
of the last instance in the stack when destroying. This is no big deal though, but it might be in cases where you have
abilities that use very tiny durations :O

The fix is fairly easy though, just subtract 1 from the index after destroying so that you could iterate over the last instance
in the list that you placed in the place of the one you just destroyed. That way, you'll never end up with inaccurate durations :D
 
Last edited:

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
Oh, I forgot to actually review the algorithm :p

Seems robust and fast since you're using a stack, there's just this one thing you could do:

if tempDat.duration<0. then
->
if tempDat.duration<=0. then

:p

Are you sure that's better? How precise does jass handle the equality of double precision numbers?

edit
I wrote my own version:

JASS:
library AddHeroAttribute
    
    globals
        constant integer HERO_STAT_AGI = 'A'
        constant integer HERO_STAT_STR = 'S'
        constant integer HERO_STAT_INT = 'I'
        
        private constant real TIMEOUT = 0.03125
    endglobals
    
    private struct AttributeData extends array
        private static integer ic = 0
        private static integer array rn
        
        private static integer array next
        private static integer array prev
        
        private static timer time = CreateTimer()
        
        private static integer array stat
        private static integer array amount
        private static real array duration
        private static unit array hero
        
        private static method iterate takes nothing returns nothing
            local thistype this = next[0]
            loop
                exitwhen this == 0
                
                set duration[this] = duration[this] - TIMEOUT
                
                if duration[this] <= 0 then
                    if stat[this] == 'A' then
                        call SetHeroAgi(hero[this], GetHeroAgi(hero[this], false) - amount[this], true)
                    elseif stat[this] == 'I' then
                        call SetHeroInt(hero[this], GetHeroInt(hero[this], false) - amount[this], true)
                    elseif stat[this] == 'S' then
                        call SetHeroStr(hero[this], GetHeroStr(hero[this], false) - amount[this], true)
                    endif
                    
                    set hero[this] = null
            
                    set next[prev[this]] = next[this]
                    set prev[next[this]] = prev[this]
                    
                    set rn[this] = rn[0]
                    set rn[0] = this
                    
                    if next[0] == 0 then
                        call PauseTimer(time)
                    endif
                endif
                
                set this = next[this]
            endloop
        endmethod
        
        static method create takes unit u, integer typ, integer howMuch, real dur returns thistype
            local thistype this = rn[0]
            
            if this == 0 then
                set ic = ic + 1
                set this = ic
            else
                set rn[0] = rn[this]
            endif
            
            set next[this] = 0
            set prev[this] = prev[0]
            set next[prev[0]] = this
            set prev[0] = this
            
            set stat[this] = typ
            set amount[this] = howMuch
            set hero[this] = u
            set duration[this] = dur
            
            if typ == 'A' then
                call SetHeroAgi(u, GetHeroAgi(u, false) + howMuch, true)
            elseif typ == 'I' then
                call SetHeroInt(u, GetHeroInt(u, false) + howMuch, true)
            elseif typ == 'S' then
                call SetHeroStr(u, GetHeroStr(u, false) + howMuch, true)
            debug else
                debug call DisplayTimedTextToPlayer(GetLocalPlayer(), 0, 0, 60, "Please delete Warcraft III you noob. Lol, don't cry, go play Tetris.")
            endif
            
            if next[0] == this then
                call TimerStart(time, TIMEOUT, true, function thistype.iterate)
            endif
            
            return this
        endmethod
    endstruct
    
    function AddHeroAttribute takes unit hero, integer attribute, integer value, real duration returns nothing
        call AttributeData.create(hero, attribute, value, duration)
    endfunction
endlibrary

And compared it to yours.
They use the same algorithms, except yours uses a stack which is a bit faster.

I have such a hard time understanding this. Can you explain this line to me?

set this = next[this]

I'm using a list here because I want to avoid that side effect that could cause us to skip the iteration
of the last instance in the stack when destroying. This is no big deal though, but it might be in cases where you have
abilities that use very tiny durations :O

The fix is fairly easy though, just subtract 1 from the index after destroying so that you could iterate over the last instance
in the list that you placed in the place of the one you just destroyed. That way, you'll never end up with inaccurate durations :D

Whoops, thanks! I always seem to forget this when I'm writing these. I'll probably have to fix the same bug now in structuredDD.

I've updated the script to reflect that change.
 
Are you sure that's better? How precise does jass handle the equality of double precision numbers?

Well, I'm not very sure, but reals in JASS are pretty unreliable in my experience (Or other people's experiences that I learn about ;P)

Either way, it's just a bit better because the real /might/ sometimes be exactly 0 :D

Oh and about this:
set this = next[this]

This means that I'm going to the next instance in the list.
I'm using this as the current instance in the loop.
By setting this to next[this], I'm setting the current instance to the next of the current one.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
AFAIR "==" acts like there is an epsilon value about 0.001, or maybe 0.01, i don't remember the exact value.

While there is no epsilon value for the other operators, excepted maybe " != ", i have not tested it.

For example :

5.99 == 6.00 -> true
5.99 <= 6.00 and 5.99 >= 6.0 -> false (<= and >= is like an == without any epsilon value)
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
Well, I'm not very sure, but reals in JASS are pretty unreliable in my experience (Or other people's experiences that I learn about ;P)

Either way, it's just a bit better because the real /might/ sometimes be exactly 0 :D

Oh and about this:
set this = next[this]

This means that I'm going to the next instance in the list.
I'm using this as the current instance in the loop.
By setting this to next[this], I'm setting the current instance to the next of the current one.

Yeah I'm still mind=blown by that script.

JASS:
            set next[this] = 0
            set prev[this] = prev[0]
            set next[prev[0]] = this
            set prev[0] = this

Seriously? Only someone on drugs could next this prev zero this so much -_-

AFAIR "==" acts like there is an epsilon value about 0.001, or maybe 0.01, i don't remember the exact value.

While there is no epsilon value for the other operators, excepted maybe " != ", i have not tested it.

For example :

5.99 == 6.00 -> true
5.99 <= 6.00 and 5.99 >= 6.0 -> false (<= and >= is like an == without any epsilon value)

Good to know. So I should use <= for the duration check, right?
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
With or without this epsilon value it makes sense to use "<= 0" rather than just "< 0" anyway.

I've read the code, the timer periodic seems overkill, and innacurate, why not just use a new timer each time here, just to avoid a requirement such as TimerUtils ?

Also you should really consider a shorter name, such as TempHeroAttribs.
For me at least the actual name is boring.

Finally how the Set/Get... native functions handle the bonus given with an ability (through an item or not) ?

EDIT : Ofc when i'm saying "without any epsilon value", reals by themselves have not an infinite accuracy, but the hell i know if you can have something reliable about the max precision you can have.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
With or without this epsilon value it makes sense to use "<= 0" rather than just "< 0" anyway.

Maybe I'm being "that guy" again, but isn't the < operator faster than <= ? I've just updated it regardless, 2 votes is enough for me.

I've read the code, the timer periodic seems overkill, and innacurate, why not just use a new timer each time here, just to avoid a requirement such as TimerUtils ?

Why would you need a system like this to be more accurate? Wc3's health fountains aren't even accurate to a degree of .5 seconds, let alone the default value of .1 on this system.

If it were my map, I'd use a fidelity value of 1./30. because it wouldn't lag and 30FPS is more than plenty. But if you'd like to make a version using TimerUtils I won't stop you.

Also you should really consider a shorter name, such as TempHeroAttribs.
For me at least the actual name is boring.

I just changed the name per request.. Until everyone agrees on a name, this stays =/

Finally how the Set/Get... native functions handle the bonus given with an ability (through an item or not) ?

I don't understand what you're asking really.. You want me to update the system to not use the Set/Get natives? Why?
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Maybe I'm being "that guy" again, but isn't the < operator faster than <= ? I've just updated it regardless, 2 votes is enough for me.

Since jass is slowly interpreted "<" would beat "<=", just because it's shorter.
But i would rather prevent any silly bug than having some meaningless efficiency win.
See, even Mag recommend "<=", while he is one of the most speedfreak dude i've ever seen.

Why would you need a system like this to be more accurate? Wc3's health fountains aren't even accurate to a degree of .5 seconds, let alone the default value of .1 on this system.

Accuracy is not a big deal, it's just one con more.
Really, the timer will expire many times for nothing while you could just use a new timer each time.
Usually you would use a duration of several seconds and it's not likely that they will merge together.

If it were my map, I'd use a fidelity value of 1./30. because it wouldn't lag and 30FPS is more than plenty. But if you'd like to make a version using TimerUtils I won't stop you.

It's just for you, no offense but when i want this kind of simple script, i just write it myself.
My point is that a periodic timer is overkill, because there is nothing periodic here.

I just changed the name per request.. Until everyone agrees on a name, this stays =/

Well, i can't be objective, but my suggestion seems way better.
Ofc people, plz give your opinion about that.
And since it's still a submission, backward compatiblity is not something important, and it's easy to fix anyway.

I don't understand what you're asking really.. You want me to update the system to not use the Set/Get natives? Why?

No, you don't get it.
I'm just aksing if Set/Get natives takes in consideration the bonus added with abilities.
For example if Get takes in consideration the bonus, while Set doesn't, then this is screwed up.
 
Yeah, that part of the script is what adds the instance to the list :p
It's very simple actually,

set next[this] = 0

We're adding this instance to the end of this list, so we set the next instance of it to 0 to signify the end of the list during iteration.

set prev[this] = prev[0]

prev[0] is acting as a dummy variable to store the last instance in the list.
This is because the previous instance of 0 is unneeded.

set next[prev[0]] = this

I could also do set next[prev[this]] = this here, but the former is faster and slightly shorter :p
What I'm doing is making 'this' instance the next of the last instance in the list.
Remember, when I'm creating an instance, I'm setting the next one of it to 0.
That's why I have to set the next one of the previous one to this ;)
Without this, the list wouldn't work :p

set prev[0] = this

At this point, we're done adding to the list, so we update the last instance to this, because 'this' is the last instance we added to the list and it happens to be the last one in the list too :p

set next[prev[this]] = next[this]
set prev[next[this]] = prev[this]

This will set the next of the previous instance to the next of the current
and it'll set the previous of the next instance to the previous of the current.

This effectively removes the current instance from the list.

Look at this image:
fig365_01_0.jpg


The dotted lines are the new connections between the instances in the list.
The current one will still be linked to the other it's neighboring instances, but that won't affect our system because we'll never come across it during iteration :D
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
So what, you're agree with Codemonkey11 that an unique periodic timer is the way to go here, rather than just X timers ?!
Or you're just saying how to build a linked list in vJass ?
Also i really prefer the syntax next.prev rather than use arrays, you argue that use the array form is easier to understand, i disagree.
At least it seems much more cumbersome and ugly for me, even if the result in jass is just the same.

Plus anyway you can't use this form with structs that don't extends array, but yeah you don't care about it, hmm maybe it's a part from you evil plan to force the use of struct extends array ?
 
Yeah, I'm just explaining the linked list.

In my opinion, it depends on how often the user needs to run the functions.
If the attributes aren't added very often during the game, X timers would be
more efficient, else, the periodic timer would be fine.
(Of course, the X Timers idea should win more often than the periodic one.)

There are cases when you need both in your map though.
Let's assume I have a hero that gains 1 strength point for 3 seconds everytime he attacks.
If he's never picked, the X timers idea would win, else, the periodic timer idea would be best.

I guess a static if should be good :p
Or maybe I should write a timer system that alternates between both of those methods :eek:
That would be epic, right? :D

Also i really prefer the syntax next.prev rather than use arrays, you argue that use the array form is easier to understand, i disagree.

It's because we see things differently and hence have different opinions.
I know some people may hate the way I code, and that's why I spam
comments everywhere so people could understand what's going on :eek:
My ASS system at one point had 600 lines of documentation, 900 lines of comments, and 700 lines of code :O
I shortened it and now it's approximately 1850 lines.

My C++ code is similar :p

JASS:
#include <windows.h>
#include <iostream>
#include "Ticker.h"
#include "GameState.h"
#include "GameConstants.h"
#include "Physics.h"
using namespace std;

/*
*	Time Data Storage
*/
int seconds = 0;
int ticks = 0;

/*
*	This function returns the number of seconds that
*	have passed so far since we initialized the ticker.
*	This will return an int, so it's not very precise.
*/
int GetSeconds() {
	return seconds;
}

/*
*	This function returns the number of minutes that
*	have passed so far since we initialized the ticker.
*	This will return an int, so it's not very precise.
*/
int GetMinutes() {
	return seconds/60;
}

/*
*	This function returns the number of hours that
*	have passed so far since we initialized the ticker.
*	This will return an int, so it's not very precise.
*/
int GetHours() {
	return seconds/3600;
}

/*
*	This function will return the number of ticks.
*	One tick is equivalent to 25 milliseconds.
*	This value can be configured in "GameConstants.h"
*/
int GetTicks() {
	return ticks;
}

/*
*	This function will run throughout the game.
*	It loops every INTERVAL to execute the functions
*	that need to be run periodically.
*/
void Iterate() {
	/*
	*	We are looping infinitely with a small
	*	delay time in between loops.
	*/
	while(true) {

		Sleep(INTERVAL);

		/*
		*	We will only be increasing ticks if the game
		*	is in the normal state. This will allow us 
		*	to pause the time while the game is paused.
		*/
		if (GetGameState() == GAME_STATE_NORMAL) {

			/*
			*	Increase tick count and seconds value
			*	everytime the ticks are equal to the 
			*	the number of frames in one second.
			*	
			*	The modulus operation allows us to 
			*	do a similar check without having
			*	to use another integer to store
			*	the current tick index for one 
			*	second.
			*/
			ticks++;

			if (ticks % FPS == 0) {
				seconds++;
			}

			/*
			*	We will then run the physics engine's loop
			*	function. All widgets in the game will move
			*	a bit depending on their acceleration, velocity
			*	and other stats.
			*/
			Physics_Loop();
		}
	}
}

/*
*	This function will initialize the system 
*	by starting the loop in the Iterate function.
*/
void InitTicker() {
	Iterate();
}

Comments FTW? xD
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
Accuracy is not a big deal, it's just one con more.
Really, the timer will expire many times for nothing while you could just use a new timer each time.
Usually you would use a duration of several seconds and it's not likely that they will merge together.

Makes sense, but does it really matter? I feel more comfortable using this global timer method, and using TimerUtils just adds another required library...

It's just for you, no offense but when i want this kind of simple script, i just write it myself.
My point is that a periodic timer is overkill, because there is nothing periodic here.

The point is that not everyone can write these themself =/ And actually I made this for one specific request.

No, you don't get it.
I'm just aksing if Set/Get natives takes in consideration the bonus added with abilities.
For example if Get takes in consideration the bonus, while Set doesn't, then this is screwed up.

call SetHeroAgi(hero,GetHeroAgi(hero,false)+value,true)

call SetHeroAgi(tempDat.hero,GetHeroAgi(tempDat.hero,false)-tempDat.value,true)

They're both set to ignore bonuses, and make "permanent".

The testing I've done is not super extensive, but I do know that it works.

Yeah, that part of the script is what adds the instance to the list :p
It's very simple actually,

set next[this] = 0

We're adding this instance to the end of this list, so we set the next instance of it to 0 to signify the end of the list during iteration.

set prev[this] = prev[0]

prev[0] is acting as a dummy variable to store the last instance in the list.
This is because the previous instance of 0 is unneeded.

set next[prev[0]] = this

I could also do set next[prev[this]] = this here, but the former is faster and slightly shorter :p
What I'm doing is making 'this' instance the next of the last instance in the list.
Remember, when I'm creating an instance, I'm setting the next one of it to 0.
That's why I have to set the next one of the previous one to this ;)
Without this, the list wouldn't work :p

set prev[0] = this

At this point, we're done adding to the list, so we update the last instance to this, because 'this' is the last instance we added to the list and it happens to be the last one in the list too :p

set next[prev[this]] = next[this]
set prev[next[this]] = prev[this]

This will set the next of the previous instance to the next of the current
and it'll set the previous of the next instance to the previous of the current.

This effectively removes the current instance from the list.

Look at this image:
fig365_01_0.jpg


The dotted lines are the new connections between the instances in the list.
The current one will still be linked to the other it's neighboring instances, but that won't affect our system because we'll never come across it during iteration :D

Yeah, I'm just explaining the linked list.

In my opinion, it depends on how often the user needs to run the functions.
If the attributes aren't added very often during the game, X timers would be
more efficient, else, the periodic timer would be fine.
(Of course, the X Timers idea should win more often than the periodic one.)

There are cases when you need both in your map though.
Let's assume I have a hero that gains 1 strength point for 3 seconds everytime he attacks.
If he's never picked, the X timers idea would win, else, the periodic timer idea would be best.

I guess a static if should be good :p
Or maybe I should write a timer system that alternates between both of those methods :eek:
That would be epic, right? :D



It's because we see things differently and hence have different opinions.
I know some people may hate the way I code, and that's why I spam
comments everywhere so people could understand what's going on :eek:
My ASS system at one point had 600 lines of documentation, 900 lines of comments, and 700 lines of code :O
I shortened it and now it's approximately 1850 lines.

My C++ code is similar :p

JASS:
#include <windows.h>
#include <iostream>
#include "Ticker.h"
#include "GameState.h"
#include "GameConstants.h"
#include "Physics.h"
using namespace std;

/*
*	Time Data Storage
*/
int seconds = 0;
int ticks = 0;

/*
*	This function returns the number of seconds that
*	have passed so far since we initialized the ticker.
*	This will return an int, so it's not very precise.
*/
int GetSeconds() {
	return seconds;
}

/*
*	This function returns the number of minutes that
*	have passed so far since we initialized the ticker.
*	This will return an int, so it's not very precise.
*/
int GetMinutes() {
	return seconds/60;
}

/*
*	This function returns the number of hours that
*	have passed so far since we initialized the ticker.
*	This will return an int, so it's not very precise.
*/
int GetHours() {
	return seconds/3600;
}

/*
*	This function will return the number of ticks.
*	One tick is equivalent to 25 milliseconds.
*	This value can be configured in "GameConstants.h"
*/
int GetTicks() {
	return ticks;
}

/*
*	This function will run throughout the game.
*	It loops every INTERVAL to execute the functions
*	that need to be run periodically.
*/
void Iterate() {
	/*
	*	We are looping infinitely with a small
	*	delay time in between loops.
	*/
	while(true) {

		Sleep(INTERVAL);

		/*
		*	We will only be increasing ticks if the game
		*	is in the normal state. This will allow us 
		*	to pause the time while the game is paused.
		*/
		if (GetGameState() == GAME_STATE_NORMAL) {

			/*
			*	Increase tick count and seconds value
			*	everytime the ticks are equal to the 
			*	the number of frames in one second.
			*	
			*	The modulus operation allows us to 
			*	do a similar check without having
			*	to use another integer to store
			*	the current tick index for one 
			*	second.
			*/
			ticks++;

			if (ticks % FPS == 0) {
				seconds++;
			}

			/*
			*	We will then run the physics engine's loop
			*	function. All widgets in the game will move
			*	a bit depending on their acceleration, velocity
			*	and other stats.
			*/
			Physics_Loop();
		}
	}
}

/*
*	This function will initialize the system 
*	by starting the loop in the Iterate function.
*/
void InitTicker() {
	Iterate();
}

Comments FTW? xD

I find your C++ code infinity times easier to understand, and I didn't read the comments.

I think it's just because I'm not familiar with linked lists. Anyway thanks for taking the time to explain it.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Makes sense, but does it really matter? I feel more comfortable using this global timer method, and using TimerUtils just adds another required library...

It's like using a machine-gun instead of a knife to kill a chicken.
It will work, but is it really needed ?
Plus it will reduce the size of the script (just one pro more), assuming TimerUtils will be used somewhere else.
I'm against boring and quite useless requirements such as BoundUtils just to avoid the creation/destruction of a rect, but here TimerUtils is a de facto standard, and it's useful.
I just like to use what's appropriated, simple as that.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
I just like to use what's appropriated, simple as that.

I think that's perfectly fair. In this case I simply did it without TimerUtils. I think it's quite reasonable they way I've done it though. Considering the timer only runs when some temporary stat change exists, I think it's efficient enough.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
If i would be a moderator i would reject it because of that, hopefully i'm not.
Personal tastes shouldn't matter that much in scripts submission, logic should be above all.

...but in the situation where the script was made with the intention of handling multiple instances with the same duration, then the way I've made this is perfectly logical, right?

Like maybe in a script where a unit siphons agility.

http://www.hiveworkshop.com/forums/2158914-post14.html
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
Not really, since there is still no need of periodic stuff, and the timer will still expire many times for nothing.
Ofc you would hardly notice any difference in game, but that doesn't make it more logical.

I still feel like you're arguing your opinion. If someone wants to make a spell that siphons n agility/strength/intelligence from all nearby heroes within x range for 10 seconds, then the 1 timer is more logical than 6*(hero count) timers, even if those have exact durations..

In the end it's completely situational, and arguing that using TimerUtil's is more logical is kind of inappropriate in my opinion.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
Of course I understand that. It just doesn't matter really.

Let's take the example of a 5 second duration modification where fidelity is .5 seconds.

All that happens is this 10 times:

JASS:
set tempDat.duration=tempDat.duration-TIMER_FIDELITY
if tempDat.duration<=0. then //this returns false

So what? You really think the overhead required by TimerUtils worth while in comparison to these minuscule lines? MAYBE if you have another system that's using it. But just the fact that my argument is even remotely relevant means changing this for the sake of changing it isn't worth my time.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
Look dude, what you're calling the "logical" method, in my mind, is completely illogical. Do you realize how much overhead TimerUtils requires? Have you ever actually read it?

In the best case, it creates a shit ton of timers on map initialization (a slow process). If a user wants to use more timers than what's available, the system completely breaks.

In the worst case, it creates a hashtable (another slow process) and then has to make those same shit ton of timers on map initialzation, and then instead of breaking when many timers are needed, it just creates more damn timers.

I just tested this myself. Take my script with as precise a fidelity as you like (I'm on a laptop and used 1./10. for the fidelity) then place 100 paladins on the map and run this script:

JASS:
scope test initializer i
    private function a takes nothing returns nothing
        local group grp=CreateGroup()
        local unit FoG
        call GroupEnumUnitsInRect(grp,bj_mapInitialPlayableArea,null)
        loop
            set FoG=FirstOfGroup(grp)
            exitwhen FoG==null
            call TemporaryHeroAttribute_add(FoG,'A',10,5.)
            call TemporaryHeroAttribute_add(FoG,'S',10,5.)
            call TemporaryHeroAttribute_add(FoG,'I',10,5.)
            call GroupRemoveUnit(grp,FoG)
        endloop
        call DestroyGroup(grp)
        set grp=null
        call DestroyTimer(GetExpiredTimer())
    endfunction
    
    private function i takes nothing returns nothing
        local timer time=CreateTimer()
        call TimerStart(time,5.,false,function a)
        set time=null
    endfunction
endscope

This script creates 300 instances in the stack instantly and then loops from 0 to 299 10 times per second without lag.

You know what would happen if you did that with TimerUtils using the default QUANTITY of 256?

In the "fast" case, it would just break the system when it asked for 300 timers and 256 were initialized.

In the "slow" case, it would start 256 timers and then be forced to CreateTimer() 44 times.

Do you understand now how ridiculous you sound to me?

Edit: And if that didn't convince you enough already, I just did the same thing with fidelity 1./100. and 200 paladins on the map. 60 fps on a laptop.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Well, i've personnaly made an other library to don't use TimerUtils, i'm perfectly aware, anyway you could still use TimerUtils by Mag :

http://www.hiveworkshop.com/forums/graveyard-418/system-timerutilsex-204500/

Anyway that doesn't matter at all how many time it takes to create X timers on red version, because it's done only on init, and i wouldn't say that that slow.

I agree that the version on wc3c.net suck hard.

A fact is a fact, as already said you would hardly see any difference in game, but using a periodic timer is not logical here, you can't say the opposite or you're just lying.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
Well, i've personnaly made an other library to don't use TimerUtils, i'm perfectly aware, anyway you could still use TimerUtils by Mag :

http://www.hiveworkshop.com/forums/graveyard-418/system-timerutilsex-204500/

Come on dude, what's the difference?

JASS:
...
@private constant integer QUANTITY = 256@
    endglobals
    
    globals
        private timer array tT
        private integer tN = 0
    endglobals
    
    private module Init
        private static method onInit takes nothing returns nothing
            static if not USE_HASH then
                local integer i = QUANTITY
                @loop@
                    @set i = i - 1@
                    @set tT[i] = CreateTimer()@
                    @exitwhen i == 0@
                @endloop@

Either way the whole point of the script is to store a ton of instanciated timers, and the same problems occur.

Anyway that doesn't matter at all how many time it takes to create X timers on red version, because it's done only on init, and i wouldn't say that that slow.

That's not the problem with red version. The problem is that if you try to use too many timers, the system just breaks. Why would I want to make such a simple system like mine use something so complicated like a timer utility library?

A fact is a fact, as already said you would hardly see any difference in game, but using a periodic timer is not logical here, you can't say the opposite or you're just lying.

I'm not saying that a periodic timer is illogical for a system that doesn't do anything periodically. I'm saying that using TimerUtils is illogical for system that's lighter weight than the utility itself, whose integration can do nothing but break it.

What more do you want from me? Fidelity of 1./200. and 400 instances?

Go for it, I'm sure that won't lag either.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Well, read Mag's code better and you will see that the blue version is not broken and only create as many timers as needed, no timers are created on init with the blue version, that's the differences.
It's a TimerUtils which is not broken, nor sensless.
The reason why it has been graveyarded it is because TimerUtils on wc3c.net was supposed to be fixed, but in fact it is not, so i suppose Bribe should ungraveyard this.
At worst, the side effect is that the version on wc3c.net will be fixed finally.

The red version is for speedfreaks, it is intended to be broken if you go higher than the max number of timers, in fact you have to make sure you will never reach it.
Note that if you don't change the value of constant variable it's the blue version which is used, not the red one.

And blablabla timer periodic, was, is, and will be for ever overkill for this stuff, no matter if it lags or not.

It is not logical, in the same way you evaluated a trigger on a timer expire event, rather than just use the timer callback.
If only it added some pro, but no, it's just adding innacuracy and more useless stuff executed in the background, while one useful library requirement is not a con.

That's the last comment from me about it, all was perfectly stated, now it's up to you to change your mind or not.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
Well, read Mag's code better and you will see that the blue version is not broken and only create as many timers as needed, no timers are created on init with the blue version, that's the differences.

I did read the script, and that's how I understand that the blue version (which doesn't create timers on initialization) instead would CreateTimer() 300 times for the script example I wrote above.

Granted, after that initial time it runs, it would recycle the timers if there were following runs, but even so it has to make 300 hashtable reads instead, which still isn't faster than what I've made.

And blablabla timer periodic, was, is, and will be for ever overkill for this stuff, no matter if it lags or not.

It is not logical.

That's the last comment from me about it, all was perfectly stated, now it's up to you to change your mind or not.

You're completely ridiculous. I've shown you with examples why this method is better in every way, and yet you're still stuck on the fact that it runs periodically when it doesn't need to. Who cares? 10,000 callback functions of mine are still faster than CreateTimer().

I'm done talking about this. Normally I'm pretty open minded when it comes to this stuff because I know I'm not even on the same playing field when it comes to the scripting knowledge a lot of you guys have. But I'm confident now that you're just being irrational.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
10,000 callback functions of mine are still faster than CreateTimer()

Huge no, you overevaluate the cost of CreateTimer, and consider too much less the cost of a timer expiration.
The cost of CreateTimer would be about the same of a timer expiration or even (much) less.
Now, again that would be unoticeable in real cases, but a periodic timer is still not logical.

You can still believe i'm irrational but i can't do more really, i will let the others state about it.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
Huge no, you overevaluate the cost of CreateTimer, and consider too much less the cost of a timer expiration.

Sorry I was a bit frustrated and exaggerating. Using a script like this:

JASS:
scope three100Timers initializer i
    private function after takes nothing returns nothing
    endfunction
    
    private function a takes nothing returns nothing
        local integer index=0
        local timer time
        call DestroyTimer(GetExpiredTimer())
        loop
            exitwhen index>999
            set time=CreateTimer()
            call TimerStart(time,10.,false,function after)
            set index=index+1
        endloop
        set time=null
    endfunction
    
    private function i takes nothing returns nothing
        local timer time=CreateTimer()
        call TimerStart(time,5.,false,function a)
        set time=null
    endfunction
endscope

I needed to create 1000 timers in a loop to experience even a slight dip in FPS (50, down from 60). And probably on a proper machine there would be no dip.

The best solution is to add a static if.
You would let the user decide.

Come on dude. You're talking about make very simple, 71 line snippet into something way more complex than it needs to be. There's no reason to let the user choose between "fast" and "fast".

I would want to see a real map where there are more than several dozens of hereos which attributes are changed with that simultaneously, else it's just not worth it.

You only have to change them simultaneously to make TimerUtils slow, but there's no combination of create/destroy that makes my script slow really. The worst thing you can do is make the stack huge so that each periodic callback has to loop a thousand times. But I've just shown you that it handles this perfectly well.

Let's not take a simple., useful snippet and make discussing it more complicated than it needs to be.

Obviously Troll-Brain and Magtheridon would just make their own script if they needed this. But not everyone knows how to make stuff like this! That's the whole point.
 
Top