• 🏆 Texturing Contest #33 is OPEN! Contestants must re-texture a SD unit model found in-game (Warcraft 3 Classic), recreating the unit into a peaceful NPC version. 🔗Click here to enter!
  • It's time for the first HD Modeling Contest of 2024. Join the theme discussion for Hive's HD Modeling Contest #6! Click here to post your idea!

[System] UnitDex - Unit Indexer

Level 17
Joined
Apr 27, 2008
Messages
2,455
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

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,456
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.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
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 :p
 

Bannar

Code Reviewer
Level 26
Joined
Mar 19, 2008
Messages
3,140
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.
 
Level 10
Joined
Sep 19, 2011
Messages
527
this:

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

Kazeon

Hosted Project: EC
Level 33
Joined
Oct 12, 2011
Messages
3,449
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
JASS:
        constant integer EVENT_UNIT_INDEX     = 0
        constant integer EVENT_UNIT_DEINDEX   = 1

you could merge this
JASS:
    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
=>
JASS:
    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 :D
 
Dalvengyr said:
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.

Dalvengyr said:
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.

Dalvengyr said:
just trying to help since you have helped me alot :D

Appreciate it. :thumbs_up:
 
Level 10
Joined
Sep 19, 2011
Messages
527
your events can have recursion problems.

edit: you have a possible leak on onEnter and onLeave

this

JASS:
            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.
 
Level 10
Joined
Sep 19, 2011
Messages
527
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

D

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

but impressive system after all :)
 
Level 10
Joined
Sep 19, 2011
Messages
527
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.
 
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.
 

Bannar

Code Reviewer
Level 26
Joined
Mar 19, 2008
Messages
3,140
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 :)
 
Level 11
Joined
Oct 11, 2012
Messages
711
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!:ogre_haosis: I like lightweight resources and since this resource is also bug-free, I will probably use this one. Thanks again !
 

Deleted member 219079

D

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.
 
Level 22
Joined
Sep 24, 2005
Messages
4,821
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.
 
Level 19
Joined
Mar 18, 2012
Messages
1,716
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 :D
 

Attachments

  • Demo.w3x
    31.4 KB · Views: 110

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,456
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.
 
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?
 
Level 31
Joined
Jul 10, 2007
Messages
6,306
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.
 
Top