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

Check for the specific modifier instead of last char in string #5638

Open
wants to merge 16 commits into
base: 2.x
Choose a base branch
from

Conversation

AjMaacc
Copy link

@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
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Toggle /pay modifiers in config
3 participants