[System] UnitDex - Unit Indexer

[TriggerHappy]

I disagree it's not better to cached handles when possible, i mean it's not a general fact.

It's common sense?

Re-use what's already there, instead of creating/destroying.

I'll consider removing them though.

 

[Troll-Brain]

Not that much, why keep handles in memory when you can destroy it just after use.
Especially since it's maybe even faster than recycling (but tests are needed if you want to get the truth, my memory is not that good at it was)

 

[TriggerHappy]

Especially since it's maybe even faster than recycling (but tests are needed if you want to get the truth, my memory is not that good at it was)

There's no way it's faster, but I do agree it's a little silly to have these static ifs.

But then again I don't.

 

[Troll-Brain]

1 native function call VS several lines of custom code, yes it can be faster.
Now again, i'm not that sure, too old.

 

[TriggerHappy]

1 native function call VS several lines of custom code, yes it can be faster.
Now again, i'm not that sure, too old.

Ah you're referring to TimerUtils?

Then yeah creating and destroying the timer may be faster, but the point is to use the cached timers instead of creating (yet another) timer at init.

 

[Troll-Brain]

No, i mean handles recycling vs handles create/destroy. (excepted costly ones, basically the "physical" handles which can interact in game, such as units, and still only in some special cases, such as dummies units)
And yeah TU is in the party, but it still gives an easy integer attachment and you couldn't if you didn't recycle (at least not in the same easy short way), so it's ok.

With recycling you realize that you will keep a timer in memory which could be never used again, right ?
While if you go for create when you need it, and destroy when you don't need it anymore, you keep memory as free as possible ?
I really don't know how to explain more :s

Now in any real case there is probably no difference (even if the jass vm don't like having many handles i've read, but never tested).
I consider so much proper to create when you need and destroy when you don't need it anymore instead of caching handles.

But oh well i don't bother that much, just trying to be accurate, as usual.
I would laugh if what i've said about handles recycling efficiency was true, because clearly that's the main point why people recycle handles.

 

[Bribe]

I would prefer a copy+pasta of Nestharus' UnitIndexer - or, better yet, manually copying it down to be justified in saying you didn't copy+pasta. This would be true Nestharus style. Oh, yeah, you have to add 1 line of code and say that your way makes it more efficient.

Nestharus once did that to my MissileRecycler. I did all the mathematics, came up with the idea, but he changed the recyling method to be slightly more efficient and did me the favor of adding backwards compatibility with my resource -_-. I thinks that's around the time I stopped coding and eventually stopped modding.

 

[TriggerHappy]

I would prefer a copy+pasta of Nestharus' UnitIndexer

Then just use his indexer (unless you were kidding)?

The API is basically the same between ours, aside from the struct so they are fairly interchangeable.

I still need to update the documentation on this..

 

[Troll-Brain]

I would prefer a copy+pasta of Nestharus' UnitIndexer - or, better yet, manually copying it down to be justified in saying you didn't copy+pasta. This would be true Nestharus style. Oh, yeah, you have to add 1 line of code and say that your way makes it more efficient.

Nestharus once did that to my MissileRecycler. I did all the mathematics, came up with the idea, but he changed the recyling method to be slightly more efficient and did me the favor of adding backwards compatibility with my resource -_-. I thinks that's around the time I stopped coding and eventually stopped modding.

Ofc if he uses my suggestions, TriggerHappy needs to give me credits for this awesome idea.
And if he doesn't i will make my own ultra more efficient unit indexer.

EDIT : @TH : Obviously Bribe was ironic, just like i did.
Hmm, or not, well at least i was

 

[TriggerHappy]

EDIT : @TH : Obviously Bribe was ironic, just like i did.

Yeah, I caught that a bit late lol.

What does anyone else think about removingoptional GroupUtils, optional TimerUtils+ associated static if's?

 

[Bannar]

I'm fine with that, yet I'm one of those who enjoys static ifs and macro usage. Not only in jass.

And: I'd rather live will less-readable code but with more functionality/adaptation ("optionals" in jass; thus we can take advantage of resources already included in map). Of course, don't forget that its your resource, so, dont be afraid to leave some flavour of urs.
->> result: if you wish to remove those optionals, go ahead. There is no real significant difference that would highly favour specific choice.

 

[TriggerHappy]

Updated.

v1.1.7
- A public unitgroup (UnitDex.Group) has been implemented to allow easy enumeration of all indexed units.
- TimerUtils removed due to module initializing.
- Documentation improved.

 

[Ruke]

this:

private module Init
	private static method onInit takes nothing returns nothing
		call RegisterUnitIndexEvent(Condition(function thistype.onIndex), EVENT_UNIT_INDEX)
		call RemoveUnit(CreateUnit(GetLocalPlayer(), 'Hpal', 0, 0, 0))
		call RegisterUnitIndexEvent(Condition(function thistype.onIndex), EVENT_UNIT_INDEX)
	endmethod
endmodule

struct test
	private static method onIndex takes nothing returns boolean
		call BJDebugMsg("indexing " + GetUnitName(GetIndexedUnit()))
		return false
	endmethod
	
	implement Init
endstruct

prints three times Paladin

 

[TriggerHappy]

this:

prints three times Paladin

Thanks for the report, it's fixed now.

v1.1.8
- Fixed a bug where events would fire an extra time on initialization.
- readonly boolean Initialized added to the UnitDex struct.

 

[Kazeon]

first, this is not user-friendly imo, and there is no point in making it configurable imo again
wait, what I was talking about? it's not in configuration part XD my bad

        constant integer EVENT_UNIT_INDEX     = 0
        constant integer EVENT_UNIT_DEINDEX   = 1

you could merge this

    function RegisterUnitIndexEvent takes boolexpr func, integer eventtype returns triggercondition
        return TriggerAddCondition(IndexTrig[eventtype], func)
    endfunction
    
    function TriggerRegisterUnitIndexEvent takes trigger t, integer eventtype returns nothing
        call TriggerRegisterVariableEvent(t, SCOPE_PRIVATE + "E", EQUAL, eventtype)
    endfunction

=>

    function TriggerRegisterUnitIndexEvent takes trigger t, boolexpr cond, code act returns nothing
        call TriggerRegisterVariableEvent(t, SCOPE_PRIVATE + "E", EQUAL, 0)
        if cond != null then
            call TriggerAddCondition(t, cond)
        endif
        if act != null then
            call TriggerAddAction(t, act)
        endif
    endfunction
    
    function TriggerRegisterUnitDeindexEvent takes trigger t, boolexpr cond, code act returns nothing
        call TriggerRegisterVariableEvent(t, SCOPE_PRIVATE + "E", EQUAL, 1)
        if cond != null then
            call TriggerAddCondition(t, cond)
        endif
        if act != null then
            call TriggerAddAction(t, act)
        endif
    endfunction

just trying to help since you have helped me alot

 

[TriggerHappy]

first, this is not user-friendly imo, and there is no point in making it configurable imo again

What's not user friendly? The constants? Yeah, you're not supposed to edit them. I should add a//DO NOT EDIT BELOW THIS LINE

The system is completely user friendly because the API mimics standard JASS functions. There should be no confusion.

you could merge this

There is absolutely no point in that. In fact it would sacrifice inlining which would significantly reduce performance and break any backwards compatibility there was.

just trying to help since you have helped me alot

Appreciate it.

 

[Kazeon]

What's not user friendly? The constants? Yeah, you're not supposed to edit them. I should add a //DO NOT EDIT BELOW THIS LINE

yeah, my post has been edited.. my bad

what's inline?

 

[TriggerHappy]

what's inline?

Consider the following code

function Foo takes nothing returns real
    return 5.
endfunction

local real r = Foo()

JassHelper (the vJass compiler) will try to optimize certain functions by inlining them which would make the above code result in:

local real r = 5.

http://www.wc3c.net/vexorian/jasshelpermanual.html#inl

 

[Ruke]

your events can have recursion problems.

edit: you have a possible leak on onEnter and onLeave

this

            static if (thistype.DEFAULT_HASHTABLE) then
                set thistype.Hash=InitHashtable()
            endif

on top

static integer LastIndex // UnitRecycleId needs access should be readonly

also, you should add a function to (de)index manually.

 

[TriggerHappy]

your events can have recursion problems.

What?

edit: you have a possible leak on onEnter and onLeave

What leaks?

static integer LastIndex // UnitRecycleId needs access should be readonly

The comment clearly says UnitRecycleId needs access which is why it's not readonly.

also, you should add a function to (de)index manually.

UnitRecycleId.

 

[Ruke]

What?

http://www.hiveworkshop.com/forums/graveyard-209/custom-event-data-187478/

What leaks?

check the returns

The comment clearly says UnitRecycleId needs access which is why it's not readonly.

you have an error design then. remember that anyone can edit that variable (which is not what you want here).

UnitRecycleId.

and for index?

 

[TriggerHappy]

you have an error design then. remember that anyone can edit that variable (which is not what you want here).

I'm aware of this.

and for index?

What do you think it recycles?

 

[Ruke]

what i'm saying is, deindex(unit) -> is_indexed(unit) == false

 

[TriggerHappy]

what i'm saying is, deindex(unit) -> is_indexed(unit) == false

and I'm saying that's exactly what UnitRecycleId does. If it's not working then it's probably because I don't reset the units user data.

It was UnitRecycleIndex before, not sure why I changed it.

 

[Ruke]

i see, also check that you're not removing the stored data from the hashtable (in UnitRecycleId)

 

[Bannar]

I've thought its already there.
Just add:

local integer prev = LastIndex
// bla bla
set LastIndex = t
// bla bla
set LastIndex = prev

 

[Deleted member 219079]

which is better this or PUI

 

[PurgeandFire]

which is better this or PUI

It depends on what you need.

PUI is better if you just need your units to be indexed. But its deindexing method isn't that great (it periodically checks if the unit exists, or it deindexes on death or w/e).

This will give you access to index events (e.g. when a unit enters a map, and after it has been indexed), and it gives you access to deindex events. If you don't need those events, you can use PUI. If you need them, you can use this. Either way, the difference is kind of negligible.

 

[Deleted member 219079]

Don't need those methods, guess I'll stick to pui.
oh pui's deindexing is bad? Guess it's not that perfect after all

but impressive system after all

 

[Ruke]

It depends on what you need.

PUI is better if you just need your units to be indexed. But its deindexing method isn't that great (it periodically checks if the unit exists, or it deindexes on death or w/e).

This will give you access to index events (e.g. when a unit enters a map, and after it has been indexed), and it gives you access to deindex events. If you don't need those events, you can use PUI. If you need them, you can use this. Either way, the difference is kind of negligible.

Both can have events.
Deindex can be the same on both methods.

The difference is that a unit will only be indexed if you require its id. Just that.

 

[PurgeandFire]

Both can have events.
Deindex can be the same on both methods.

The difference is that a unit will only be indexed if you require its id. Just that.

Both can have events. And the deindex can be the same. But PUI doesn't do that. PUI is not a method, it is a system. Of course, you can extend the system to have those events, but that wasn't his question.

 

[Geshishouhu]

Since this resource hasn't been approved, is it safe to use it? How does it compare with Bribe's Unit Indexer?

 

[Bannar]

Bribe's indexer does almost everything in the same way - the main difference is that it is written in GUI so you a) dont require JNPG b) don't have script code -> you have triggers which GUI-ers love instead

This is lightweight, especially compared to new Nes monstrum. Differences between most available indexers are negligible since most of those also contain index/deindex event.

I see there has been discussion around PUI. My personaly opinion: I find PUI the worst out of: AutoIndex, AIDS, PUI, Nes UI --5, Nes UI new, TH UnitDex, Bribes GUI UI. I just don't like periodic recycling since we can avoid it, and the approach could be better. Anyway, probably because of mistakes made by cohadar and others, new indexers emerged.

This resource, even that it is not approved works properly and is fully functional. I have tested this one multiple times, you can even read the earlier posts in regard to previous issues I've found. TH already fixed everything. The last thing he could add at the top is AIDS - support which I've written for him. It doesn't need such for UI of Nes since its API is almost the same. AIDS is probably the 3rd most popular indexer, thus its nice to have such compatibile script around

 

[Geshishouhu]

Bribe's indexer does almost everything in the same way - the main difference is that it is written in GUI so you a) dont require JNPG b) don't have script code -> you have triggers which GUI-ers love instead

This is lightweight, especially compared to new Nes monstrum. Differences between most available indexers are negligible since most of those also contain index/deindex event.

I see there has been discussion around PUI. My personaly opinion: I find PUI the worst out of: AutoIndex, AIDS, PUI, Nes UI --5, Nes UI new, TH UnitDex, Bribes GUI UI. I just don't like periodic recycling since we can avoid it, and the approach could be better. Anyway, probably because of mistakes made by cohadar and others, new indexers emerged.

This resource, even that it is not approved works properly and is fully functional. I have tested this one multiple times, you can even read the earlier posts in regard to previous issues I've found. TH already fixed everything. The last thing he could add at the top is AIDS - support which I've written for him. It doesn't need such for UI of Nes since its API is almost the same. AIDS is probably the 3rd most popular indexer, thus its nice to have such compatibile script around

Bannar, you are my savior!!!! I wish I could give you 100 Rep for your help! I like lightweight resources and since this resource is also bug-free, I will probably use this one. Thanks again !

 

[Deleted member 219079]

My personaly opinion: I find PUI the worst out of: AutoIndex, AIDS, PUI, Nes UI --5, Nes UI new, TH UnitDex, Bribes GUI UI. I just don't like periodic recycling since we can avoid it, and the approach could be better. Anyway, probably because of mistakes made by cohadar and others, new indexers emerged.

lol.

 

[chobibo]

Cohadar didn't like the "defend event catching" solution like how you don't like his method too. Also, that solution wasn't unknown to cohadar at that time, people were actually convincing him to use the "defend event catching" solution but he had an argument against it.

EDIT: I'd also want to add that he was the one who originally questioned the existing systems back then (mostly customized for maps and not publicly available). Like how things would fail if a handle id was assigned formerly was re-assigned again, as handle id's were used as checks back then. He also proposed that on deindexing, an event should be fired to clear the data assigned to the unit index via structs and other shit.

 

[TriggerHappy]

Updated main post with compatibility libraries for AIDS and UnitIndexer.

 

[Bannar]

I'm glad you've finally added that ^)^

 

[TriggerHappy]

I'm glad you've finally added that ^)^

Just added AutoIndex support as well.

They're unfinished but I will complete them later.

 

[BPower]

I couldn't find something wrong in here. I'm up for approval, any arguments against it?

It's a good and working unit indexing tool without requirements.
The defend ability abuse is long time known and works as implemented by TriggerHappy into UnitDex.

UnitIndexer ( Nes ), became very complex and may be too complicated for the average user.
Many just want to abuse UnitUserData to allocate instances directly or fire here and there an onIndex/Deindex event.
The extreme modularity and multiple different interfaces of current UnitIndexer remain unused in 95% of all cases. ( I guess )
This one offers easy implementation, covering all basics of an indexing tool.

Bribes Unit Indexer is a different appraoch when it comes to cleanup, as it runs with a periodic timer callback.

Ergo I vote for approval

 

[IcemanBo]

I use it currently in here: http://www.hiveworkshop.com/forums/submissions-414/snippet-getunitscale-262274/

And I had problems with it.

The onIndex event doesn't seem to fire for the last indexed unit.
So the unit with highest index won't run the event.

Here a little demo attached:
(it should print 4x footman, but only does 3)

 

[BPower]

Ok I will test it. I think there is no chance getting TriggerHappy to fix it.

 

[edo494]

not to mention Bribes is in GUI in spell section, and this is in vJass in Jass section

 

[btdonald]

Ok I will test it. I think there is no chance getting TriggerHappy to fix it.

I got same problem with IceManBo

 

[btdonald]

Hashtable Index seem doesn't work. It's return index 0 at all.

 

[Ruke]

i really disagree on using modules as a configuration medium

 

[Bribe]

Here's a list of bugs with this resource:

Registering a unit index event will not fire for any preplaced units. This happens because the map-wide initialization is done within a module. It is impossible to reconcile this without using an IsGameStarted system as Nestharus implemented for his resource.

Leaks indices when paused units are removed from transport.

Compatibility resources have lock methods that do nothing.


Here's issues I have with it beyond those:

You don't need a unit group in this resource for storing variables. Just toss it. Some other resource could be built if a user decides it to add unit to group during index and remove during deindex.

If you're going to use a hashtable, the reason is not because you're at risk of going over 8190 indices - it is because you're using UnitUserData elsewhere in the map. That's why Nestharus didn't bother to include that option, because you should not be messing with UnitUserData unless it's for a unit who you deliberately didn't index.

Locks are kind of unneeded, but if you intend to bridge compatibility with AIDS and Nes' indexer, you'll need them.

This resource doesn't need to exist. AIDS, AutoIndex and UnitIndexer exist for vJass already. AutoIndex is by far the slowest, but least buggy due to hooking RemoveUnit. AIDS is more adopted than this and offers approximately the same, give or take a feature here or there.

For GUI users, GUI Unit Indexer exists. It has no bugs because it doesn't use undefend. The undefend method is strictly for GUI Unit Event - where it belongs. If you want a vJass version of GUI Unit Indexer, use http://www.hiveworkshop.com/forums/...unit-indexer-vjass-plugin-268592/#post2716401

We don't need another Unit Indexer "just because". If you want to implement these changes I mentioned, you'd end up with a faster AutoIndex but without silly function interfaces.

 

[Bribe]

Graveyarding until author is ready to update.

 

[TriggerHappy]

I updated this because of personal use.

Bugs are fixed you can un-graveyard this now.

I use it currently in here: http://www.hiveworkshop.com/forums/submissions-414/snippet-getunitscale-262274/

And I had problems with it.

The onIndex event doesn't seem to fire for the last indexed unit.
So the unit with highest index won't run the event.

Here a little demo attached:
(it should print 4x footman, but only does 3)

Fixed.

Registering a unit index event will not fire for any preplaced units. This happens because the map-wide initialization is done within a module. It is impossible to reconcile this without using an IsGameStarted system as Nestharus implemented for his resource.

Fixed.

Leaks indices when paused units are removed from transport.

and how do I fix that?

Compatibility resources have lock methods that do nothing.

I don't even think I made those scripts, but I assume it's to prevent compile errors. I'm not sure how bad that could break someone's code.

You don't need a unit group in this resource for storing variables. Just toss it. Some other resource could be built if a user decides it to add unit to group during index and remove during deindex.

I suppose but I think the idea was that if multiple scripts wanted to enumerate all units they wouldn't have to each populate a separate group.

If you're going to use a hashtable, the reason is not because you're at risk of going over 8190 indices - it is because you're using UnitUserData elsewhere in the map. That's why Nestharus didn't bother to include that option, because you should not be messing with UnitUserData unless it's for a unit who you deliberately didn't index.

Good point. I still like the hashtable option though and it's already implemented so..

Locks are kind of unneeded, but if you intend to bridge compatibility with AIDS and Nes' indexer, you'll need them.

ill look into it

This resource doesn't need to exist. AIDS, AutoIndex and UnitIndexer exist for vJass already. AutoIndex is by far the slowest, but least buggy due to hooking RemoveUnit. AIDS is more adopted than this and offers approximately the same, give or take a feature here or there.

We don't need another Unit Indexer "just because". If you want to implement these changes I mentioned, you'd end up with a faster AutoIndex but without silly function interfaces.

I don't really care if this gets approved, but I don't agree with this.

AutoIndex is outdated
AIDs requires textmacros and is just odd to me
UnitIndex is harder to find and has like 7 dependencies

I also think my API is more JASS-like.

i really disagree on using modules as a configuration medium

Why?

 

[Nestharus]

Ehhh...

If you allocate using a queue instead of a stack, locks aren't really necessary. Locks were really there for lazy people that didn't want to clean things up while a spell was running or something and a unit was removed. It's convenience really. Queue does the same thing more or less ; ).

As for the group, only one script would need to do this. From here, other scripts could use it. It does raise dependencies though, so it's give and take. For me, I have no problem with 10000000 dependencies, but I know that other people do : ). This would be your own personal choice I guess. I think a list is more useful than a group.