Essentials icon indicating copy to clipboard operation
Essentials copied to clipboard

Check for the specific modifier instead of last char in string

Open AjMaacc opened this issue 1 year ago • 5 comments

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

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

Demonstration:

AjMaacc avatar Feb 04 '24 05:02 AjMaacc

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

mbax avatar Apr 04 '24 18:04 mbax

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

🤷‍♂️

AjMaacc avatar Apr 04 '24 19:04 AjMaacc

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

pop4959 avatar Apr 05 '24 06:04 pop4959

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 avatar Apr 05 '24 06:04 AjMaacc

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.

pop4959 avatar Apr 05 '24 06:04 pop4959