HomeUser Control Panel (unavailable in archive)ForumsTutorialsArt GalleryResourcesMaps

OCUS, overly costly unit struct

07-12-2008, 05:53 AM#1
Vexorian
Remember what I said the other day about unit indexing and how it is low level and doesn't help doing stuff correctly like detection of recycling, etc?

I tried implementing something that would follow that overly demanding ideal I described, it turns out it is possible, but it is also a little costly. I am quite sure it is an amazing turtle when compared to PUI specially if there are many structs attached to a unit, since the search is linear, *something* is missing there to make the search non-linear, one option is gamecache, another would be making a whole hash table that receives pairs as input, and another one would be replacing the linked list with a red-black tree. Red black trees are fast yet they are HORRIBLY complicated to code, in fact I heard that the last person who tried to code them in Jass committed suicide (his girlfriend dumped him so he killed himself 2 years after trying the implementation of the red-black trees) Oh and I am not sure if it is possible to implement those things without requiring a lot of function calls, that might end up becoming worse than gamecache...

Then it is the CSData-like array + subtraction lookup, you know, the initial intention was to allow userdata to be used and provide the CSData-like version as an alternative for people that already use UserData for something else. But then, UserData from removed units becomes zero, so it would be hard to do the stuff after the removal, unless I kept paralel arrays instead of just seenunits a-la PUI , well, I would probably change that if I actually end up using this, but that would only come from improving the linear search, which seems quite nasty.

Edit: Oh, and when the struct needs to be created, it calls a TriggerEvaluate, so just picture it: Linear search + CSData-like thing + TriggerEvaluate ...

The good part of it, is the result, you can tell it to get a struct of certain type for a certain unit, if the unit already had such struct, it returns it, else it would create it, which means that the default values for members and the static create method are called whenever a struct type is needed by a unit that didn't use it before. Then when remove is detected, it will call .destroy() on all the structs attached to the removed unit.


Expand UnitStruct:

So for example:
Collapse JASS:
library sandbox initializer init requires UnitStruct

    private struct test extends UnitStruct
        integer a

        method onDestroy takes nothing returns nothing
            call BJDebugMsg("Dies: "+I2S(this.a))
        endmethod
    endstruct

    private struct kills extends UnitStruct
        integer count=0
    endstruct

    private function H2I takes handle h returns integer
        return h
        return 0
    endfunction
    private function counter takes nothing returns nothing
     local unit u=GetKillingUnit()
     local kills k=UnitStruct.get(u, kills.typeid) //It is trivial to make jasshelper make 'kills' return the typeid, Instead of requiring .typeid, I think I will add that.

        set k.count=k.count+1
        call BJDebugMsg(GetUnitName(u)+I2S(H2I(u))+" has killed "+I2S(k.count)+" so far " )

     set u=null
    endfunction

    private function init takes nothing returns nothing
     local unit u
     local unit v
     local test x
     local integer i=30
     local trigger t=CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_DEATH)
        call TriggerAddAction(t,function counter)
        loop
            exitwhen i<0
            set u=CreateUnit(Player(0),'hfoo',0,0,0)
            set x=UnitStruct.get(u,test.typeid)
            set x.a=i
            call SetWidgetLife(u,20)
            set i=i-1
        endloop
        call TriggerSleepAction(5.0)
        call RemoveUnit(u)
        set u=null
        set v=null
    endfunction
endlibrary

Notice how the kill counter thing survives recycling without really handling it manually. Of course, it is more suitable for bigger things, like when a single spell buff needs to attach 6 values to a single unit. Etc. But ouch ,the performance is bad right now.
07-12-2008, 08:09 AM#2
cohadar
local kills k=UnitStruct.get(u, kills.typeid)
It would be better to make the static members that return struct type to be automatically overloaded to return proper type in extended structs.
(or at least that is how all 'real' OO languages do it)

Collapse JASS:
               if(r.getType()!=typ) then
                   set .oohmagic=u
                   set r.next=interf.create(typ) //this interface create thing uses TriggerEvaluate...
                   set r=r.next
               endif


That is the worst case of type safety abuse I saw in a long time (and I've been MVB programmer once)

Besides this whole thing is non-optimal not because of search method but because you create all structs that extend UnitStruct on get.

The way to do it properly would be to make UnitStruct onDestroy method call destroy methods of all subtypes that have been created on that index.

That way you would not need that MAX_STUFF shit because all data you need would already be in st__UnitStruct_onDestroy
So you just do a loop from 1 to total_number_of extensions (which sadly you currently don't generate by jasshelper) and call st__UnitStruct_onDestroy[i]

But how would you know if extended struct is allocated so it be safe to destroy it? I did not figure that one, but I am sure you can.

Oh, and using parallel array will always kick ass out of any method when it comes to unit indexing.
I mean it is even easier with structs, you just have to declare one unit member variable.
07-12-2008, 12:38 PM#3
cohadar
Expand JASS:

Expand JASS:


And you use them like this:
Collapse JASS:
//! runtextmacro PUI_PROPERTY("private", "integer", "KillCounter", "0")


//===========================================================================
private function KillConditions takes nothing returns boolean
    local unit killer = GetKillingUnit()
    
    if killer == null then
        return false
    endif
    
    // it does not get much cooler than this.
    set KillCounter[killer] = KillCounter[killer] + 1
    
    set killer = null
    return false
endfunction

Lets imagine we need to attach this struct to a spell
Collapse JASS:
private struct UnitData
    private timer t
    unit caster
    unit target
    real x
    real y
    integer counter
     
    //-----------------------------------------------------------------------
    static method create takes unit caster, unit target, real x, real y returns UnitData
        local UnitData dat = UnitData.allocate()
        set dat.t = NewTimer()
        set dat.caster = caster
        set dat.target = target
        set dat.x = x
        set dat.y = y
        set dat.counter = 0
        return dat
    endmethod
    
    //-----------------------------------------------------------------------
    private method onDestroy takes nothing returns nothing
        call ReleaseTimer(.t)
    endmethod
endstruct

Collapse JASS:
//! runtextmacro PUI_STRUCT("private", "UnitData", "UnitDataArray")

//===========================================================================
private function Actions takes nothing returns nothing
    local location temp = GetSpellTargetLoc()
    local UnitData dat = UnitDataArray[GetTriggerUnit()]

    // is struct attached?
    if dat == 0 then
        // create new struct
        set dat = UnitData.create(GetTriggerUnit(), GetSpellTargetUnit(), GetLocationX(temp), GetLocationY(temp)) 
        set UnitDataArray[GetTriggerUnit()] = dat // attach to unit
    else
        // load new data to already attached struct
        set dat.target = GetSpellTargetUnit()
        set dat.x = GetLocationX(temp)
        set dat.y = GetLocationY(temp) 
    endif

    call DoSomeShit(dat)
        
    call RemoveLocation(temp)
    set temp = null
endfunction

=====================================================
So it is simple to use and it is efficient.
Unit Attaching == OWNED

Of course it would be even cooler if structs could be declared static so jasshelper does not generate
integer array si__UnitDataArray_V and similar.
But since that arrays are never used == no memory loss I guess it is fine the way it is.
07-12-2008, 12:58 PM#4
Vexorian
Quote:
It would be better to make the static members that return struct type to be automatically overloaded to return proper type in extended structs.
(or at least that is how all 'real' OO languages do it)
Or, I could leave it that way, then I won't have to generate new copies for all static methods in extended structs and don't have to code it.

Quote:
Besides this whole thing is non-optimal not because of search method but because you create all structs that extend UnitStruct on get.
You only create the ones you need.

Quote:
That way you would not need that MAX_STUFF shit because all data you need would already be in st__UnitStruct_onDestroy
So you just do a loop from 1 to total_number_of extensions (which sadly you currently don't generate by jasshelper) and call st__UnitStruct_onDestroy[i]
Multiple instances cannot never share an id, I wouldn't change jasshelper's ways just to make this work, would require me to have backwards priorities, and I really have no idea on where to start to allow this change, the whole MAX_STUFF thing is not that bad anyway.

I think the only way to do this and keep .create()/.destroy() would be modifying jasshelper to allow it, or you would need to use the internal struct arrays and make the script break when a new jasshelper versions changes one of the internal arrays' name or availability.

Quote:
But how would you know if extended struct is allocated so it be safe to destroy it? I did not figure that one, but I am sure you can.
With current jasshelper you need a linked list. Either way, it is not really a problem, on destruction you would call the destroy() for every struct type that was attached to the unit. Both methods do the same. The only issue is instance limit. But I saw that even if two systems were attaching 2 structs to every unit and spells were randomly increasing it for one unit the limit remains around 2000. Edit: This reminds me this thing would also need a way to remove a single attached struct so that it doesn't use space anymore - most spells would be able to work like that - besides of a O(1) fix before it is usable, I think it is possible to do it allowing .destroy() to be called by the user.

Even if I could make the same index share multiple instances of every type (which right know I'd say requires intense hacking) there's still the question to know if a type was already initialized, another thing that requires a hash. Nevertheless if you had many units, you can just increase the constants, it gets mildly slower (A function call is the least of this thing's problems) . So I think, if there was a fast way to know the position of an already attached type, it would be nice.

Quote:
=====================================================
So it is simple to use and it is efficient.
Unit Attaching == OWNED

You are using a freaking textmacro there. It is not that bad of an idea though. I would actually use PUI with that stuff, if it wasn't for the textmacro, of course.

I think jasshelper needs a way to clone structs based in some template. I like this abuse of operators so that they look like PUI things, all it needs to look better is a way to make them without a textmacro.

Quote:
Of course it would be even cooler if structs could be declared static so jasshelper does not generate
integer array si__UnitDataArray_V and similar.
But since that arrays are never used == no memory loss I guess it is fine the way it is.
If you do not use .allocate() or .destroy() or .getType() I am quite sure the optimizer will find out and wipe those functions plus the variables that are only used by them. Though I should add static structs, cause I keep using stuff like that.

Quote:
Collapse JASS:
    // it does not get much cooler than this.
    set KillCounter[killer] = KillCounter[killer] + 1
Yeah, reminds me of Table, but, one question ARE you cohadar? The cohadar I know would have never done this.

..
I personally wish there was a way to just put labels on units something like killer.KillCounter though since the label could as well be private . would not seem appropiate, killer->killCounter ? killer["killCounter"] I think we could enable -> and then ollow operator overloading on it just to allow postfix. Haha, I can count 4 guys that hate the idea already.

Quote:
That is the worst case of type safety abuse I saw in a long time (and I've been MVB programmer once)
I know, I am excited.


Collapse JASS:
//===========================================================================
//  blah blah blah
//===========================================================================
//! textmacro PUI_PROPERTY takes VISIBILITY, TYPE, NAME, DEFAULT
$VISIBILITY$ struct $NAME$
    private static unit array Unit
    private static $TYPE$ array Data
    
    //-----------------------------------------------------------------------
    //  Returns default value when first time used
    //-----------------------------------------------------------------------
    static method operator-> takes unit whichUnit returns $TYPE$
        local integer pui = GetUnitIndex(whichUnit)
        if .Unit[pui] != whichUnit then
            set .Unit[pui] = whichUnit
            set .Data[pui] = $DEFAULT$
        endif
        return .Data[pui]
    endmethod
    
    //-----------------------------------------------------------------------
    static method operator->= takes unit whichUnit, $TYPE$ whichData returns nothing
        local integer pui = GetUnitIndex(whichUnit)
        set .Unit[pui] = whichUnit
        set .Data[pui] = whichData
    endmethod
endstruct
//! endtextmacro

Collapse JASS:
//! runtextmacro PUI_PROPERTY("private", "integer", "KillCounter", "0")


//===========================================================================
private function KillConditions takes nothing returns boolean
    local unit killer = GetKillingUnit()
    
    if killer == null then
        return false
    endif
    
    // it does not get much odder than this.
    set killer->KillCounter = killer->KillCounter + 1 //!?
    
    set killer = null
    return false
endfunction
blah -> is looking a little ugly, could just use . or let [] abuse stay.

--
Edit 1e6:
I really like PUI_STRUCT, it is convincing me that I need to do something about the struct cloning, if you could just add an "extends pui_struct" in a way that it doesn't make it use an allocator but instead inherit the static methods, or something.
07-12-2008, 01:50 PM#5
Vexorian
too much edits.

Consider this alternative:

Expand PUI_STRUCT:

Expand A pui_struct:

Expand usage:

Edit: updated PUI_STRUCT added release() a little messy.

The runtextmacro is so small and simple now, it would be like adding an option to the struct, the rest are just tidbits that are not important.
07-12-2008, 03:06 PM#6
cohadar
Hay this is nice:
Collapse JASS:
private struct UnitData
    //! runtextmacro PUI_STRUCT()

The farthest I have gone in that direction is:
Collapse JASS:
private struct UnitData
    //! runtextmacro PUI_STRUCT("UnitData")
But I did not find the way to remove all variables.

...wait a sec... $TYPE$ <-----<<

Damn it, I was so happy for a sec.


======================================
May I suggest a crazy thing?
Collapse JASS:
private unit struct // <--- note the keyword
    integer killcounter = 0
endstruct

And than you make jasshelper generate [] and []= methods.
Ok I know what you mean, change jasshelper to fit custom system are you NUTS!
But it is not like that, since [] and []= methods would be using GetUnitIndex function it does not matter where that function is declared.
If at some point someone decides to use some other system than PUI he only needs to make different GetUnitIndex function.
.....
It sounds awful even to me.

EDIT:
Templates ftw.
07-12-2008, 03:14 PM#7
Vexorian
It is a typo, it can work just as well with integer there. Edit: Still, it would be nice to have a way to use the current type name without explicitely telling it.
07-12-2008, 03:40 PM#8
cohadar
No it could not because of .destroy() call

Ok so this is the best we got so far:
Collapse JASS:
//===========================================================================
//  Never destroy PUI structs directly.
//  Let the recycling destroy them or set StructName[unit]= 0
//===========================================================================
//! textmacro PUI takes NAME
    private static unit array Unit
    private static $NAME$ array Data
    
    //-----------------------------------------------------------------------
    //  Returns zero if no struct is attached to unit
    //-----------------------------------------------------------------------
    static method operator[] takes unit whichUnit returns $NAME$
        local integer pui = GetUnitIndex(whichUnit)
        if .Data[pui] != 0 then
            if .Unit[pui] != whichUnit then
                set  .Unit[pui] = whichUnit
                call .Data[pui].destroy()
                set  .Data[pui] = 0
            endif
        endif
        return .Data[pui]
    endmethod
    
    //-----------------------------------------------------------------------
    static method operator[]= takes unit whichUnit, integer whichData returns nothing
        local integer pui = GetUnitIndex(whichUnit)
        set .Unit[pui] = whichUnit
        if .Data[pui] != 0 then
            call .Data[pui].destroy()
        endif
        set .Data[pui] = whichData
    endmethod
//! endtextmacro

Collapse JASS:
//===========================================================================
private struct UnitData
    //! runtextmacro PUI("UnitData")
   
   // blah blah
endstruct

I just forgot to obfuscate the array names.. but that is the least of problems now.


EDIT:
You know Vex you don't have to create full templates,
I we had only ONE template parameter we could give to structs it would be ownage.
07-12-2008, 03:58 PM#9
Vexorian
Quote:
Collapse JASS:
//===========================================================================
//  Never destroy PUI structs directly.
//  Use .release() instead, will call .destroy()
//===========================================================================
//! textmacro PUI_STRUCT
//===========================================================================
//  Never destroy PUI structs directly.
//  Let the recycling destroy them or set $NAME$[unit]= 0
//===========================================================================
//! textmacro PUI_STRUCT
    private static unit array pui_Unit
    private static integer array pui_Data
    private static integer array pui_id=0
    
    //-----------------------------------------------------------------------
    //  Returns zero if no struct is attached to unit
    //-----------------------------------------------------------------------
    static method operator[] takes unit whichUnit returns $TYPE$
        local integer pui = GetUnitIndex(whichUnit)
        if .pui_Data[pui] != 0 then
            if .pui_Unit[pui] != whichUnit then
                set  .pui_Unit[pui] = whichUnit
                call .destroy(pui_Data[pui])
                set  .pui_Data[pui] = .allocate()
                set .pui_id[.pui_Data[pui] ] = pui //this mess will improve if I add/figure out a way to do type casts to the current struct without typing its name.
            endif
        else
            set .pui_Data[pui] = .allocate()
            set .pui_id[.pui_Data[pui] ] = pui //this mess will improve if I add/figure out a way to do type casts to the current struct without typing its name.
        endif
        return .pui_Data[pui]
    endmethod


    static method exists takes unit whichUnit returns boolean
        local integer pui = GetUnitIndex(whichUnit)
        if .Data[pui] != 0 then
            if .Unit[pui] != whichUnit then
                set  .Unit[pui] = whichUnit
                call .destroy(.Data[pui])
                set  .Data[pui] = 0
            endif
        endif
        return (.Data[pui]!=0)
    endmethod
    //-----------------------------------------------------------------------
    static method operator[]= takes unit whichUnit, integer whichData returns nothing
        local integer pui = GetUnitIndex(whichUnit)
        set .pui_Unit[pui] = whichUnit
        if .pui_Data[pui] != 0 then
            call .destroy(.pui_Data[pui] )
        endif
        set .pui_Data[pui] = whichData
    endmethod

    method release takes nothing returns nothing
     local integer pui= .pui_id[ integer(this) ]
        set .pui_Unit[pui] = null
        set .pui_Data[pui] = 0
        call .destroy()
    endmethod

endstruct
//! endtextmacro

Quote:
I we had only ONE template parameter we could give to structs it would be ownage.
In reality adding more parameters doesn't really make it more complicated. Though in one perspective, we would prevent it from getting too complex to understand by forcing one parameter.
07-12-2008, 05:37 PM#10
cohadar
I really think one parameter would be more than enough.
I mean all we ever want to parametrize are handle types and basic types

So with just one parameter you could do amazing stuff like:
Collapse JASS:
struct List<TYPE>
    // blah blah
endstruct

struct Hast<TYPE>
    // blah blah
endstruct

struct Cache<TYPE>
    // blah blah
endstruct

declare List<handle>
declare List<integer>
declare Hash<unit>

You can even make it simpler to parse by forcing standard parameter name to be TYPE or even something like @, to make it easier to type and easier to read template code.

====================

I think I now have sufficient reasons to update PUI, see ya in 2 hours.


EDIT:
.pui_id[.pui_data[pui]] = pui
This part is so great, first pui was used as index for data and then data is used as index for pui.
It's like a porn movie with integers.


EDIT2:
Expand JASS:
I noticed you used forced get here. I disagree with that.
First because it will avoid the create method which is bad, people should be able to use PUI with structs that have non-default create.
Second because I really thing get method should return zero if no struct is attached.
(otherwise what is the point of exists method?)
Let the user create and attach the struct, it is his job anyways.
Please do tell me if you have better idea.

EDIT3:
Best version so far:
Expand JASS:
07-12-2008, 06:57 PM#11
Vexorian
Quote:
Second because I really thing get method should return zero if no struct is attached.
(otherwise what is the point of exists method?)
Actually, what's the point of exists() if [] will already return 0 when it is not attached? How about a method that does what my [] does but is named something like make() I don't know. Could make it use .create() and force the structs to all have a create that takes nothing, I guess that's what is not to like with that... Either way, I said, the most important part of the suggestion is the way the textmacro is called, the rest is all irrelevant...

--I don't get what the declare keyword does.
07-12-2008, 07:03 PM#12
cohadar
Quote:
Originally Posted by Vexorian
Actually, what's the point of exists() if [] will already return 0 when it is not attached? How about a method that does what my [] does but is named something like make() I don't know. Could make it use .create() and force the structs to all have a create that takes nothing, I guess that's what is not to like with that... Either way, I said, the most important part of the suggestion is the way the textmacro is called, the rest is all irrelevant...
Forced get is too limiting, I know it seems simpler but it is only at first sight.
What you save from checking if struct is already attached you lose on checking if unit was fresh new (from make, contains default data) or it is old (already contains valid data).

Quote:
Originally Posted by Vexorian
--I don't get what the declare keyword does.
declare ~ runtextmacro


I will remove exists method, it is just bloat.