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

Non-Destructive Universal Language #899

Conversation

VMSolidus
Copy link
Member

@VMSolidus VMSolidus commented Sep 10, 2024

Description

UniversalLanguageSpeakerComponent has NUMEROUS issues, which previously were easy to ignore since it was a language only obtainable by Admins. But now that it's both a Psionic Power(And a trait that adds said power), the issues with it have been highlighted. Here's just a FEW

  1. UniversalLanguageSpeaker overwrites all known languages, preventing you from speaking anything else
  2. It overwrites the ability to use sign language.
  3. It also overwrites the Mute trait.

To fix that, I've made it follow MOSTLY the logic all the other languages use, so that it's less of a special snowflake case. Now if you have access to it, it will appear in your language list alongside other languages, rather than fully replacing the entire list. That way you can intentionally choose not to speak in a language understood by all.

Fuck it, I also added the ability for the TraitSystem to just call LanguageSystem and directly add arbitrarily any desired language, rather than needing a component to do so.

Changelog

🆑

  • fix: UniversalLanguageSpeaker(And Xenoglossy by extension) will now appear in your language menu alongside other known languages, rather than replace all known languages. You can effectively now choose whether or not to speak it, or to use a normal language.
  • add: Traits can now add Languages directly.
  • add: Traits can now remove Languages directly.

@github-actions github-actions bot added Changes: C# Changes any cs files Changes: YML Changes any yml files labels Sep 10, 2024
@FoxxoTrystan
Copy link
Member

Reading this from a laptop, i suggest letting @Mnemotechnician a read as currently unable to do it correctly.

@Alithsko
Copy link

I'm the one who brought this to light to OP, and it was worse than overwriting mute as it gave you the message that you cannot speak with no other mute friendly choices like sign, which made it a strict downgrade if you get it, happened to me in a game so this is a godsend

Copy link
Contributor

@Mnemotechnician Mnemotechnician left a comment

Choose a reason for hiding this comment

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

On a side note, I hate how this PR is straight up hardcoding this behavior in the trait system, but I guess it's acceptable for now since we do not have a proper system in place... Maybe in the future we can refactor the trait system to accept polymorphic actions instead, similarly to e.g. reagent effects, or damage behaviors, or interactions...

Content.Server/Language/LanguageSystem.cs Outdated Show resolved Hide resolved
Content.Server/Language/LanguageSystem.cs Outdated Show resolved Hide resolved
Content.Server/Language/LanguageSystem.cs Outdated Show resolved Hide resolved
Content.Server/Traits/TraitSystem.cs Show resolved Hide resolved
Content.Server/Traits/TraitSystem.cs Show resolved Hide resolved
@Mnemotechnician
Copy link
Contributor

Mnemotechnician commented Sep 10, 2024

Also, this PR needs to update the method LanguageSystem.Networking.SendLanguageStateToClient.

Co-authored-by: Mnemotechnican <[email protected]>
Signed-off-by: VMSolidus <[email protected]>
Copy link
Contributor

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

@github-actions github-actions bot added the Status: Merge Conflict FIX YOUR PR AAAGH label Sep 14, 2024
@github-actions github-actions bot removed the Status: Merge Conflict FIX YOUR PR AAAGH label Sep 14, 2024
@VMSolidus
Copy link
Member Author

8mb.video-Hk8-dYRuHvKC.mp4

I have finished fixing this PR, and finished testing to verify that everything works correctly.

@VMSolidus
Copy link
Member Author

I ALSO REQUIRE THIS PR IN ORDER TO CONTINUE WORKING ON PSIONICS.

@github-actions github-actions bot added the Status: Needs Review Someone please review this label Sep 17, 2024
@FoxxoTrystan FoxxoTrystan added Priority: 2-High Needs to be resolved as soon as possible Size: 3-Medium For medium issues/PRs Status: Needs Review Someone please review this and removed Status: Needs Review Someone please review this labels Sep 17, 2024
@FoxxoTrystan FoxxoTrystan requested review from a team, DEATHB4DEFEAT, Peptide90, Pspritechologist and OldDanceJacket and removed request for a team September 17, 2024 19:35
@VMSolidus
Copy link
Member Author

image

Translator on:
image

Translator off:
image

Done here,

Copy link
Contributor

@Mnemotechnician Mnemotechnician left a comment

Choose a reason for hiding this comment

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

Do not remove the optional component parameters. Passing components around instead of fetching them anew every time they are needed is the preferred style in ss14. You can use Entity<SomeComp> instead of EntityUid if you want prettier code.

Content.Server/Language/LanguageSystem.cs Show resolved Hide resolved
Content.Server/Language/LanguageSystem.cs Show resolved Hide resolved
@VMSolidus
Copy link
Member Author

Do not remove the optional component parameters. Passing components around instead of fetching them anew every time they are needed is the preferred style in ss14. You can use Entity<SomeComp> instead of EntityUid if you want prettier code.

No.

Copy link
Member

@FoxxoTrystan FoxxoTrystan left a comment

Choose a reason for hiding this comment

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

Could have been nicer then just "no"
But at least this works.

@VMSolidus
Copy link
Member Author

Could have been nicer then just "no" But at least this works.

Yep, and I'm going to go immediately start work on Language Menu now.

@VMSolidus VMSolidus merged commit 8b5dcd6 into Simple-Station:master Sep 20, 2024
9 of 10 checks passed
SimpleStation14 added a commit that referenced this pull request Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: C# Changes any cs files Changes: YML Changes any yml files Priority: 2-High Needs to be resolved as soon as possible Size: 3-Medium For medium issues/PRs Status: Needs Review Someone please review this
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants