Unit Indexer omits pre-enter indexing

[BlueSaint]

import UnitIndexer
import ClosureTimers

init
    let u   = createUnit(Player(0),'hfoo',vec2(0,0),angle(0))
    print(u.toUnitIndex() castTo int.toString())
    doAfter(0,() -> print(u.getIndex().toString())) // <-- prints different index

I don't know how to create pull requests, but you just change line 38 in UnitIndexer to: onEnter(() -> getEnterLeaveUnit().toUnitIndex())

[peq]

I don't think your proposed change would make any difference. Instead the UnitIndexer should probably pick all units on the map at the beginning and register them. @Frotty , any comments?

[BlueSaint]

@peq I think I made a pull request successfully now, I make onEnter() proc sooner: https://github.com/peq/wurstStdlib/compare/master...BlueSaint:patch-1

[Frotty]

I don't think your proposed change would make any difference. Instead the UnitIndexer should probably pick all units on the map at the beginning and register them

Which it already does.

@BlueSaint I'm pretty sure the id differs because the onEnter event fires after your function has finished and not after the unit is created. Can't test right now if your fix fixes it.

Also, no. if you would have made a PR it would be under "Pull Requests". Github has a wonderful documentation on PRs.

[BlueSaint]

I had to remove timer execution: https://github.com/peq/wurstStdlib/compare/master...BlueSaint:patch-1 And now my example code works.

[Frotty]

Explain? and https://help.github.com/articles/creating-a-pull-request/

[peq]

Ok, I got the problem now:

I suggest the following changes:

in UnitIndexer: change as you suggested, I was wrong here: onEnter(() -> getEnterLeaveUnit().toUnitIndex()) (use cache instead of assigning a new index)

in OnUnitEnterLeave: Keep a group of registered units. When onEnter is called, execute the callback with all units in that group.

If you have a working solution you can create a pull request. I can also just merge from your clone.

[BlueSaint]

Explain?

Timer execution will not respect init order; it will be after the thread that calls the initializers. I don't know what the purpose of the doAfter initially was, maybe you wanted to avoid hitting OPLIMIT? But it breaks it all and has to go. Also, it causes potential double register when a library that is after OnUnitEnterLeave creates an unit before the timed enumeration has ran.

Filter seems to be considered instantly on unit creation, therefore Unit Indexer works fine and needs no change. Proof:

init
    let r = CreateRegion()
    RegionAddRect(r,bj_mapInitialPlayableArea)
    CreateTrigger()..registerEnterRegion(r,Filter(()->print("filter")))
    CreateUnit(Player(0),'hfoo',0,0,270)
    print("post-creation")

"filter" will be printed before "post-creation"

[Frotty]

Timer execution will not respect init order; it will be after the thread that calls the initializers.

That's the whole point of it.

[peq]

I am not saying that the timer is a good solution. But if you remove the timer, you will miss all units created before.

Wurst runs all inits of imported packages before the importing package. So the init of OnUnitEnterLeave will run before any other package had a chance to register a callback. So there needs to be a mechanism to catch up on all units created before onEnter was called. The nulltimer at least handles this partially.

[BlueSaint]

Inside init of OnUnitEnterLeave, pick all units into a group, and consider them later on inside nullTimer(). The current setup is broken, example:

import OnUnitEnterLeave

init
    onEnter(()->print("enter"))
    CreateUnit(Player(0),'hfoo',0,0,270)

Enter is printed twice (with the current StdLib's OnUnitEnterLeave). So if you have a resource that requires Unit Indexer which, inside its init block, creates n units and then removes then, you have n dead unit indexes.

Edit: I uploaded my new solution, but I think it's on a different branch now: https://github.com/peq/wurstStdlib/compare/master...BlueSaint:master

[peq]

The new solution looks like it should work. Merged it.

(Commits 71e86635fc5d18ed686613b49cfee076e9fc669c and 7ea2806aa56320bf00d6a9fa3615adfca851d66c)

[peq]

By the way: The while-loop can probably be replaced by a for-from loop.

[BlueSaint]

I first had for tempUnit in g, but it seemed to create local copy, so ended up with that version.

[peq]

for u in g creates a copy and leaves g unchanged. for u from g takes the units from g so that g is empty at the end of the loop.

[BlueSaint]

Well you can change it if you want of course. The tempUnit would still create local variable, so you would end up with assignment inside loop. I don't know how good the Wurst optimizer is so I took the safer bet but longer code.