KaTaNNa Code Safe Update

07-17-2008, 11:53 PM

Disclaimer: This code is ugly. The bits for purging are a disaster (I'll probably redo them; any ideas on methology would be nice). It is considerably slower than KaTaNNa's system. You should not use this code for new code, nor should you use KaTaNNa's code either - both are very much outdated.

So - why would you use this code? Basically, the idea is you can just replace KaTaNNa's code with this, and it'll make the system safe, in two ways:

- No more I2H, which can get wrong handle types. (With the exception of pseudo-handles, which aren't handles, just have handle type, and act differently, believed to be safe.)
- If the subject has changed, data will NOT carry forward, even for the same handle type. (This was dropped, but won't affect you if your code was correct to start with - this is designed to fix blizzard bugs, not your buggy code; can be fixed for units by PurplePoot's RemovalDetection system.)

If your map is already lagging, for the love of all that is holy and unholy, redo your code totally, as this will only make your laggy code worse. If you don't want to or cannot redo the code, are not lagging, and are using KaTaNNa's system, then you might want to implement this to get that extra safety.

If that sounds like you, then reply here. Do not use the code yet except for testing. I have not fully tested or polished it (mainly the code for dealing with flushing of indexes).

Oh, and if at any point you are using more than 8000 handle / int / string / boolean / real data values, things'll go to pot (it'll purge it after 8000, getting rid of those without a correct subject / that's been flushed off; if it's still over 8000, then it'll do it again, and again, and again...which'll lag like crazy, I guess). That'd be 16 bits of data on 500 handles (for example), which is quite a lot, although I suppose possible.

Updated, now works fine with any amount of data. However, in the event that your handle indexes go above the maximum, it'll die. The maximum is set to 73710 atm, which should be more than ample, I think. (you'd need to leak more than 1000 indices a minute for an hour). The implementation no longer needs purging separately to deal with flushing. It's now a very neat, if very slow, implementation. For strings, booleans, integer and real attachements, there is almost no difference in speed, despite the added safety. Handle attachments are over 2 times slower, however.

In response to making the H2I function private, the H2I function was provided by the KaTTaNa system for all below it, and hence is provided by this for that as well; anything else could break compatability, and thus misses the whole point of the exercise.

Updated again: Fixed a HUGE bug that was taking every handle index it touched hostage. Removed, and now it won't do that at all. Further more, it should now run faster, since the GetIndex function (which should now be inlined) doesn't need to have a check in it. As a result, it requires the RemovalDetection script.

Needed: http://www.wc3campaigns.net/showthread.php?t=102588

Erm...yea, give me a bit to try and work out a new version that doesn't take over handle indexes.

Updated AGAIN: Now should be fine, no more handle index leaks, since they get cleaned by a nice efficient way (the exception to this is effects, locations and a few others; triggers and timers are deliberately not recycled, so as to avoid the bugs associated with that). This relies upon at least one get (that isn't a generic handle get) being called on each handle, and on your coding being correct (but it'll bug less with this than under KaTaNNa's for those of you with buggy code due to your own crap coding). Locations can easily be added, but only if you have a function for getting locations directly and use it (rather than getting it as a handle then type casting to location). That isn't in the KaTaNNa code I have, but it'd be fairly easy to add it, just drop me a PM or something.

Pseudo handles now use I2H. Since they aren't actually handles, and aren't involved in the handle stack, that should be safe. If you only attach one type of pseudohandle (eg: just lightning), then you could avoid using I2H, but it shouldn't be anything to worry about. Attaching to pseudohandles is okay, but attaching to more than one type of pseudohandle could cause collisions, same as with KaTaNNa's.

Long story short, if you coded correctly for KaTaNNa's system, this shouldn't bug, but will be a bit slower, if you didn't code correct for it, this'll be slower and still safer, but you may still get bugs due to your own coding (obviously).

Anyway, the code:

JASS:

library LocalHandleVarsReplacement initializer Init
    globals
        private gamecache gc
        private integer valueIndex = 0
        private integer checkInteger = 0
        private string subjectString = null
        
        private constant integer ARRAY_SIZE = 409550 // the higher the number, the less speed, but you can have more handle index leaks.
        
        private integer array indexType[ARRAY_SIZE]
        private integer array indexNext[ARRAY_SIZE] // Which index to jump to, part of the index.
        private handle array indexHandle[ARRAY_SIZE]
        private integer array indexCheck[ARRAY_SIZE] // note that the check will start at zero, but will always be greater than 1 if handle != null
                                                    // Now just used to check for attaching handles to handles, so you DO need to flush handles if they might recycle.
                                                    // Units - RemoveDetection library (@wc3c), the rest you're in full control of anyway, so should already be flushing
        
        private constant string CHECK_SUFFIX = "c&++-&$$" // this should be unique so that nothing will collide with it.
        
        private constant string GAME_CACHE_STRING = "cgkattanavars.w3v"
    endglobals
    
    // This stuff is used to make handle indexes recycle properly while avoiding I2H
    // You may need to alter this somewhat if you have some non-wc3jass standard library.
    globals // 0 = don't know (not gotten yet), 1 = Cannot/shouldn't do anything, 2 = widget, 3 = group, 4 = lightning
        private constant integer TYPE_Handle = 0 // tells us nothing.
        private constant integer TYPE_Timer = 1 // Don't really want to recycle these
        private constant integer TYPE_Unit = 2
        private constant integer TYPE_Trigger = 1 // SHOULD NOT be allowed to recycle. Evar.
        private constant integer TYPE_Effect = 1 // cannot tell if this has been destroyed or not...
        private constant integer TYPE_Group = 3
        //private constant integer TYPE_Lightning = 1 // Lightning recycles anyway. // Not needed
        private constant integer TYPE_Widget = 2
        
        private integer loopCurrent = 0
        private constant integer loopInc = 100 // how many to do per loop, max
        private integer loopSize = 0 // how many in the loop
        private constant real INTERVAL = 1.
        private timer tim
        
        private unit constantUnit
    endglobals
    
    private function H2W takes handle h returns widget
        return h
    endfunction
    //private function H2L takes handle h returns lightning
    //    return h
    //endfunction
    private function H2G takes handle h returns group
        return h
    endfunction
    
    private function CleanLoop takes nothing returns nothing // only loops through those that can be potentially recycled.
        local integer i = loopInc
        local integer typ
        local real r
        local integer last
        local boolean recycle = false
        
        local real a
        local real b
        local real g
        
        local widget w
        local group gr
        local lightning l
        
        if loopSize < loopInc then
            set i = loopSize // no need to loop more than once around in any one go
        endif
        
        loop
            exitwhen i <= 0
            
            set last = loopCurrent // needed for removing one from loop
            set loopCurrent = indexNext[loopCurrent]
            
            set typ = indexType[loopCurrent]
            if typ == TYPE_Widget then
                set w = H2W(indexHandle[loopCurrent])
                set r = GetWidgetLife(w)
                if r == 0. then
                    call SetWidgetLife(w, 1.)
                    if GetWidgetLife(w) == 0. then
                        set recycle = true
                    else
                        call SetWidgetLife(w, r)
                    endif
                endif
            elseif typ == TYPE_Group then
                set gr = H2G(indexHandle[loopCurrent])
                if FirstOfGroup(gr) == null then
                    call GroupAddUnit(gr, constantUnit)
                    if IsUnitInGroup(constantUnit, gr) then
                        call GroupRemoveUnit(gr, constantUnit)
                    else
                        set recycle = true
                    endif
                endif
            //elseif typ == TYPE_Lightning then
            //    set l = H2L(indexHandle[loopCurrent])
            //    set a = GetLightningColorA(l)
            //    if a == 0. then
            //        set r = GetLightningColorR(l)
            //        set g = GetLightningColorG(l)
            //        set b = GetLightningColorB(l)
            //        call SetLightningColor(l, 0., 0., 0., 1.)
            //        if GetLightningColorA(l) == 0. then
            //            //set recycle = true
            //        else
            //            call SetLightningColor(l, r, g, b, a)
            //        endif
            //    endif
            endif
            
            if recycle then
                set indexNext[last] = indexNext[loopCurrent]
                set indexHandle[loopCurrent] = null
                //set indexCheck[loopCurrent] = indexCheck[valueIndex] + 1 // whenver the value changes. // not actually needed here, since null is return for both
                set loopSize = loopSize - 1
                if loopSize == 0 then
                    call PauseTimer(tim) // loop is now empty, so stop the timer rolling.
                    exitwhen true
                endif
                
                set recycle = false
            endif
            
            set i = i - 1
        endloop
        
        set w = null
        set gr = null
        set l = null
    endfunction
    
    function LocalVars takes nothing returns gamecache
        return gc // this should be inlined automatically
    endfunction
    // ===========================
    
    function H2I takes handle h returns integer
        return h
        return 0
    endfunction
    
    private function GetIndex takes handle h returns integer // no need anymore for the check since it cannot recycle if the global is set to it
        return H2I(h) - 0x0FFFFF
    endfunction
    
    // ===========================

    function SetHandleHandle takes handle subject, string name, handle value returns nothing
        if subject == null then
            return
        endif
        
        set subjectString = I2S(GetIndex(subject))
        
        if value == null then
            call FlushStoredInteger(LocalVars(), subjectString, name)
            call FlushStoredInteger(LocalVars(), subjectString, name+CHECK_SUFFIX)
            return
        endif
        
        set valueIndex = GetIndex(value)
        if valueIndex < 0 then // it must be a pseudohandle, in which case we don't really need security, since it has no part on the handle stack.
            call StoreInteger(LocalVars(), subjectString, name, valueIndex + 0x0FFFFF)
            call StoreInteger(LocalVars(), subjectString, name+CHECK_SUFFIX, -1) // to show it's a pseudohandle
            return
        elseif indexHandle[valueIndex] != value then
            set indexHandle[valueIndex] = value
            set indexType[valueIndex] = 0 // we don't know what type it is.
            set indexCheck[valueIndex] = indexCheck[valueIndex] + 1 // whenver the value changes.
        endif
        
        call StoreInteger(LocalVars(), subjectString, name, valueIndex) // stores the value
        call StoreInteger(LocalVars(), subjectString, name+CHECK_SUFFIX, indexCheck[valueIndex]) // stores the checkvalue
    endfunction
    
    //! textmacro GetHandleH takes TYPECAP, TYPELOWER
    
    function GetHandle$TYPECAP$ takes handle subject, string name returns $TYPELOWER$
        if subject == null then
            return null
        endif
        
        set subjectString = I2S(GetIndex(subject))
        set valueIndex = GetStoredInteger(LocalVars(), subjectString, name)
        
        if valueIndex == 0 then
            return null
        elseif indexCheck[valueIndex] == GetStoredInteger(LocalVars(), subjectString, name+CHECK_SUFFIX) then // if it checks out
            if TYPE_$TYPECAP$ > 1 and indexType[valueIndex] != 0 then
                set indexType[valueIndex] = TYPE_$TYPECAP$
                if loopSize == 0 then
                    call TimerStart(tim, INTERVAL, true, function CleanLoop)
                    set loopSize = 1
                    set loopCurrent = valueIndex
                    set indexNext[valueIndex] = valueIndex // make it a loop in the linked list
                else
                    set loopSize = loopSize + 1
                    set indexNext[valueIndex] = indexNext[loopCurrent]
                    set indexNext[loopCurrent] = valueIndex // inserted into the linked list loop
                endif
            endif
            
            return indexHandle[valueIndex]
        else
            return null
        endif
    endfunction
    
    //! endtextmacro
    
    function GetHandleHandle takes handle subject, string name returns handle
        if subject == null then
            return null
        endif
        
        set subjectString = I2S(GetIndex(subject))
        set valueIndex = GetStoredInteger(LocalVars(), subjectString, name)
        set checkInteger = GetStoredInteger(LocalVars(), subjectString, name+CHECK_SUFFIX)
        
        if valueIndex == 0 then
            return null
        elseif checkInteger == -1 then
            return valueIndex // only I2H type casts are for pseudo handles.
            return null
        elseif indexCheck[valueIndex] == checkInteger then // if it checks out
            // Handle type tells us nothing, so cannot insert to be cleaned
            return indexHandle[valueIndex]
        else
            return null
        endif
    endfunction
    
    function GetHandleLightning takes handle subject, string name returns lightning
        if subject == null then
            return null
        endif
        
        set subjectString = I2S(GetIndex(subject))
        set valueIndex = GetStoredInteger(LocalVars(), subjectString, name)
        
        if valueIndex == 0 then
            return null
        elseif -1 == GetStoredInteger(LocalVars(), subjectString, name+CHECK_SUFFIX) then // if it checks out
            return valueIndex // pseudo handle, so alright.
            return null
        else
            return null
        endif
    endfunction
    
    // //! runtextmacro GetHandleH( "Handle", "handle" ) // edited
    //! runtextmacro GetHandleH( "Unit", "unit" )
    //! runtextmacro GetHandleH( "Timer", "timer" )
    //! runtextmacro GetHandleH( "Trigger", "trigger" )
    //! runtextmacro GetHandleH( "Effect", "effect" )
    //! runtextmacro GetHandleH( "Group", "group" )
    // //! runtextmacro GetHandleH( "Lightning", "lightning" ) // Lightning is weird, and a pseudo-handle
    //! runtextmacro GetHandleH( "Widget", "widget" )
    
    //! textmacro GetSetHandle takes TYPEFUNC, TYPECAP, TYPELOWER, DEFAULT
    
    function SetHandle$TYPEFUNC$ takes handle subject, string name, $TYPELOWER$ value returns nothing
        if subject == null then
            return
        elseif value == $DEFAULT$ then
            call FlushStored$TYPECAP$(LocalVars(), I2S(GetIndex(subject)), name)
        else
            call Store$TYPECAP$(LocalVars(), I2S(GetIndex(subject)), name, value)
        endif
    endfunction
    
    function GetHandle$TYPEFUNC$ takes handle subject, string name returns $TYPELOWER$
        if subject == null then
            return $DEFAULT$
        endif
        return GetStored$TYPECAP$(LocalVars(), I2S(GetIndex(subject)), name)
    endfunction
    
    //! endtextmacro
    
    //! runtextmacro GetSetHandle("Int", "Integer", "integer", "0")
    //! runtextmacro GetSetHandle("Boolean", "Boolean", "boolean", "false")
    //! runtextmacro GetSetHandle("Real", "Real", "real", "0.")
    //! runtextmacro GetSetHandle("String", "String", "string", "null")
    
    function FlushHandleLocals takes handle subject returns nothing
        call FlushStoredMission(LocalVars(), I2S(GetIndex(subject)) )
    endfunction
    private function Init takes nothing returns nothing
        set tim = CreateTimer()
        set constantUnit = CreateUnit(Player(15), 'hfoo', 0., 0., 0.)
        call ShowUnit(constantUnit, false)
        
        call FlushGameCache(InitGameCache(GAME_CACHE_STRING))
        set gc = InitGameCache(GAME_CACHE_STRING)
    endfunction
endlibrary

07-18-2008, 12:21 AM	#2
moyack	Set the H2I function private and add a constant string, so the user can change easily the gamecache filename. Everything else looks fine and elegant. I'll do a test when I finish some setups of my linux box.

07-18-2008, 12:24 AM	#3
Rising_Dusk	Of course, you could also just use StoreInteger() to store a struct to anything that can then point to whatever data you'd want it to. :)

07-18-2008, 03:36 AM	#4
darkwulfv	And Dusk's method is probably a lot safer. That's what I do, personally. Attach struct with SetHandleInt, then get it where I need it. EDIT: I also just scanned the code I have (KaTaNNa's), and I see no use of "I2H" anywhere; only "H2I" and "I2S". unless "I2H" is used in a way I'm not seeing, or I have a more recent version... or something.

07-18-2008, 03:59 AM	#5
Rising_Dusk	KaTTaNa's Handle Vars abuses the return bug to return integers to their respective types. The problem is that you can return any integer to a type; there is no type safety, thus handle indexing corruptions cause the game to go haywire and break. Something like GetHandleUnit() abuses such I2H conversions. (Integer to Handle)

07-18-2008, 04:49 AM

darkwulfv

JASS:

    function GetHandle$TYPECAP$ takes handle subject, string name returns $TYPELOWER$
        local cachehandle ch = GetStoredInteger(LocalVars(), I2S(H2I(subject)), name)
        if ch.subject == subject then
            return ch.value
            return null
        else
            return null
        endif
    endfunction

Looks very very similar to:

JASS:

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

They use similar data retrieval methods (I2S(H2I(subject))). I guess I must be missing something, so pardon my ignorance if I am.

Whatever, I'll trust that CG's is safer and whatnot, and I'll wait for an official release to use it. It works like the previous Handle Vars, right? call SetHandlehandle and call GetHandleHandle etc.?

07-18-2008, 09:20 AM

Captain Griffen

Quote:

Of course, you could also just use StoreInteger() to store a struct to anything that can then point to whatever data you'd want it to. :)

Then the code would need to be updated to use a new syntax, at which point you really shouldn't be using the code. This is more of a hot-fix for system/maps that may still use it.

Quote:

Originally Posted by darkwulfv

JASS:

    function GetHandle$TYPECAP$ takes handle subject, string name returns $TYPELOWER$
        local cachehandle ch = GetStoredInteger(LocalVars(), I2S(H2I(subject)), name)
        if ch.subject == subject then
            return ch.value
            return null
        else
            return null
        endif
    endfunction

Looks very very similar to:

JASS:

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

Whoops, I just forgot to remove the 'return null' in the GetHandleXXX. Since it's actually stored in a handle variable, it's perfectly safe and doesn't need to abuse the return bug for I2H.

And yes, the syntax is exactly the same. Code that works with the old should work almost the same with this; the only difference should be the improved safety (no transference between subjects due to recycling, no I2H mistaken returns) and the somewhat reduced performance.

I should re-iterate that it isn't designed to be used for new code, but to fix old code easily. (Although if speed isn't important, and you don't use the FlushHandle function, then it should be perfectly viable and safe, if very slow, alternative that is very flexible).

07-18-2008, 10:42 AM

d07.RiV

Can you tell me how is it better than this:

JASS:

//! textmacro GetSetHandle takes TYPE, TYPENAME
function SetHandle$TYPENAME$ takes handle subject, string name, $TYPE$ value returns nothing
  if value == null then
    call FlushStoredInteger (hcache, I2S (H2I (subject)), "$type$" + name)
  else
    call StoreInteger (hcache, I2S (H2I (subject)), "$type$" + name, H2I (value))
  endif
endfunction
function GetHandle$TYPENAME$ takes handle subject, string name returns $TYPE$
  return GetStoredInteger (hcache, I2S (H2I (subject)), "$type$" + name)
  return null
endfunction
//! endtextmacro

And what do you mean by "If the subject has changed, data will NOT carry forward"?
Doesn't comparing handles give the same result as comparing their H2I's?

07-18-2008, 11:03 AM

Captain Griffen

Quote:

Originally Posted by d07.RiV

Can you tell me how is it better than this:

JASS:

//! textmacro GetSetHandle takes TYPE, TYPENAME
function SetHandle$TYPENAME$ takes handle subject, string name, $TYPE$ value returns nothing
  if value == null then
    call FlushStoredInteger (hcache, I2S (H2I (subject)), "$type$" + name)
  else
    call StoreInteger (hcache, I2S (H2I (subject)), "$type$" + name, H2I (value))
  endif
endfunction
function GetHandle$TYPENAME$ takes handle subject, string name returns $TYPE$
  return GetStoredInteger (hcache, I2S (H2I (subject)), "$type$" + name)
  return null
endfunction
//! endtextmacro

And what do you mean by "If the subject has changed, data will NOT carry forward"?
Doesn't comparing handles give the same result as comparing their H2I's?

I2H is bad. That's what KaTTaNa's system uses to return handle type objects. This can result in all sorts of bad stuff happening. We have several threads on why it's bad, so look them up, as otherwise this thread will get deluged by it.

And comparing handles doesn't give the same result as their H2Is, as handle variables keep a reference, which means that, when a handle is recycled, it doesn't incorrectly carry stuff over. Hmm...I should probably have it return null/0 when a null subject is used.

07-18-2008, 01:03 PM	#10
Vexorian	A lot of people that use handle vars don't clean the stuff properly, so I'd say this will eventually end up using all available slots...

07-18-2008, 01:49 PM

#11

Captain Griffen

Quote:

Originally Posted by Vexorian

A lot of people that use handle vars don't clean the stuff properly, so I'd say this will eventually end up using all available slots...

When it purges, it removes those on subjects that have since turned to null. That should avoid the issue; it depends on how much lee-way there is on real slot usage, how long handles are around, etc. I may need to change it to use big arrays and manually deal with stuff.

07-18-2008, 02:30 PM

#12

darkwulfv

I'm just not fully understanding where the Handle Vars I use (I posted an example) used I2H, because I see it nowhere, and the data retrieval lines are exactly the same between that and this system. Maybe I have a weird offshoot of KaTTaNa's system, or this has a safety feature I'm not seeing somewhere.

But if, for example, I took the old one out and plugged this one in, it would basically keep everything the same? (I wouldn't have to run around and change function calls?)

07-18-2008, 02:47 PM

#13

Captain Griffen

Quote:

Originally Posted by darkwulfv

KaTaNNa's system uses the return bug to typecast from integer to handle (it doesn't actually use a function called I2H, but it uses the same principle). That is unsafe. It also has no protection against handle recycling.

Quote:

But if, for example, I took the old one out and plugged this one in, it would basically keep everything the same? (I wouldn't have to run around and change function calls?)

That is the entire point of it.

07-18-2008, 02:54 PM	#14
darkwulfv	Alright, I think I understand now. It just didn't make sense to me when your system used (what looked like) almost the exact same setup, just with structs and privates and the like. I do see the comparison checks in there for recycling safety and stuff though, which is good.

07-18-2008, 03:53 PM	#15
d07.RiV	Why would a handle become 0 if you don't set it to 0? Its like integer a = 2 integer b = a set a = 0 B IS NOT 0 ?!? How is it different with handles? Unless blizzard were really high when they made it, because anyone in right mind would have at least made proper reference counting.