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 plural identification for class infos in patterns. #7304

Merged
merged 4 commits into from
Dec 26, 2024

Conversation

Moderocky
Copy link
Member

Description

A follow-up to this fix.
This makes plural detection first check if the word could be singular.

Words like gui already had correct plural forms (-ui -> -uis), but when plurality was checked, the singular was identified as a plural because it matched the -i plural ending.

Now, it will check if it matches a singular pattern before checking if it matches a plural pattern.
Note: it is possible this may break some obscure word (that looks like the singular form of another word) but none of the existing class infos break, so I'm assuming it's probably fine.

This also adds a special method for addons to register specific word overrides, so if there are any future problems, they can be fixed specifically with Utils.addPluralOverride("foot", "feet") (for example).
This uses a special 'complete word' mode to make sure that specific overrides won't interfere with any existing plural endings.

I also made some corrections to annoying -ves endings (wolves, shelves, calves etc.).


Target Minecraft Versions: any
Requirements: none
Related Issues: #6982, fixes #7253

@Moderocky Moderocky added bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. 2.10 Targeting a 2.10.X version release labels Dec 23, 2024
Comment on lines 295 to 330
if (word.isEmpty())
return new NonNullPair<>("", Boolean.FALSE);
return new NonNullPair<>("", false);
if (!couldBeSingular(word)) {
for (final WordEnding ending : plurals) {
if (ending.isCompleteWord()) {
// Complete words shouldn't be used as partial pieces
if (word.length() != ending.plural().length())
continue;
}
if (word.endsWith(ending.plural()))
return new NonNullPair<>(
word.substring(0, word.length() - ending.plural().length()) + ending.singular(),
true
);
if (word.endsWith(ending.plural().toUpperCase(Locale.ENGLISH)))
return new NonNullPair<>(
word.substring(0, word.length() - ending.plural().length())
+ ending.singular().toUpperCase(Locale.ENGLISH),
true
);
}
}
return new NonNullPair<>(word, false);
}

private static boolean couldBeSingular(String word) {//todo
for (final WordEnding ending : plurals) {
if (word.endsWith(ending.plural()))
return new NonNullPair<>(word.substring(0, word.length() - ending.plural().length()) + ending.singular(), Boolean.TRUE);
if (word.endsWith(ending.plural().toUpperCase(Locale.ENGLISH)))
return new NonNullPair<>(word.substring(0, word.length() - ending.plural().length()) + ending.singular().toUpperCase(Locale.ENGLISH), Boolean.TRUE);
if (ending.singular().isBlank())
continue;
if (ending.isCompleteWord() && ending.singular().length() != word.length())
continue; // Skip complete words

if (word.endsWith(ending.singular()) || word.toLowerCase().endsWith(ending.singular())) {
return true;
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Git chose an insane way to diff this. It's two separate methods, in case that isn't clear.

@TheAbsolutionism
Copy link
Contributor

TheAbsolutionism commented Dec 23, 2024

Though it looks good, iirc when I did my testing, it only actually threw the exception/error when calling the element and not just by registering it.
If you wanted to add proper testing, optional

Edit: nvm lol

@Moderocky Moderocky merged commit 0b6cfc0 into SkriptLang:dev/feature Dec 26, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.10 Targeting a 2.10.X version release 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.

4 participants