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

Improve registering event values #7269

Merged
merged 12 commits into from
Dec 19, 2024

Conversation

Efnilite
Copy link
Member

@Efnilite Efnilite commented Dec 16, 2024

Description

  • Deprecates Getter
  • Makes registering an event value require a Converter instead of a Getter
  • Adds method for registering an event value without having to specify the time

Example
Before

EventValues.registerEventValue(WorldEvent.class, World.class, new Getter<World, WorldEvent>() {
		@Override
		@Nullable
		public World get(final WorldEvent e) {
			return e.getWorld();
		}
}, EventValues.TIME_NOW);

After

EventValues.registerEventValue(WorldEvent.class, World.class, WorldEvent::getWorld);

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

@Efnilite Efnilite added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Dec 16, 2024
@sovdeeth
Copy link
Member

related #5405

@APickledWalrus
Copy link
Member

I might like to see Getter deprecated, and perhaps you could have it extend Function and just change the existing method signatures

@Efnilite
Copy link
Member Author

I might like to see Getter deprecated, and perhaps you could have it extend Function and just change the existing method signatures

this PR was the beginning of my war against Getter. glad you agree :)

Copy link
Contributor

@Fusezion Fusezion left a comment

Choose a reason for hiding this comment

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

efy, told me to, I have no qualms about this, it's something I've been waiting for, makes event values not a pain

Copy link
Contributor

@Burbulinis Burbulinis left a comment

Choose a reason for hiding this comment

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

love it need it like it very nice!

@Efnilite Efnilite added the 2.10 Targeting a 2.10.X version release label Dec 18, 2024
@Efnilite Efnilite added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Dec 18, 2024
Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

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

Do you intend to finish updating BukkitEventValues?

Copy link
Member

@Moderocky Moderocky left a comment

Choose a reason for hiding this comment

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

Just to make sure this doesn't get merged before Sovde's comment gets a response.

@Efnilite
Copy link
Member Author

Do you intend to finish updating BukkitEventValues?

It was mostly for example purposes. Idm though, should I or is it too prone to merge conflicts?

@Efnilite Efnilite requested a review from Moderocky December 19, 2024 09:52
@Moderocky Moderocky merged commit 537087f into SkriptLang:dev/feature Dec 19, 2024
5 checks passed
@Efnilite Efnilite deleted the event-value-function branch December 22, 2024 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.10 Targeting a 2.10.X version release enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants