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

Fix Locale-related String conversion issues #4822

Conversation

APickledWalrus
Copy link
Member

Description

In many parts of Skript, when converting to lower/upper case, a Locale is not specified. This means that the default Locale is used (usually dependent on the user's system).

The following, taken from the String#toLowerCase() javadoc, is an example of why this is an issue:

Note: This method is locale sensitive, and may produce unexpected results if used for strings that are intended to be interpreted locale independently. Examples are programming language identifiers, protocol keys, and HTML tags. For instance, "TITLE".toLowerCase() in a Turkish locale returns "t\u0131tle", where '\u0131' is the LATIN SMALL LETTER DOTLESS I character.

The result within Skript is that unexpected characters sometimes pop up during the parsing process for users on different locales, causing parsing to unexpectedly fail without much explanation to the user.

Also see: #4821 (comment)

The fix is to simply specify Locale.ENGLISH for most occurences of String#toLowerCase and String#toUpperCase .


Target Minecraft Versions: N/A
Requirements: N/A
Related Issues:

@APickledWalrus APickledWalrus added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Jun 19, 2022
Copy link
Member

@TPGamesNL TPGamesNL left a comment

Choose a reason for hiding this comment

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

Apart from that import, should be good

@APickledWalrus APickledWalrus merged commit 7ece3f7 into SkriptLang:master Jun 20, 2022
@APickledWalrus APickledWalrus deleted the fix/locale-dependent-conversions branch June 20, 2022 17:01
AyhamAl-Ali pushed a commit to AyhamAl-Ali/Skript that referenced this pull request Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants