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 untranslatable string in advancefrom deprecated message #5922

Closed
wants to merge 2 commits into from

Conversation

fujimo-t
Copy link
Contributor

@fujimo-t fujimo-t commented Jul 4, 2021

I noticed that second chunk is not marked to translatable while working 1.16 translation.

The target branch is set to master because I heard 1.16 is string freezed.
Is it possible to merge this also 1.16 and re-run pot update?

@stevecotton
Copy link
Contributor

I think this one shouldn't be translatable at all, as it's an error that will only make sense to WML authors. @ivanovic?

@CelticMinstrel
Copy link
Member

CelticMinstrel commented Jul 4, 2021

Our generic "X has been deprecated" message is translatable, so I think it can make sense to make "use Y instead" translatable too… but probably it should be a single string, like "use $elem instead" and substitute in the replacement?

That doesn't quite work here, mind you, but other deprecation messages have an untranslatable portion with additional details, so perhaps the same could be done here.

interpolate_into_string(
  _("Use $elem instead."),
  {"elem", "[modify_unit_type]\n" + modify_unit_type.debug() + "\n [/modify_unit_type]"
) + "This goes in the [campaign] or [multiplayer] tag rather than in the [unit_type]."

However, in that case it's probably something that has to wait for 1.16.1?

@fujimo-t
Copy link
Contributor Author

fujimo-t commented Jul 5, 2021

Indeed, using substitution looks to better and this untranslatable thing is not important for most players.

@Wedge009 Wedge009 added the Translations Issues with translations, translation tooling, the translations engine, or code that uses it. label Sep 9, 2021
@Wedge009
Copy link
Member

Wedge009 commented Sep 9, 2021

This seems to be hanging around unnecessarily, given it's a small change. If waiting for 1.16.1, it can just be added to the back-port list, no?

Does fujimo need help adapting celmin's suggestion?

@fujimo-t
Copy link
Contributor Author

I'm sorry for late reply.
I had left GitHub for a while.

I will try to fix by string interpolation.

@stevecotton
Copy link
Contributor

This message never triggers, the code to trigger it is broken and looks for [units][advancefrom] instead of [unit_type][advancefrom]. This is issue #6204.

If we're going to make it a translatable player-readable string, I suggest phrasing it as Error: [advancefrom] does not work, "Cuttle Fish" units will not be able to advance to "Kraken". Even if the unit names aren't translated, that's probably the info the player needs to decide whether to wait for the campaign author to fix it.

@stevecotton
Copy link
Contributor

Many thanks to you @fujimo-t, but as I've already written other C++ changes from investigating #6204 it felt logical to open a new PR that supersedes this one.

@stevecotton stevecotton closed this Jan 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Translations Issues with translations, translation tooling, the translations engine, or code that uses it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants