New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove compiletime call in OnUnitEnterLeave #20

Merged
merged 5 commits into from Dec 9, 2017

Conversation

Projects
None yet
2 participants
@IgorSamurovic
Copy link
Contributor

IgorSamurovic commented Dec 8, 2017

Apparently, ever since the order of compiletime calls has become deterministic, the compiletime(x) function is just messing things up. I had to remove this in order to get the same order in compiletime/game.

@Frotty

This comment has been minimized.

Copy link
Member

Frotty commented Dec 8, 2017

I don't think so. Compl-expressions are the way to go to have ids only generated at compiletime.
You have to make sure to not use it outside of those.

@IgorSamurovic

This comment has been minimized.

Copy link
Contributor Author

IgorSamurovic commented Dec 8, 2017

Alright, I found the actual issue that was plaguing the map, it works for preplaced units on a brand new map now, too. Previously it wouldn't trigger onEnter on units preplaced by the editor.

@@ -11,18 +11,20 @@ import RegisterEvents
import ObjectIdGenerator
import MapBounds

This comment has been minimized.

@Frotty

Frotty Dec 8, 2017

Member

prune empty line

/* Provides event API for units entering and leaving the map.
The event will also fire for preplaced units on the map. */

constant eventTrigger = CreateTrigger()
unit tempUnit = null
let eventTrigger = CreateTrigger()

This comment has been minimized.

@Frotty

Frotty Dec 8, 2017

Member

why change to let? I prefer constant outside of functions tbh

@IgorSamurovic

This comment has been minimized.

Copy link
Contributor Author

IgorSamurovic commented Dec 9, 2017

Alright, so I did a lot of tinkering with this, and I noticed a problem -

No matter how you use the timer for preplaced units, it's impossible to know whether you'll catch them in the group loop or via an enter event because some scripts will be initialized after OnUnitEnterLeave, some will be initialized before it, which means that, while ensuring that the unit will get processed at all costs (I moved the enum inside the nulltimer), there's a possibility of the unit triggering the enter event.

This can be fixed by moving OnUnitEnterLeave to default imports (meaning all user scripts would be initialized after it, giving us a deterministic order). But I've found a workaround without needing this.

So, in order to remove the possibility of double processing the unit (enum + event), I put everything in the null timer. After the null timer ticks:

  1. Units are first going to be enum'd across the map, and processed.
  2. Unit entering detection is initialized
  3. Unit leaving detection is initialized

Before the nullTimer ticks, all the system does is add adequate onEnter and onLeave functions to the trigger, so this is intended behavior.

This means that no matter where you use onEnter, as long as it is on map init, it will be processed for all existing units after nullTimer ticks.

The only downside of this now is that units won't be processed by onEnter immediately after being created during map init. To make this possible, I'd have to keep the track of processed units which would create some overhead. Should I do this? I'd just have to add a group that contains all "prepared" units and make sure I don't prepare units already in that group.

Also, DamageDetection didn't work for non-preplaced units, because it used the old onEnter semantics which used Filtered unit for some reason. Now it simply adds a working onEnter action, and works for both preplaced and units added after init and after the null timer.

@Frotty Frotty merged commit ccdfe8e into wurstscript:master Dec 9, 2017

@IgorSamurovic IgorSamurovic deleted the IgorSamurovic:compiletimefix branch Dec 9, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment