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

EventValues - add an option to use changers #7512

Open
wants to merge 7 commits into
base: dev/feature
Choose a base branch
from

Conversation

ShaneBeee
Copy link
Contributor

Description

This PR aims to add the ability to have changers for event-values.

NOTES:

  • I extended from converter, EventConverter, this will have the same things a converter has plus the option to set.
  • I only added "set" as I figured other changers (reset, delete, add, remove, etc) would just end up clogging up the EventValues class. If someone needs more changers, that would be a great time to create an expression.
  • The EventValueExpression class scared me... I'm not 100% sure if I did this in the nicest way possible (Feedback would be appreciated here)
  • I added an example for event-item in the consume event. TESTED HEAVILY with both a single value and array. Works nicely.

Target Minecraft Versions: any
Requirements: none
Related Issues: #7485

@ShaneBeee
Copy link
Contributor Author

Showcase:

on consume:
	send "Before: %event-item%"
	set event-item to 1 of stick
	send "After: %event-item%"
Screen.Recording.2025-01-22.at.10.42.50.PM.mov

Copy link
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code itself seems fine. I'm wondering how this might interact with existing event value expressions, such as https://github.com/SkriptLang/Skript/blob/master/src/main/java/ch/njol/skript/expressions/ExprItem.java. It appears there would be a difference with set the item to ... and set the event-item to ...

@ShaneBeee
Copy link
Contributor Author

ShaneBeee commented Jan 23, 2025

The code itself seems fine. I'm wondering how this might interact with existing event value expressions, such as https://github.com/SkriptLang/Skript/blob/master/src/main/java/ch/njol/skript/expressions/ExprItem.java. It appears there would be a difference with set the item to ... and set the event-item to ...

I'll be honest, this didn't cross my mind. Ill test and get back to you.

EDIT:
As per video/code sent in DM on discord, the item expression still works as it should.

Copy link
Contributor

@NotSoDelayed NotSoDelayed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Room for adding JUnit tests? (Expecting to exceed beyond the limit of the sky)

@sovdeeth sovdeeth added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Jan 25, 2025
@ShaneBeee ShaneBeee requested a review from sovdeeth January 25, 2025 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants