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

Auto Command Localization #2331

Closed

Conversation

nootsponge
Copy link

@nootsponge nootsponge commented Dec 13, 2021

To begin satisfying space-wizards/space-station-14#5543, these changes allow for command localization to be automatically recognized if the correct keys are defined in the locale. This takes some heavy inspiration from how it was implemented for EntityPrototypes:

Additions:

  • New abstract ConsoleCommand class, extends interface and overrides the Description and Help members.
    • Commands should inherit this new class to use the automatic localization
  • CommandLocData record to house the localization data.

Modifications:

  • Helper method GetCommandData(), which fetches the fluent message from the locale and returns it for the abstract.
    • This was also added to the interface.

I see that the LocalizationManager has been split between 3 partials, (main, Entity, and Fluent functions), but I didn't see a reason to add an entire separate partial just for one method, let me know if this is unwanted.

I also just used the [Dependency] attribute to resolve the loc manager in the abstract, and didn't include a PostInject or anything like that, but I figured it was fine. Let me know if this is also problematic.

Eventually I would think about changing over the reflection manager for console commands to look for the abstract rather than the interface, but for backwards compatibility I've left it unchanged for now. I'm working on a content-side to this PR that implements the new class and adds localization for all commands.

Thoughts?

Edit: meant to include that the message keys it looks for are of the format cmd-<command>, so for defining localization for the changelog command, it would be cmd-changelog with the value being the description, then the attribute .help being the help localization.

@nootsponge
Copy link
Author

@Acruid @PJB3005 @Silvertorch5 This is my first contrib and I'd like some feedback!

@metalgearsloth
Copy link
Contributor

someone give this man a review.

@PJB3005
Copy link
Member

PJB3005 commented Jan 25, 2022

Didn't we discuss this in a maintainer meeting at some point?

@DrSmugleaf
Copy link
Member

Didn't we discuss this in a maintainer meeting at some point?

Yes, fourth one. Conclusion was to localize everything about commands except command names.

@Acruid
Copy link
Contributor

Acruid commented Jan 25, 2022

So is this ready for review?

@nootsponge
Copy link
Author

So is this ready for review?

I can't really remember the state of this MR, I'll need to boot up VSCode again and see if it's still valid and why this a draft. Afaik this is almost ready to go, just need to work on getting the content-side ready.

@Acruid Acruid added the Status: Stale The PR has gone more than 14 days without activity. label Feb 16, 2022
@metalgearsloth
Copy link
Contributor

re-open when you get a chance to work on it

@Ygg01 Ygg01 mentioned this pull request Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Stale The PR has gone more than 14 days without activity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants