System PUI - Perfect Unit Indexing

Discussion in 'Systems and Snippets' started by Cohadar, Dec 1, 2007.

  1. Cohadar

    Cohadar master of fugue

    Ratings:
    +209 / 0 / -0
    No, dead units can also have indexes.

    PUI does not create any handles it uses constant amount of memory at all times.
    I really doubt you can break 8190 unit limit but please do try.
    (Put say 7000 units on the map and see what happens :p)

    PS: I strongly advise you do NOT manually remove unit indexes because sooner or later you will want to use PUI for something else than aggro and than it will be a problem.
     
  2. Zwiebelchen

    Zwiebelchen You can change this now in User CP.

    Ratings:
    +60 / 0 / -0
    Alright, I'll leave the indexes be then.
    Hope I will not reach the maximum number some time.
     
  3. Cohadar

    Cohadar master of fugue

    Ratings:
    +209 / 0 / -0
    Tip: If you want to speed up index recycling reduce the decay time of corpses.
    Gameplay Constants -> Decay Time - Bones: 20
     
  4. Zwiebelchen

    Zwiebelchen You can change this now in User CP.

    Ratings:
    +60 / 0 / -0
    Btw, got a question to struct attachment on PUI:

    If I don't want to use a local struct in my function, is something like this already enough to check, wether a struct is already attached to a unit?

    JASS:
    
    if UnitProps[GetTriggerUnit()] == 0 then
                set UnitProps[GetTriggerUnit()] = UnitProps.create()
    endif

    And yes, I have already run the textmacro on the struct.



    Also, I have a question on using global struct attachment on PUI:

    Lets say I have a trigger that Initializes my global arrays, such as the PUI Struct.
    I add the PUI Textmacro.
    Then I want to have the struct in another trigger. How can I adress it? If I use the simple
    JASS:
    set Data = StructName[Unit]

    It tells me, that the struct Data does not allow arrayed variables.

    Hm, maybe I just made a mistake.

    Though it works when using PUI_PROPERTY, but it would be better if I could use structs instead of Propertys ...

    However, it works when the Struct declaration of the attached struct type is in the same trigger.
     
  5. Cohadar

    Cohadar master of fugue

    Ratings:
    +209 / 0 / -0
    You can find answers to both of your questions in PUI Tutorial as well as in the demo map.
     
  6. Joker(Div)

    Joker(Div) Always Here..

    Ratings:
    +86 / 0 / -0
    Just stopping by to say this saves so much time.
     
  7. Romek

    Romek Super Moderator Staff Member

    Ratings:
    +961 / 0 / -0
    You could use modules to replace the PUI textmacro. :)

    A little code (3 lines) to not break older scripts could also be included.
     
  8. TERRYperfect

    TERRYperfect New Member

    Ratings:
    +0 / 0 / -0
    If a spell is using a former version of PUI, will errors occur when I changed the PUI to the latest version?
     
  9. Tyrande_ma3x

    Tyrande_ma3x .

    Ratings:
    +91 / 0 / -0
    No, the system is 100% backwards compatible - there is only a new feature and that's the textmacros.
     
  10. Cohadar

    Cohadar master of fugue

    Ratings:
    +209 / 0 / -0
    I had no time to keep up with recent jasshelper updates.
    When I get the time I will decide if it is worth the hussle.

    IMHO Vexorian is experimenting too much with jasshelper,
    he should have declared version 1.0.0.0 two years ago.
     
  11. Romek

    Romek Super Moderator Staff Member

    Ratings:
    +961 / 0 / -0
    > I had no time to keep up with recent jasshelper updates.
    I check occasionally, though I saw this, and though it'd be perfect for PUI.

    > he should have declared version 1.0.0.0 two years ago.
    I don't think anyone disagrees with that. :p
     
  12. SerraAvenger

    SerraAvenger Cuz I can

    Ratings:
    +236 / 0 / -0
    due to the new JassHelper update, this should be a fix you certainly need soon:

    JASS:
    //REQUIRES: JassHelper 0.9.G.0 or newer! 
    // add this. Integers are deprecated for using as structs, some sort of "type safety" our dear vex is currently fighting for.
    
    //! textmacro PUI
        implement PUI
    //! endtextmacro
    
    module PUI
        private static unit    array pui_unit
        private static thistype 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 thistype
            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
        
        //-----------------------------------------------------------------------
        //  This will overwrite already attached struct if any
        //-----------------------------------------------------------------------
        static method operator[]= takes unit whichUnit, thistype 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, the 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
    endmodule


    EDIT: Ouch, definetly beaten by Romek. I should read threads before I reply >_>
    You will still have to fix the "returns integer" in the PUI textmacro. thistype is required instead.
     
  13. Romek

    Romek Super Moderator Staff Member

    Ratings:
    +961 / 0 / -0
    > static method operator[]= takes unit whichUnit, integer whichData returns nothing
    static method operator[]= takes unit whichUnit, thistype whichData returns nothing

    > private static integer array pui_data
    private static thistype array pui_data

    And a textmacro for backwards compatibility:
    JASS:
    //! textmacro PUI
    implement PUI
    //! endtextmacro
     
    • Like Like x 1
  14. Builder Bob

    Builder Bob Live free or don't

    Ratings:
    +249 / 0 / -0
    I was working on a map when I noticed PUI wasn't recycling units like I expected it to.

    After looking into it trying to figure out why that was, I noticed that it was because of my very high rate of creating and removing units with a PUI index.

    When a large number of units with an index are created and removed in a short period of time, PUI will put indexes in a decaying state, but will not free them up until the creation and removal of new units end to let PUI catch up.

    Is this working as intended?


    By the way, I realize creating and removing units in this manner probably is a bad idea. I am just asking if the system should be able to handle it or not.


    Edit: It seemes that when encountering a long streak of indexes to decay, PeriodicRecycler can enter a state where it won't release any indexes as long as new ones are decaying.

    By making the decay and recycle loops go in different directions, the same did not happen.
    JASS:
    set decayer = decayer + 1
    if decayer > decayindex then
        set decayer = 1
    endif

    to
    JASS:
    set decayer = decayer - 1
    if decayer < 1 then
        set decayer = decayindex
    endif


    Can I run into any other bad situations that I should know of by doing this? I'm testing a very special case scenario, so I'm not sure how much I can rely on my results.
     
  15. Cohadar

    Cohadar master of fugue

    Ratings:
    +209 / 0 / -0
    PUI has a constant rate of index recycling:
    JASS:
    
    private constant real PERIOD = 0.03125   // 32 fps


    It basically says I can recycle 32 indexes per second.

    If you create more than 32 units per second during a large period of time you will get into problems.
    The solution is of course to increase the fps.

    Notice also that unit indexes will get recycled only after unit is actually removed from the game. (not when it is killed)
    So if you have a very large corpse/bone decay times (check gameplay constants) it will hold onto indexes that you are not using.

    WARNING: do NOT set UnitUserData of dead units to zero, your spells will fail.
    Having a unit index on a corpse is needed by some spells, especially the ones of D.O.T kind.

    PS: If you give me more info on your problem I will give you more info on your solution :p
     
  16. Builder Bob

    Builder Bob Live free or don't

    Ratings:
    +249 / 0 / -0
    I thought I gave quite a bit of info on the problem. Is any of it unclear?

    The problem is solved though. By reversing the release loop I prevented a special case scenario where PUI recycles 0 indexes per second.

    Maybe this will help explain the situation:

    Code:
    topindex == 0
    decayindex == 0
    decayer == 0
    
    loop
        [add unit]
        topindex++
        [kill unit]
        decayindex++
        decayer++
    endloop
    decayer > decayindex will never become true as long as there are more removed units having their index in the process of being recycled. Thus, no indexes will be released. New units created will be assigned fresh indexes instead of getting recycled ones as they should have, clogging up the system.

    This will only happen in heavy strain situations, and might not be very realistic. However, I think it's a weakness.
     
  17. Cohadar

    Cohadar master of fugue

    Ratings:
    +209 / 0 / -0
    Ok I gave this a detailed look.

    No, but PUI does not work the way you describe it here so it is ok :p
    PUI will always remove indexes after their time expires and this time DOES NOT depend on number of units or decaying indexes currently in the system.

    This makes no sense, if decayer never gets over decayindex that means that you are creating/destroying more than 32 units per second.
    Reversing the decayer scan order will only help you temporarily until it returns to the top of decay section, which would have grown by that time even more (because as we assumed in the beginning new indexes are coming on top faster that decayer can free the old ones.)
    So this actually is not a solution, it behaves exactly the same as default algorithm.

    I sense there is something here you are not telling me.

    Did you perhaps change your INDEX_DECAY_TIME to zero?

    Do you for some reason want to prevent indexes from getting too big?
    (doing some big array matrix stuff with pui?)

    ===========================
    In the case that I simply cannot grasp exactly what are you doing here
    can you please make a demo map for me that demonstrates the effect you just described.
    ===========================

    And finally are you sure PUI is what you need in this case?
    If there is a constant(or at least bounded) number of special type units that you create/destroy you can assign them indexes "manually" and also remove them manually at the same time you remove the units.

    The best way to do this is to assign a high and specific index area to them
    for example indexes 8000-8100
    That way you make sure they don't collide with PUI indexes and also you can do checks like this:
    JASS:
    globals
        private constant integer SHIFT = 8000
    endglobals
    
    set pui = GetUnitIndex(someUnit)
    if pui > SHIFT then
        // special unit: do some magic
        // note how we can (but not must) use SHIFT with special arrays 
        // but must not use shift with arrays that are used by normal PUI units
        set SpecialUnitArray1[pui-SHIFT] = specialProperty1
        set NormalUnitArray2[pui] = normalProperty2
    else
        set NormalUnitArray1[pui] = normalProperty1
        set NormalUnitArray2[pui] = normalProperty2
    endif
     
  18. Builder Bob

    Builder Bob Live free or don't

    Ratings:
    +249 / 0 / -0
    I've been meaning to answer you before, but I wanted to take a much closer look at the whole thing so I could respond better.

    A map created to help testing special case scenarios is attached at the bottom.

    It doesn't behave exactly the same as the default. It prevents the decayer from potentially getting stuck in long streaks of checking newly decaying units. When the decayer reach the top of decay section as you say, there is a good chance those units are newly decaying units, and checking them is somewhat a waste. However, since the decayer is going backwards, it will not get stuck only checking newly decaying units at the same rate the checker push them on for decaying.

    Anyway, forget all that above. There are some additional issues that I won't bother talking about because this post is getting too long as it is. Long story short, I don't think PUI was designed for indexing at such a high rate.

    I'll just give you the test map you asked for, and propose a change to linked lists to hold the index information instead. Maybe not executed the same way as this, but with the same reasoning.

    The linked list suggestion for PUI is in the attached map. It'll insert new indexes and decay them in such a manner that the last added will be the last checked at all times.

    The reasoning behind it is:
    • The last indexed unit is the least likely to die and decay.
    • The last decaying index is the least likely to have passed the decay time.

    I'm using default INDEX_DECAY_TIME = 5.

    This map should demonstrate how PUI currently cannot recycle 32 indexes per second.

    I've not done any modifications to how PUI 5.1 functions. I've only added a text representation of up to 8 decaying and 8 non-decaying indexes to more easily follow what PUI does in it's PeriodicRecycler.
    • The number in the boxes represent the array index
    • Red boxes are decaying indexes
    • Green boxes are indexes currently in use
    • Bright green box is the one currently being checked for decaying
    • Bright red box is the one currently being checked for recycling

    Commands:
    • -units #
      Will create units to the total number of units on the map is #. If # is less than current number of units, Total - # will be killed.
    • -indexpersec #
      Will create and kill # number of units per second. Be careful not to set this too high.



    Btw: There's a bug in the decayer code:
    JASS:
    (...)
    set Indexz[decayer] = Indexz[decayindex]
    set Decayz[decayer] = Decayz[decayindex] //added
    set Tickz[decayer] = Tickz[decayindex] //added
    set Indexz[decayindex] = temp
    (...)


    Those two lines should be added to keep the decaying time for every index intact.

    Edit: Modified the linked list code a little bit and uploaded a new map
     

    Attached Files:

    • Like Like x 1
  19. Cohadar

    Cohadar master of fugue

    Ratings:
    +209 / 0 / -0
    Wow that was really nice testmap.
    So linked lists solved the problem hummm.

    The thing about this "problem" is that noone on this planed had it except you. :D

    Nice bug find, luckily it is not really a bug but a decay time "misbehaviour" so it cannot create problems. (i.e. no need for a quickfix)

    I seriously don't like your list implementation, I avoided depending on struct for good reasons.
    There is a much simpler solution. (use 2 decayers :eek:)

    Anyways you get some cookies now, and some credit later when I update PUI, good job.
     
  20. Builder Bob

    Builder Bob Live free or don't

    Ratings:
    +249 / 0 / -0
    Thanks :)

    I uploaded a new map which I think has a better solution to the decaying code. Instead of cycling through the list, you can just wait for one index to finish decaying before moving to the next. When new indexes are put into the decaying list, they will be put in the back, so this will ensure the decaying list always moves from short decay time to long decay time indexes.

    Hehe, yeah, it's more of a misbehavior than a real problem. Just like the decay time misbehavior.

    I actually discovered this by accident. I had a bouncing projectile spell which created a new unit for every bounce. Some other place in my code I was foolishly assigning unit indexes to all newly created units, regardless if they needed it or not. The bouncing spell would sometimes get stuck within a wall and consequently create lots and lots of units in a very short time, each one of them getting their own index.

    I regularly check -puistats to see if everything is working as it is supposed to, and that's when I saw PUI stopped releasing indexes. From there I went on to try to figure out why that was.

    I'd very much like to see a better implementation of the linked lists. I'm not sure how to best set it up. When I try to make linked lists without structs I tend to lose my mind.

    I'll just use what I have until you make a better version of it.

    Thanks for the cookies :D
     

Share This Page