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

Add experience point support to ExprTotalExperience #7581

Conversation

erenkarakal
Copy link
Member

Description

This PR adds 'experience point' support to the total experience expression, cleans up the Experience and the syntax class and adds some tests.
I made this PR for the "on level progress change:" event. It did not let you get how much experience was changed as a number.

on level progress change:
  set {_xp} to event-experience     <--- what can you even do with this besides giving it to a player

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

@erenkarakal erenkarakal requested review from Efnilite and sovdeeth and removed request for Efnilite February 5, 2025 16:37
assert experience of {_xp} is 0 with "clearing xp of Experience doesn't work"
set {_xp} to 5 xp
reset experience of {_xp}
assert experience of {_xp} is 0 with "resetting xp of Experience doesn't work"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
assert experience of {_xp} is 0 with "resetting xp of Experience doesn't work"
assert experience of {_xp} is 0 with "resetting xp of Experience doesn't work"

@Efnilite Efnilite added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Feb 5, 2025
Copy link
Contributor

@TheAbsolutionism TheAbsolutionism left a comment

Choose a reason for hiding this comment

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

Won't let me highlight these, but
public void change(Event event, Object @Nullable [] delta, ChangeMode mode) {
and add an empty line after the #getPropertyName

@@ -25,44 +25,43 @@
"",
"if player's total experience is greater than 100:",
"\tset player's total experience to 0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"\tset player's total experience to 0",
"\tset player's total experience to 0",

@@ -25,44 +25,43 @@
"",
"if player's total experience is greater than 100:",
"\tset player's total experience to 0",
"\tgive player 1 diamond"
"\tgive player 1 diamond",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"\tgive player 1 diamond",
"\tgive player 1 diamond",

Comment on lines +31 to +32
"\tset {_xp} to event-experience",
"\tbroadcast experience of {_xp}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"\tset {_xp} to event-experience",
"\tbroadcast experience of {_xp}"
"\tset {_xp} to event-experience",
"\tbroadcast experience of {_xp}"

default:
return null;
}
public @Nullable Class<?>[] acceptChange(ChangeMode mode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public @Nullable Class<?>[] acceptChange(ChangeMode mode) {
public Class<?> @Nullable [] acceptChange(ChangeMode mode) {

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.

Does an experience->number converter cause any issues? Seems like it'd be a nicer solution.

@erenkarakal
Copy link
Member Author

closing in favor of #7602

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.

4 participants