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

Parse /pay modifiers more strictly #5638

Merged
merged 17 commits into from
Nov 25, 2024
Merged

Parse /pay modifiers more strictly #5638

merged 17 commits into from
Nov 25, 2024

Conversation

AjMaacc
Copy link
Contributor

@AjMaacc AjMaacc commented Feb 4, 2024

Information

This PR fixes #5495.

Details

Proposed fix:

This PR fixes half of the issue presented.
It is also more simplistic and a much better approach compared to my previous PR

Instead of checking for the last character in the string -> Character.toLowerCase(ogStr.charAt(ogStr.length() - 1)),
check for the exact modified being use via removing the amount from the string -> ogStr.replace(sanitizedString, "")
If the result of ogStr.replace(sanitizedString, "") is not exactly one of the modifiers, throw an exception (unless an empty string is returned due to a modifier not being present).

Environments tested:

OS: Linux 5.15.0-79-generic

Java version: java 19.0.1

  • Most recent Paper version (1.20.4, git-Paper-408)
  • CraftBukkit/Spigot/Paper 1.12.2
  • CraftBukkit 1.8.8

Demonstration:

@AjMaacc AjMaacc closed this Apr 4, 2024
@mbax
Copy link
Member

mbax commented Apr 4, 2024

A two month wait on a large project with busy maintainers? That's nothing to complain about.

@AjMaacc

This comment was marked as abuse.

@pop4959
Copy link
Member

pop4959 commented Apr 5, 2024

Is there any specific reason you decided to close this? It looks like this fixes a valid issue.

@AjMaacc
Copy link
Contributor Author

AjMaacc commented Apr 5, 2024

Is there any specific reason you decided to close this? It looks like this fixes a valid issue.

Well, the longevity of the pr being open. If my pr was still being considered, was overlooked, or any other reason, then I can re-open it.

@AjMaacc AjMaacc reopened this Apr 5, 2024
@pop4959
Copy link
Member

pop4959 commented Apr 5, 2024

Sorry for the delays, most of us have been very busy recently and the bulk of our focus has been on the adventure update (and all of the fallout / bugs following from that, trying to get us into a better place to lean towards a release).

We appreciate your dedication to keeping this up to date, although for the most part unless there are conflicts it should merge cleanly. If there are we'll generally let you know and/or do it ourselves before merging, so please don't worry too much about it especially until we find time to properly review this. Thanks for remaining patient.

@JRoy JRoy added the module: main Issues or PRs for the main Essentials module label Nov 25, 2024
@JRoy JRoy changed the title Check for the specific modifier instead of last char in string Parse /pay modifiers more strictly Nov 25, 2024
@JRoy JRoy enabled auto-merge (squash) November 25, 2024 02:15
@JRoy
Copy link
Member

JRoy commented Nov 25, 2024

Thanks for the contribution :)

@JRoy JRoy merged commit 3203e97 into EssentialsX:2.x Nov 25, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: main Issues or PRs for the main Essentials module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Toggle /pay modifiers in config
4 participants