Skip to content

Add localisation support to various pluralisable strings#38047

Open
diquoks wants to merge 7 commits into
ppy:masterfrom
diquoks:localisation/pluralisable-strings
Open

Add localisation support to various pluralisable strings#38047
diquoks wants to merge 7 commits into
ppy:masterfrom
diquoks:localisation/pluralisable-strings

Conversation

@diquoks

@diquoks diquoks commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

@bdach bdach added the area:localisation Deals with localisation and regional display issues. label Jun 12, 2026
@bdach bdach self-requested a review June 12, 2026 11:34
public DeleteCollectionDialog(Live<BeatmapCollection> collection, Action deleteAction)
{
BodyText = collection.PerformRead(c => $"{c.Name} ({"beatmap".ToQuantity(c.BeatmapMD5Hashes.Count)})");
BodyText = collection.PerformRead(c => DialogStrings.DeleteCollectionBodyText(c.Name, c.BeatmapMD5Hashes.Count));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is much too specific. I would rather have this composed from parts. Something along the lines of:

LocalisableString.Interpolate($@"{c.Name} ({CommonStrings.BeatmapCount(c.BeatmapMD5Hashes.Count)}")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah, i thought about that too, to be honest

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

addressed in 881cad0

Comment thread osu.Game/Localisation/CommonStrings.cs Outdated
/// <summary>
/// "{0:#,0} item|{0:#,0} items"
/// </summary>
public static LocalisableString ItemUnit(int itemsCount) => new PluralisableString(new TranslatableString(getKey(@"item_unit"), @"{0:#,0} item|{0:#,0} items", itemsCount), itemsCount, '|');

@bdach bdach Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would prefer ItemCount or ItemCounter

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

renamed in 881cad0

/// <summary>
/// "{0:#,0} match|{0:#,0} matches"
/// </summary>
public static LocalisableString MatchUnit(int matchesCount) => new PluralisableString(new TranslatableString(getKey(@"match_unit"), @"{0:#,0} match|{0:#,0} matches", matchesCount), matchesCount, '|');

@bdach bdach Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would prefer MatchCount or MatchCounter

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

renamed in 881cad0

- rename some strings
- split `DeleteCollectionBodyText` string
- create separate version for `ItemsCount` in `DrawableCollectionListItem` with special formatting
/// <summary>
/// "{0:#,0} item|{0:#,0} items"
/// </summary>
public static LocalisableString ItemsCount(int quantity) => new PluralisableString(new TranslatableString(getKey(@"items_count"), @"{0:#,0} item|{0:#,0} items", quantity), quantity, '|');

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

idk if you'll like it or not, but i wanted to keep the original formatting in both cases, so i had to duplicate this string for usage in DrawableCollectionListItem

@diquoks diquoks requested a review from bdach June 12, 2026 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:localisation Deals with localisation and regional display issues. size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants