-
-
Notifications
You must be signed in to change notification settings - Fork 414
Event Value Registry #8326
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
Open
UnderscoreTud
wants to merge
26
commits into
dev/feature
Choose a base branch
from
feature/event-value-registry
base: dev/feature
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Event Value Registry #8326
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…event value is registered and an event-projectile is requested) Replace EventValue#forEventClass and EventValue#convertTo methods with a unified EventValue#getConverted method
…lve event values at runtime
…thout calling init()
# Conflicts: # src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
sovdeeth
requested changes
Dec 24, 2025
Member
sovdeeth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking great
wonderful work tud, especially for your 2nd ever contribution
src/main/java/org/skriptlang/skript/bukkit/lang/eventvalue/EventValueRegistry.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/lang/eventvalue/EventValueRegistryImpl.java
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/lang/eventvalue/EventValueRegistryImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/lang/eventvalue/EventValueRegistryImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/lang/eventvalue/EventValueRegistryImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/lang/eventvalue/EventValue.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/lang/eventvalue/EventValue.java
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/lang/eventvalue/EventValue.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/lang/eventvalue/ConvertedEventValue.java
Show resolved
Hide resolved
src/main/java/ch/njol/skript/expressions/base/EventValueExpression.java
Outdated
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
breaking changes
Pull or feature requests that contain breaking changes (API, syntax, etc.)
enhancement
Feature request, an issue about something that could be improved, or a PR improving something.
needs reviews
A PR that needs additional reviews
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Event values are a mess. The main method to get an event value using the
EventValuesclass is almost 100 lines long and super hard to follow along.Event values don't support any changers apart from
SET, which is fine most of the time, but if you want your event value to support other changers you'd have to register an entire expression for it.Additionally, there is no way to register an event value with a custom identifier (i.e.
event-<identifier>) without making it its own expression.Solution
EventValueRegistryserves as a full replacement to theEventValuesclass with a more modern API similar to Skript's new registration API with the ability to register and unregister event values at runtime as well as cache resolved event values to avoid computing it again in the future.A new
EventValueclass that represents an event value, which now supports custom identifiers, event validation and any combination of changers. AnEventValueobject can be constructed throughEventValue.builder.Testing Completed
A lot of the present tests utilize event-values in some way or another, and the
EventValues.sktest covers a lot of cases.Supporting Information
I tried my best to keep parity between the old
EventValuesand the newEventValueRegistryclasses, so there shouldn't be any breaking changes, unless I missed some obscure cases.Breaking Changes
This PR removes two
registermethods inEventValuesthat use theGetterclass as a parameter, which have been deprecated and marked for removal since 2.10.A check has been added to ensure no duplicate event values are registered.
Completes: none
Related: none
AI assistance: Junie was used to generate most of the javadocs for
EventValueandEventValueRegistrywith some minor tweaks