Skip to content
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

Expressions that aren't EventValues can't be used in "TimeStates" #3150

Open
TheLimeGlass opened this issue Jul 15, 2020 · 2 comments
Open
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. priority: low Issues that are not harmful to the experience but are related to useful changes or additions. up for debate When the decision is yet to be debated on the issue in question

Comments

@TheLimeGlass
Copy link
Collaborator

TheLimeGlass commented Jul 15, 2020

Basically every expression is suppose to convert the type of an expression and check with the event-values of that type from the event. Njol put a comment in the Expression class to automatically do it, which I think I will implement.

Example:

on block place:
        # This below is suppose to grab any default expressions of the type the expression is.
	# But... it doesn't.
	if past block at event-location was short grass:
		broadcast "&6Test 2. placed on short grass"

This won't work, but if you use event-block it will with my pull #3149

If you want to read about time states and my pull to fix them checkout #3149

This below is what I found when debugging #671 and is a different issue from that.

The expression type in patterns exists %location@-1% to get past location value of an expression, but this ultimately doesn't work on most expressions, it only works for getting the default expression which is an event-values

@TheLimeGlass
Copy link
Collaborator Author

TheLimeGlass commented Jul 16, 2020

Tested example:

I register a GameMode event value for PlayerGameModeChangeEvent for before and future, past defaults to present

// PlayerGameModeChangeEvent
EventValues.registerEventValue(PlayerGameModeChangeEvent.class, GameMode.class, new Getter<GameMode, PlayerGameModeChangeEvent>() {
	@Override
	@Nullable
	public GameMode get(PlayerGameModeChangeEvent e) {
		return e.getPlayer().getGameMode();
	}
}, 0);
EventValues.registerEventValue(PlayerGameModeChangeEvent.class, GameMode.class, new Getter<GameMode, PlayerGameModeChangeEvent>() {
	@Override
	@Nullable
	public GameMode get(PlayerGameModeChangeEvent e) {
		return e.getNewGameMode();
	}
}, 1);

and make the ExprGameMode player expression optional. This below will work.

on gamemode change:
	if gamemode was creative:
		message "was creative"
	if gamemode will be survival:
		message "will be survival"

because it's using the default expression of the player and the setTime of the ExprGameMode to do this.

But the second you define gamemode of player it errors because Skript uses a different init method from a different expression rather than using the EventValueExpression of the Player Classinfo which sets the time correctly in my pull #3149

@TheLimeGlass TheLimeGlass added bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. priority: low Issues that are not harmful to the experience but are related to useful changes or additions. priority: lowest "Nice to have" updates that are not required (tiny low impact bug fixes or QoL enhancements). and removed priority: lowest "Nice to have" updates that are not required (tiny low impact bug fixes or QoL enhancements). labels Jan 14, 2022
@TheLimeGlass TheLimeGlass added the up for debate When the decision is yet to be debated on the issue in question label Jul 17, 2022
@TheLimeGlass
Copy link
Collaborator Author

TheLimeGlass commented Jul 17, 2022

To simplify or ignore all the details and debugging I said above.

Basically an expression has a setTime which allows it to define the time state of itself.

Well if it doesn't define it, time states won't work on it, but lets say an EventValue has the time state of a block (The expression also returns a block) should it check if the block equals the event value and then grant the time to be allowed (Which Njol commented in the Expression class that he was going to do this way or consider it).

Or should we do the painful process of having to add all setTime methods to expressions that should support time.

This issue mainly targets expressions that default themselfs like gamemode [of %players%] like my example, because you're turning a property expression into an aliases event value, but the time state doesn't transfer over unless the expression defines it's setTime

Note: I lost where the comment was because it wasn't a perm link I posted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. priority: low Issues that are not harmful to the experience but are related to useful changes or additions. up for debate When the decision is yet to be debated on the issue in question
Projects
None yet
Development

No branches or pull requests

1 participant