System PUI - Perfect Unit Indexing

Cohadar

master of fugue
Reaction score
209
Well, I think you'll need over 8000 living units on the map for that to happen.
No, dead units can also have indexes.

Hm, basicly I use the GetUnitIndex() on any unit that comes in range of 2000 of one of 8 player controlled heroes (required for the aggro system i use).
I know that when a unit dies, the index gets recycled ... however; I have a trigger that removes the unit from the aggro system when the unit leaves a certain distance to all player heroes (to "clean" the system up and avoid having too much units on the aggro system). This trigger would be perfect to also remove the unit index, since the index is only used for the aggro system at my map.

I didn't check the code of PUI completely (because I trust you coding skills :D). Is it a problem if there are hundrets of units indexed? I mean; are there loopcalls, handle creations, etc. that will increase the memory usage or processing power usage? My RPG has an enormous size (480x480) and thus also has a lot of creeps. I just don't want to use up all the 8190 handle slots so fast with loads of units indexed.

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.
 

Zwiebelchen

You can change this now in User CP.
Reaction score
60
Alright, I'll leave the indexes be then.
Hope I will not reach the maximum number some time.
 

Cohadar

master of fugue
Reaction score
209
Tip: If you want to speed up index recycling reduce the decay time of corpses.
Gameplay Constants -> Decay Time - Bones: 20
 

Zwiebelchen

You can change this now in User CP.
Reaction score
60
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.
 

Cohadar

master of fugue
Reaction score
209
You can find answers to both of your questions in PUI Tutorial as well as in the demo map.
 

Romek

Super Moderator
Reaction score
963
You could use modules to replace the PUI textmacro. :)

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

Cohadar

master of fugue
Reaction score
209
You could use modules to replace the PUI textmacro. :)

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

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.
 

Romek

Super Moderator
Reaction score
963
> 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
 

SerraAvenger

Cuz I can
Reaction score
234
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.
 

Romek

Super Moderator
Reaction score
963
> 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
 

Builder Bob

Live free or don't
Reaction score
249
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.
 

Cohadar

master of fugue
Reaction score
209
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
 

Builder Bob

Live free or don't
Reaction score
249
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

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.
 

Cohadar

master of fugue
Reaction score
209
Ok I gave this a detailed look.

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?
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.

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 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
 

Builder Bob

Live free or don't
Reaction score
249
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.

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.
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 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.
===========================

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
 

Attachments

  • PUITest.w3x
    24.7 KB · Views: 297

Cohadar

master of fugue
Reaction score
209
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.
 

Builder Bob

Live free or don't
Reaction score
249
Wow that was really nice testmap.
So linked lists solved the problem hummm.
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.

The thing about this "problem" is that noone on this planed had it except you. :D
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 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.

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
 
General chit-chat
Help Users
  • No one is chatting at the moment.

      The Helper Discord

      Members online

      No members online now.

      Affiliates

      Hive Workshop NUON Dome World Editor Tutorials

      Network Sponsors

      Apex Steel Pipe - Buys and sells Steel Pipe.
      Top