Past and future event-values

[TheLimeGlass]

Ok so in Skript you can define EventValues time state right. https://github.com/bensku/Skript/blob/master/src/main/java/ch/njol/skript/registrations/EventValues.java#L82-L85

You can have event values like

future event-location
past event-location

So I tired to make past and future state locations a long time ago and i'm coming back to the same question now.

EventValues.registerEventValue(PlayerMoveEvent.class, Location.class, new Getter<Location, PlayerMoveEvent>() {
    @Override
    public Location get(PlayerMoveEvent playerMoveEvent) {
        return playerMoveEvent.getFrom();
    }
}, -1);
EventValues.registerEventValue(PlayerMoveEvent.class, Location.class, new Getter<Location, PlayerMoveEvent>() {
    @Override
    public Location get(PlayerMoveEvent playerMoveEvent) {
        return playerMoveEvent.getTo();
    }
}, 1);

But both event-values return the same value

on any movement:
	"%player%" is "LimeGlass"
	message "%future event-location% & %past event-location%"

I asked this a long time ago how to do stuff like this without making an expression, and no one knew anything about it. I looked through source code of other addons and no one even uses the event-value time option. The only place they're used is in Skript itself from Njol of course.

I was just wondering how this can work properly?

I have a feeling that if I wait a tick the event-location will change to getTo() (The future event-location) just like some of Skript's events and SkQuery's on block land (Get block before it lands and the aftermath block) but I don't want to wait a tick for an event-value I need before, so I was wondering if I could get some info on this? Thanks.

Also this has a spelling mistake when someone is here https://github.com/SkriptLang/Skript/blob/master/src/main/java/ch/njol/skript/registrations/EventValues.java#L46

Referencing #1182

[28andrew]

Use your IDE's debugger to see what return playerMoveEvent.getFrom(); and return playerMoveEvent.getTo(); return.

[TheLimeGlass]

They return two different values. It's just Skript that is using the getFrom() for both future and past

[bensku]

I'm not sure how this is supposed to work or if it works the way it is supposed to. Will investigate.

[28andrew]

Can reproduce still.

[Nicofisi]

This issue is quite complicated but I'll try to explain what I managed to discover so far.

There are three states - past, present (default), and future.

ExprTimeState, the expression which has the syntaxes like "past %~object%" and "future %~object%", is a WrapperExpression. ExprTimeState, in its init method, calls the setTime method of the expression being wrapped.

The init method of the wrapped expression is called before the init method of the ExprTimeState.

EventValueExpression is the class that's used by all event-something expressions. It seems to sort of cache the possible getters in its init method. The getters for each of the states are completely different (link 1, link 2, link 3). And look here:

Skript/src/main/java/ch/njol/skript/expressions/base/EventValueExpression.java

Line 142 in e521140

final Getter<? extends T, ?> getter = EventValues.getEventValueGetter(e, c, getTime());

To get the correct getter, the getTime() method is used. And like I've wrote before, the time is actually set after the init method of the wrapped expression finishes executing, what in practice means that in the piece of code shown above, getTime() always returns 0 (= present, the default)

Perhaps the states are broken somehow else too, but this thing above is one of the issues for sure.

---

Some rather irrelevant stuff below

The code that I used to test it:

Index: src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
===================================================================
--- src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java	(revision b10cbd6b250483af1636d211df1b386a7f407509)
+++ src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java	(date 1531332699823)
@@ -647,12 +647,26 @@
 		}, 0);
 		// PlayerMoveEvent
 		EventValues.registerEventValue(PlayerMoveEvent.class, Block.class, new Getter<Block, PlayerMoveEvent>() {
+			@Nullable
 			@Override
+			public Block get(final PlayerMoveEvent e) {
+				return e.getFrom().getWorld().getBlockAt(-10, 0, -10);
+			}
+		}, -1);
+		EventValues.registerEventValue(PlayerMoveEvent.class, Block.class, new Getter<Block, PlayerMoveEvent>() {
 			@Nullable
+			@Override
 			public Block get(final PlayerMoveEvent e) {
-				return EvtMoveOn.getBlock(e);
+				return e.getFrom().getWorld().getBlockAt(0, 0, 0);
 			}
 		}, 0);
+		EventValues.registerEventValue(PlayerMoveEvent.class, Block.class, new Getter<Block, PlayerMoveEvent>() {
+			@Nullable
+			@Override
+			public Block get(final PlayerMoveEvent e) {
+				return e.getFrom().getWorld().getBlockAt(10, 0, 10);
+			}
+		}, 1);
 		
 		// --- HangingEvents ---
 		// 1.4.3

and a debugger

the script:

on step on sand:
    message "%past event-block% %event-block% %future event-block%"

The block at -10, 0, -10 was set to dirt, at 0, 0, 0 to stone, at 10, 0, 10 to leaves. Yet it always displayed stone stone stone

[bensku]

Changing setTime to be called before initialization might require parser changes. That will be "fun", certainly. Also, it would be a potentially breaking change.

Still probably worth it. Figuring out what was wrong was likely the most time-consuming part anyway.

[Nicofisi]

A workaround would be to call the init method of the wrapped expression the second time, in ExprTimeState... but it wouldn't work with expressions that have only a future or past state, since they wouldn't load for the first time at all (where the time is 'present'). So I'm not sure whether it's worth it @bensku

[TheLimeGlass]

Found this again, this is literally the issue
https://github.com/SkriptLang/Skript/blob/master/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java#L285-L286

Somewhere down the line the isDefault got changed or tampered with for expressions, so now it's always returning false, thus cancelling most of the past and future states from happening here https://github.com/SkriptLang/Skript/blob/master/src/main/java/ch/njol/skript/expressions/ExprTimeState.java#L68

It may also have to do with it being called after init

I think a second issue is addons not being able to register event values properly. I hate this bug so much.

I removed the isDefault method test highlighted above and this worked flawless

on vehicle exit:
	kill the former (player's vehicle)

If the isDefault is still there it will always error.

Also another issue, if you don't place the object in parse marks (player's vehicle) The expression thinks it's asking for the past state of the player, rather than the vehicle. Weird.

I also have no clue what the isDefault is, I know it's designed for variables and I read Njol's post, but I still don't understand why, and why this has to be default. It's basically like checking if the expression has changed since the start of the event right?

[Snow-Pyon]

Default expressions are literally that, default expressions, if you don't input a value that isn't nullable of a type that has a default expression, Skript will use that expression. The isDefault method is to determine whether an expression is default or not.

As for why it has to be a default expression, it is more like, it has to be an event expression, it can't have a past and future state otherwise (unless you override setTime and explicitly implement it, but this is pretty much never the case), your example should work just fine because it is setting the time of either ExprEntity, which I've implemented isDefault to fix this at some point, or ExprEventExpression, that is also a default expression.

I think I've mentioned it before (I don't remember if it was here or in the Discord, but anyways), exprs that have the logic for getting time implemented in their get method, do work with time states (your example is one). The ones that don't work are event values' time states (past event-location for example)

[TheLimeGlass]

My example doesn't work though... and vehicle has a time implementation. Removing isDefault fixes it for me.

[Snow-Pyon]

Then, something else might be being fired, that isn't the ExprEntity and ExprEventExpression which isn't a default value. Removing the check isn't exactly the fix to what this issue is anyways.

[Nicofisi]

@TheLimeGlass did you read my post? Sure, there might be more issues, but what I described is one of them