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

Endgame screen with rmlui #4384

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

patroldo
Copy link

@patroldo patroldo commented Feb 23, 2025

Work done

Test steps

  • to check endgamescreen in 1v1(few games which can produce or not produce some awards
  • to check endgamescreen in 2v2(few games which can produce or not produce some awards)
  • to check endgamescreen in 3v3(few games which can produce or not produce some awards)
  • to check endgamescreen in 8v8(few games which can produce or not produce some awards)
  • to check endgamescreen in coop(few games which can produce or not produce some awards)
  • try few other languages

Screenshots:

BEFORE:

2025-02-14T11:47:27,350231422+02:00

AFTER:

2025-02-13T21:56:12,638395611+02:00

2025-02-23T12:14:58,215021198+02:00

NOTE: On Second screen position of buttons is corrected

@WatchTheFort WatchTheFort marked this pull request as draft February 23, 2025 14:49
@patroldo
Copy link
Author

patroldo commented Feb 23, 2025

2025-02-23T17:36:26,424621679+02:00

Example from real game. Unfortunantly translations didn't worked, but this is only due to some "workaround" to hook engine with my LuaUI in order to run replay. In actual game text is looking good

@patroldo
Copy link
Author

Progress + conversation in discord:
https://discord.com/channels/549281623154229250/1333885512364195891

Copy link
Member

Choose a reason for hiding this comment

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

These additional entries shouldn't be needed. We need to be able to use the interpolated string entries, so if those aren't working, we need to fix that.

Copy link
Author

Choose a reason for hiding this comment

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

interpolated you mean those, which already presented. Right?

Copy link
Member

Choose a reason for hiding this comment

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

The ones that contain variables. "Lorum ipsum %{interpolatedVariable}"

Copy link
Author

@patroldo patroldo Feb 23, 2025

Choose a reason for hiding this comment

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

Not sure if rml supports interpolated, at least didn't find it in the doc. There're 2 things which are coming into my mind in order to work with such strings:

  1. Once game ended - dynamically put into RmlUi translations string with interpolated values. I mean already replaced variable with time and only then put it into RmlUi translation table
  2. In order to translate such string, use I18N in lua widget and pass it via documentHandler model. In this case we just avoid using RmlUi translation table

Not sure which approach is better

Comment on lines 73 to 81
RmlUi.AddTranslationString("ui.awards.enemiesDestroyed", Spring.I18N('ui.awards.enemiesDestroyed'))
RmlUi.AddTranslationString("ui.awards.resourcesDestroyed", Spring.I18N('ui.awards.enemiesDestroyed'))
RmlUi.AddTranslationString("ui.awards.resourcesEfficiency", Spring.I18N('ui.awards.resourcesEfficiency'))
RmlUi.AddTranslationString("ui.awards.traitor", Spring.I18N('ui.awards.traitor'))
RmlUi.AddTranslationString("ui.awards.didEverything", Spring.I18N('ui.awards.didEverything'))

RmlUi.AddTranslationString("ui.awards.ecoAward", Spring.I18N('ui.awards.ecoAward'))
RmlUi.AddTranslationString("ui.awards.damageReceivedAward", Spring.I18N('ui.awards.damageReceivedAward'))
RmlUi.AddTranslationString("ui.awards.commanderSleepAward", Spring.I18N('ui.awards.commanderSleepAward'))
Copy link
Member

Choose a reason for hiding this comment

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

These are specific a single widget, they should not be in the general rmlui setup code.

Copy link
Author

Choose a reason for hiding this comment

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

From one side - agree. From another - i would say widget is not the right place to work with translations. I think separate module need to work with translations for RmlUi including handling of changing language

Copy link
Member

Choose a reason for hiding this comment

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

This question is being handled in #4387

offset = offset + offsetAdd
local function calculateAwardScore(award, score)
if award == "commanderSleepAward" then
return math.round(score / 60) .. " minutes"
Copy link
Member

Choose a reason for hiding this comment

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

This needs to support translation

Copy link
Author

Choose a reason for hiding this comment

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

That is a tricky one tbh. Translations can be there using I18N. But let's take few examples of time-translations. English-Ukrainian:
a) 21 minute - "21 хвилина"
b) 22 minutes - "22 хвилини"
c) 23 minutes - "23 хвилини"
d) 25 minutes - "25 хвилин"

The point is that time cannot be translated 1:1 into every language as every language may have unique ending or suffix or similar. Thus separated module needs to handle translations of time. On the other hand - even if time will be slightly incorrect, maybe not a big deal as mainly it should show statistics and doesn't mean translation should be accurate in all and every place. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

The I18N library we use has pluralization support, we just haven't used it yet.
https://github.com/gajop/i18n?tab=readme-ov-file#pluralization

Any string we display on the UI needs to support translation.

@patroldo patroldo force-pushed the endgame-screen-with-rmlui branch from ea55b6a to 0cf44c2 Compare February 24, 2025 00:50
Copy link
Member

@WatchTheFort WatchTheFort left a comment

Choose a reason for hiding this comment

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

The folder structure has now been settled on, please rearrange as follows:

\luaui
  \RmlWidgets
    \gui_awards
      gui_awards.lua
      gui_awards.rcss
      gui_awards.rml

You will have to create the RmlWidgets folder, since this will be the first rml widget added to the repo.

@patroldo
Copy link
Author

The folder structure has now been settled on, please rearrange as follows:

\luaui
  \RmlWidgets
    \gui_awards
      gui_awards.lua
      gui_awards.rcss
      gui_awards.rml

You will have to create the RmlWidgets folder, since this will be the first rml widget added to the repo.

Already done :)

@patroldo patroldo force-pushed the endgame-screen-with-rmlui branch from 648271d to 3543789 Compare March 6, 2025 21:58
@patroldo patroldo requested a review from WatchTheFort March 9, 2025 12:55
@patroldo patroldo force-pushed the endgame-screen-with-rmlui branch from 24b20d7 to e710a8d Compare March 9, 2025 13:42
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.

2 participants