Attempt to fix all unlocalized lines (part 2: cmd) #40752

Pok27 · 2025-10-07T13:58:57Z

IConsoleCommand dead, now only LocalizedEntityCommands/LocalizedCommands
I looked through all command files and their localization keys and fixed/added the missing ones

Required space-wizards/RobustToolbox#6248

github-actions · 2025-10-07T13:59:18Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Siginanto · 2025-10-08T03:21:55Z

Holy moth

Princess-Cheeseballs · 2025-10-08T05:52:26Z

Is it possible to split this into multiple PRs? Or is the large diff unavoidable?

VerinSenpai

I be doing a naughty naughty and looking at this while Im at work so this is all Ive got to so far

VerinSenpai · 2025-10-08T07:18:43Z

Content.Client/Commands/DebugCommands.cs


    public override string Command => "showsubfloor";

+    public override string Help => Loc.GetString($"cmd-{Command}-help", ("command", Command));


This line and description aren't necessary in localized commands as they're fetched automatically by the parent class.

It does not contain a parameter with the command passed to the key.

LocalizedCommand which is inherited by LocalizedEntityCommand does have it.

#37991

See the linked PR and the associated commands

There is no need to pass the command name in {$command} (although this should be corrected), but here and in other commands it is necessary.

example:

I like it better this way. It could be useful for renaming the command. We need a third person to decide. D:

I have had a moment of enlightenment that has foretold your reasoning behind this and I've come to agree.

I then bring 2 questions:

What does Loc.GetString do if a format value is passed but the string its formatting doesn't contain the formatee.

And if it silently ignores it, can we just update the help line in the parent class to get the name, as you've suggested above. This would let us keep these help overrides cleaned up.

Was also thinking that usage string with the command name could have its own locale string that could be used by this. Maybe I'm being too ambitious. I will dig deeper this weekend.

I think everything will be fine. I'll do the PR in RobustToolbox a little later. I don't know about localizing the commands, but I think can do it in another PR if necessary.

From what i can see in fluent source code it looks pretty close to the fact that it should ignore arguments passed if they are not used. It cretes Scope from arguments and then asks to resolve arguments as it sees them in string to be formatted. So it would probably be fine to put additional arguments all the time? At the same time, when you put no arguments, it does not create bunch of objects, so it gives less memory workload. Meh. Its not like command helper props are on hot path :D

That works out great then. Shouldn't be an issue regardless as this updates most if not all help strings to take the command name as an argument and any future command additions should follow suit.

VerinSenpai · 2025-10-08T07:20:46Z

Content.Client/Access/Commands/ShowAccessReadersCommand.cs

            _overlay.AddOverlay(new AccessOverlay(EntityManager, _cache, _xform));

-        shell.WriteLine(Loc.GetString($"cmd-showaccessreaders-status", ("status", !existing)));
+        shell.WriteLine(Loc.GetString($"cmd-{Command}-status", ("status", !existing)));


Not a fan of doing this as I believe it hurts readability in the commands and I've been removing this in my own localization prs. Pinging @slarticodefast since he's reviewed most of mine to see if he has an opinion on it.

I took the style from this PR #22897.

I would avoid doing this, in case we rename the command in the future it would break all ftl string names and we would have to rename all of them.

Pok27 · 2025-10-08T07:23:08Z

Is it possible to split this into multiple PRs? Or is the large diff unavoidable?

It's not impossible, but it will be difficult. It was easier for me to fix all .flt files to one format, after all the commands, without thinking about which commands with .flt will be in this PR.

github-actions · 2025-10-08T14:22:28Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

slarticodefast · 2025-10-09T12:23:40Z

Content.Client/Access/Commands/ShowAccessReadersCommand.cs


    public override string Command => "showaccessreaders";

+    public override string Help => Loc.GetString($"cmd-{Command}-help", ("command", Command));


Suggested change

public override string Help => Loc.GetString($"cmd-{Command}-help", ("command", Command));

no need to override this, this is already inherited from LocalizedEntityCommands

Same for all the other cases.

This was being done to dynamically pass the command name as an argument to the usage. I was a bit off put by it but maybe its worth considering? Would help for example if the command name is changed to something russian in a fork.

Then do that in the parent instead of overriding it manually in every single command.

slarticodefast · 2025-10-09T12:24:32Z

Content.Client/Access/Commands/ShowAccessReadersCommand.cs

            _overlay.AddOverlay(new AccessOverlay(EntityManager, _cache, _xform));

-        shell.WriteLine(Loc.GetString($"cmd-showaccessreaders-status", ("status", !existing)));
+        shell.WriteLine(Loc.GetString($"cmd-{Command}-status", ("status", !existing)));


I would avoid doing this, in case we rename the command in the future it would break all ftl string names and we would have to rename all of them.

github-actions · 2025-10-12T21:56:06Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

VerinSenpai · 2025-10-13T16:04:57Z

@slarticodefast @Pok27 Engine change was merged. I assume there needs to be an engine update for this now yes?

VerinSenpai · 2025-10-13T16:14:34Z

Content.Server/Actions/Commands/AddActionCommand.cs

    {
        if (args.Length != 2)
        {
-            shell.WriteError(Loc.GetString(Loc.GetString("cmd-addaction-invalid-args")));


Wow, this happened twice. Not a lot, just weird its happened twice.

Pok27 · 2025-10-13T16:43:37Z

@slarticodefast @Pok27 Engine change was merged. I assume there needs to be an engine update for this now yes?

Yeah

VerinSenpai

This nearly killed me. I'm still dizzy.

Overall I think its pretty good.
I've suggested some cleanup that should be done as part of converting comands to LocalizedEntityCommands. Namely removing the IEntityManager dependency and replace any usages of it with EntityManager (as its inherited from the parent class).

I've also gone ahead and suggested that you use generics from shell.ftl where possible. I intentionally didn't make this suggestion if the command was already using its own localized variant.

I believe I also made a few comments drawing attention to some strings that can have generics made for them as they're used a few times throughout, but that can be saved for a later date.

Cheers to command localizing 🥂

VerinSenpai · 2025-10-13T16:18:43Z

Content.Server/Administration/Commands/AddReagent.cs

            if (args.Length < 4)
            {
-                shell.WriteLine($"Not enough arguments.\n{Help}");
+                shell.WriteLine(Loc.GetString("cmd-addreagent-not-enough-args"));


I'd prefer the generic shell loc string be used here, but given that this PR is already big enough, that can probably be saved for a future cleanup.

Additionally some of these can be converted to LEC but that can also be saved.

VerinSenpai · 2025-10-13T16:21:34Z

Content.Server/Administration/Commands/AdminLogBulk.cs

        if (shell.Player?.AttachedEntity is not { } entity)
        {
-            shell.WriteError("This command can only be ran by a player with an attached entity.");
+            shell.WriteError(Loc.GetString("cmd-adminlogbulk-player-only"));


Change this one to shell-must-be-attached-to-entity

VerinSenpai · 2025-10-13T16:27:19Z

Content.Server/Administration/Commands/OSay.cs

        if (args.Length < 3)
        {
-            shell.WriteLine(Loc.GetString("osay-command-error-args"));
+            shell.WriteLine(Loc.GetString("cmd-osay-error-args"));


This can use the generic shell-wrong-arguments-number-needs-specific or shell-wrong-arguments-number

VerinSenpai · 2025-10-13T16:28:42Z

Content.Server/Administration/Commands/PanicBunkerCommand.cs

        shell.WriteLine(Loc.GetString(toggle.Value
-            ? "panicbunker-command-disable-with-admins-enabled"
-            : "panicbunker-command-disable-with-admins-disabled"
+            ? "cmd-panicbunker_disable_with_admins-enabled"


Was there a reason to change these line titles like this?

Command key renamed

oh right right, but why the underscore? convention would use hyphen.

cmd-panicbunker-disable-with-admins-enabled

The structure cmd-(commandName)-(text) seems to be more important

VerinSenpai · 2025-10-13T16:37:43Z

Content.Server/Administration/Commands/RoleBanListCommand.cs

        if (args.Length != 1 && args.Length != 2)
        {
-            shell.WriteLine($"Invalid amount of args. {Help}");
+            shell.WriteLine(Loc.GetString("cmd-rolebanlist-invalid-args", ("help", Help)));


This can be converted to a generic too, but that can be saved for later.

VerinSenpai · 2025-10-13T17:56:11Z

Content.Server/UserInterface/StatValuesCommand.cs

+public sealed class StatValuesCommand : LocalizedEntityCommands
 {
    [Dependency] private readonly EuiManager _eui = default!;
    [Dependency] private readonly IEntityManager _entManager = default!;


Chuu Chuu

Same here. Make sure to replace _entManager with EntityManager below too.

VerinSenpai · 2025-10-13T18:13:48Z

Content.Server/Verbs/Commands/InvokeVerbCommand.cs

-    public sealed class InvokeVerbCommand : IConsoleCommand
+    public sealed class InvokeVerbCommand : LocalizedEntityCommands
    {
        [Dependency] private readonly IEntityManager _entManager = default!;


Remove this dependency. Replace _entManager with EntityManager below.

VerinSenpai · 2025-10-13T18:14:53Z

Content.Server/Verbs/Commands/ListVerbsCommand.cs

-    public sealed class ListVerbsCommand : IConsoleCommand
+    public sealed class ListVerbsCommand : LocalizedEntityCommands
    {
        [Dependency] private readonly IEntityManager _entManager = default!;


Remove this dependency. Replace _entManager with EntityManager below.

VerinSenpai · 2025-10-13T18:14:55Z

Content.Server/Verbs/Commands/ListVerbsCommand.cs

-    public sealed class ListVerbsCommand : IConsoleCommand
+    public sealed class ListVerbsCommand : LocalizedEntityCommands
    {
        [Dependency] private readonly IEntityManager _entManager = default!;


Remove this dependency. Replace _entManager with EntityManager below.

VerinSenpai · 2025-10-13T18:16:28Z

Content.Server/Whitelist/WhitelistCommands.cs


            if (!await _dbManager.GetWhitelistStatusAsync(session.UserId))
-                _netManager.DisconnectChannel(session.Channel, Loc.GetString("whitelist-not-whitelisted"));
+                _netManager.DisconnectChannel(session.Channel, Loc.GetString("whitelist-not-whitelisted")); // TODO: No localization key


I'd suggest going ahead and resolving this as appose to adding a comment.

I need to figure out what this is, where to create the key, and what to write there.

Pok27 · 2025-10-13T18:43:31Z

Pok27 · 2025-10-13T18:43:43Z

Thanks for the review!

VerinSenpai

You're bringing tears of joy to my eyes. This looks great. I forgot to clarify that entity systems can just be brought in as dependencies for LEC commands but other than that the code looks good.

Also really appreciate the cleanup, especially to the dozens of $ strings I dotted around because I didn't realize those were just C# format strings 😅

A maintainer still needs to give it the ole slice and dice but I reckon you're pretty much there.

VerinSenpai · 2025-10-14T11:41:38Z

Content.Server/Administration/Commands/VariantizeCommand.cs

-        var mapsSystem = _entManager.System<SharedMapSystem>();
-        var tileSystem = _entManager.System<TileSystem>();
-        var turfSystem = _entManager.System<TurfSystem>();
+        var mapsSystem = EntityManager.System<SharedMapSystem>();


:despair:

I should've clarified in my initial review, I'm sorry ;(

These can be made dependencies in LEC commands.

VerinSenpai · 2025-10-14T11:42:36Z

Content.Server/Atmos/Commands/SetMapAtmosCommand.cs

        }

-        var atmos = _entities.EntitySysManager.GetEntitySystem<AtmosphereSystem>();
+        var atmos = EntityManager.EntitySysManager.GetEntitySystem<AtmosphereSystem>();


Same here. This is just an even more convoluted way to get the AtmosphereSystem dependency.

VerinSenpai · 2025-10-14T11:46:16Z

Content.Server/Maps/PlanetCommand.cs

        }

-        var biomeSystem = _entManager.System<BiomeSystem>();
+        var biomeSystem = EntityManager.System<BiomeSystem>();


The dependency goblin strikes again 😈

missing-localization-cmd

7b2f0d0

Pok27 requested review from DrSmugleaf, Princess-Cheeseballs, TheShuEd, crazybrain23 and metalgearsloth as code owners October 7, 2025 13:58

github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Oct 7, 2025

Merge branch 'master' into missing-localization-cmd

e7130b6

github-actions bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Oct 7, 2025

Pok27 added 3 commits October 7, 2025 17:31

fix merge

88c5a01

fix merge 2

bd2d130

fix test

74df9b2

VerinSenpai suggested changes Oct 8, 2025

View reviewed changes

github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Oct 8, 2025

Merge branch 'master' into missing-localization-cmd

bf05f51

github-actions bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Oct 8, 2025

VerinSenpai added the A: Localization Area: Support for localizing the game's content for other languages. label Oct 9, 2025

slarticodefast requested changes Oct 9, 2025

View reviewed changes

PJBot added S: Awaiting Changes Status: Changes are required before another review can happen and removed S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. labels Oct 9, 2025

Pok27 mentioned this pull request Oct 9, 2025

Fix LocalizedCommands for ftl space-wizards/RobustToolbox#6248

Merged

VerinSenpai added the S: Needs Engine PR Merged Status: Requires an existing Robust Toolbox PR to be merged first. label Oct 9, 2025

fix for review

f2c7b93

PJBot removed the Changes: UI Changes: Might require knowledge of UI design or code. label Oct 9, 2025

github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Oct 12, 2025

Pok27 added 2 commits October 13, 2025 12:06

Merge branch 'master' into missing-localization-cmd

6aacdb5

fix marge

2ddee2e

github-actions bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Oct 13, 2025

VerinSenpai added S: Needs Engine Update Status: Requires a newer version of the Robust Toolbox engine. and removed S: Needs Engine PR Merged Status: Requires an existing Robust Toolbox PR to be merged first. labels Oct 13, 2025

VerinSenpai reviewed Oct 13, 2025

View reviewed changes

VerinSenpai suggested changes Oct 13, 2025

View reviewed changes

Pok27 added 2 commits October 14, 2025 11:59

fix for review 2

9b228d5

oops

9c05b32

Pok27 requested a review from VerinSenpai October 14, 2025 09:19

the end

4debb0e

VerinSenpai approved these changes Oct 14, 2025

View reviewed changes

Pok27 added 2 commits October 14, 2025 14:57

LocalizationManager -> Loc

d80db9c

minor update

53b1819


		public override string Command => "showsubfloor";

		public override string Help => Loc.GetString($"cmd-{Command}-help", ("command", Command));


		public override string Command => "showaccessreaders";

		public override string Help => Loc.GetString($"cmd-{Command}-help", ("command", Command));

Attempt to fix all unlocalized lines (part 2: cmd) #40752

Are you sure you want to change the base?

Attempt to fix all unlocalized lines (part 2: cmd) #40752

Conversation

Pok27 commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 7, 2025

Uh oh!

Siginanto commented Oct 8, 2025

Uh oh!

Princess-Cheeseballs commented Oct 8, 2025

Uh oh!

VerinSenpai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pok27 Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pok27 Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VerinSenpai Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pok27 Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pok27 commented Oct 8, 2025

Uh oh!

github-actions bot commented Oct 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slarticodefast Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 12, 2025

Uh oh!

VerinSenpai commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pok27 commented Oct 13, 2025

Uh oh!

VerinSenpai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pok27 commented Oct 7, 2025 •

edited

Loading

Pok27 Oct 8, 2025 •

edited

Loading

Pok27 Oct 8, 2025 •

edited

Loading

VerinSenpai Oct 9, 2025 •

edited

Loading

Pok27 Oct 9, 2025 •

edited

Loading

slarticodefast Oct 9, 2025 •

edited

Loading

VerinSenpai commented Oct 13, 2025 •

edited

Loading