| 07-12-2008, 05:53 AM | #1 |
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. UnitStruct:library UnitStruct initializer init globals private constant integer MAX_UNITS=8190 private constant integer MAX_STUFF=8190 //Max stuff, Max units * average structs attached to each unit. //If the average is 3, then 8190 means you can have at most //2730 units at the same time, which is probably fine? private constant integer MAX_HANDLE_COUNT = 400000 private group seengroup private integer seenn=0 private unit array seenunits[MAX_UNITS] private UnitStruct array first[MAX_HANDLE_COUNT] private constant real CLEAN_INTERVAL = 0.1 private constant integer CLEAN_MAX_CHECKS=10 private constant boolean CLEAN_IF_DEAD = true private constant integer MIN_HANDLE=0x100000 endglobals private interface interf[MAX_STUFF] static method create takes nothing endinterface private function H2I takes handle h returns integer return h return 0 endfunction private keyword next //ouch friend members through keyword suck struct UnitStruct extends interf private static unit oohmagic readonly unit attached=UnitStruct.oohmagic //I'll officiallize readonly in the next version of the manual, it is a nice feature UnitStruct next=0 static method get takes unit u, integer typ returns UnitStruct local UnitStruct r local integer h=H2I(u) if(not IsUnitInGroup(u,seengroup) ) then call GroupAddUnit(seengroup,u) set seenunits[seenn]=u set .oohmagic=u set r=interf.create(typ) set r.attached=u set first[h-MIN_HANDLE]=r set seenn=seenn+1 else set r=first[h-MIN_HANDLE] // Evil O(n) search! somebody implement a red black tree or... gamecache! loop exitwhen r.getType()==typ //getTypeId gets inlined, thank god. exitwhen r.next==0 set r=r.next endloop 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 endif return r endmethod endstruct private function destroyStuff takes UnitStruct p returns nothing local UnitStruct q loop exitwhen p==0 set q=p set p=p.next call q.destroy() endloop endfunction globals private integer check=0 endglobals private function clean takes nothing returns nothing local integer i=0 local unit u loop exitwhen (i==CLEAN_MAX_CHECKS) if(seenn==0) then return endif if(check>=seenn) then set check=0 endif set u=seenunits[check] if(IsUnitInGroup(u,seengroup) and not( CLEAN_IF_DEAD and (IsUnitType(u,UNIT_TYPE_DEAD) and not IsUnitType(u,UNIT_TYPE_HERO) ) ) ) then set i=i+1 set check=check+1 else call destroyStuff( first[ H2I(u)-MIN_HANDLE ]) set seenn=seenn-1 set seenunits[check]=seenunits[seenn] set seenunits[seenn]=null exitwhen true endif set u=null endloop endfunction private function init takes nothing returns nothing set seengroup=CreateGroup() call TimerStart(CreateTimer(), CLEAN_INTERVAL, true, function clean) endfunction endlibrary So for example: 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 |
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) 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 |
JASS://=========================================================================== // Allowed PUI_PROPERTY TYPES are: unit, integer, real, boolean, string // Do NOT put handles that need to be destroyed here (timer, trigger, ...) // Instead put them in a struct and use PUI_STRUCT //=========================================================================== //! 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 JASS://=========================================================================== // Never destroy PUI structs directly. // Let the recycling destroy them or set $NAME$[unit]= 0 //=========================================================================== //! textmacro PUI_STRUCT takes VISIBILITY, TYPE, NAME $VISIBILITY$ struct $NAME$ private static unit array Unit private static $TYPE$ array Data //----------------------------------------------------------------------- // Returns zero if no struct is attached to unit //----------------------------------------------------------------------- static method operator[] takes unit whichUnit returns $TYPE$ 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, $TYPE$ 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 endstruct //! endtextmacro And you use them like this: 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 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 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 | ||||||||
Quote:
Quote:
Quote:
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:
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:
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:
Quote:
.. 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:
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 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 -- 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 |
too much edits. Consider this alternative: PUI_STRUCT://=========================================================================== // 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, $TYPE$ 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 A pui_struct:private struct UnitData //! runtextmacro PUI_STRUCT() private timer t unit caster unit target real x real y integer counter //----------------------------------------------------------------------- static method create takes nothing returns UnitData local UnitData dat = UnitData.allocate() set dat.t = NewTimer() set dat.counter = 0 return dat endmethod //----------------------------------------------------------------------- private method onDestroy takes nothing returns nothing call ReleaseTimer(.t) endmethod endstruct usage://=========================================================================== private function Actions takes nothing returns nothing local location temp = GetSpellTargetLoc() local UnitData dat = UnitData[GetTriggerUnit()] set dat.target = GetSpellTargetUnit() //load new data ... set dat.x=GetLocationX(temp) set day.y=GetLocationY(temp) call DoSomeShit(dat) call RemoveLocation(temp) set temp = null endfunction 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 |
Hay this is nice: JASS:private struct UnitData //! runtextmacro PUI_STRUCT() The farthest I have gone in that direction is: JASS:private struct UnitData //! runtextmacro PUI_STRUCT("UnitData") ...wait a sec... $TYPE$ <-----<< Damn it, I was so happy for a sec. ====================================== May I suggest a crazy thing? 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 |
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 |
No it could not because of .destroy() call Ok so this is the best we got so far: 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 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 | ||
Quote:
Quote:
|
| 07-12-2008, 05:37 PM | #10 |
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: 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: JASS:
//-----------------------------------------------------------------------
static method operator[] takes unit whichUnit returns integer
local integer pui = GetUnitIndex(whichUnit)
if .pui_data[pui] != 0 then
if .pui_unit[pui] != whichUnit then
call .destroy(.pui_data[pui])
set .pui_unit[pui] = whichUnit
set .pui_data[pui] = .allocate()
set .pui_id[.pui_data[pui]] = pui
endif
else
set .pui_unit[pui] = whichUnit
set .pui_data[pui] = .allocate()
set .pui_id[.pui_data[pui]] = pui
endif
return .pui_data[pui]
endmethod
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: JASS://=========================================================================== // Never destroy PUI structs directly. // Use .release() instead, will call .destroy() //=========================================================================== //! textmacro PUI private static unit array pui_unit private static integer array pui_data private static integer array pui_id //----------------------------------------------------------------------- // Returns zero if no struct is attached to unit //----------------------------------------------------------------------- static method operator[] takes unit whichUnit returns integer local integer pui = GetUnitIndex(whichUnit) if .pui_data[pui] != 0 then if .pui_unit[pui] != whichUnit then // recycled handle detected call .destroy(.pui_data[pui]) set .pui_unit[pui] = null set .pui_data[pui] = 0 endif endif return .pui_data[pui] endmethod //----------------------------------------------------------------------- static method exists takes unit whichUnit returns boolean local integer pui = GetUnitIndex(whichUnit) if .pui_data[pui] != 0 then if .pui_unit[pui] != whichUnit then // recycled handle detected call .destroy(.pui_data[pui]) set .pui_unit[pui] = null set .pui_data[pui] = 0 endif endif return (.pui_data[pui]!=0) endmethod //----------------------------------------------------------------------- // This will overwrite already attached struct if any //----------------------------------------------------------------------- static method operator[]= takes unit whichUnit, integer whichData returns nothing local integer pui = GetUnitIndex(whichUnit) if .pui_data[pui] != 0 then call .destroy(.pui_data[pui]) endif set .pui_unit[pui] = whichUnit set .pui_data[pui] = whichData set .pui_id[whichData] = pui endmethod //----------------------------------------------------------------------- // If you do not call release struct will be destroyed when unit handle gets recycled //----------------------------------------------------------------------- method release takes nothing returns nothing local integer pui= .pui_id[integer(this)] call .destroy() set .pui_unit[pui] = null set .pui_data[pui] = 0 endmethod //! endtextmacro |
| 07-12-2008, 06:57 PM | #11 | |
Quote:
--I don't get what the declare keyword does. |
| 07-12-2008, 07:03 PM | #12 | ||
Quote:
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:
I will remove exists method, it is just bloat. |
