Fix Languages Again (#893)

# Description
4 fixes:
- Fixed translators stopping to provide a language for a fraction of a
tick, causing the provided language to get de-selected when moving a
translator within your inventory (from hands to pocket or otherwise)
- Fixed translators not updating your languages when running out of
power (literally forgot to call UpdateEntityLanguages)
- Fixed language menu breaking after you reconnect to the server (the
issue is tricky, apparently all subscriptions made by ui controllers are
invalidated when the client switches from gameplay state to menu state
(the "you are disconnected" one), but never calls Initialize() for them
again, which means they can never re-create the same subscriptions...
Which explains why the ahelp menu was breaking for me after
reconnecting. All UI controllers that make event subscriptions are
affected by this bug)
- Fixed the language menu button not updating when you close the menu
manually.

# Changelog
:cl:
- fix: Fixed a couple issues with the language menu UI and translators.

Content.Client/Language/LanguageMenuWindow.xaml.cs

@@ -1,4 +1,5 @@
 using Content.Client.Language.Systems;
+using Content.Shared.Language.Events;
 using Robust.Client.AutoGenerated;
 using Robust.Client.UserInterface.Controls;
 using Robust.Client.UserInterface.CustomControls;
@@ -7,7 +8,7 @@
 namespace Content.Client.Language;
 
 [GenerateTypedNameReferences]
-public sealed partial class LanguageMenuWindow : DefaultWindow
+public sealed partial class LanguageMenuWindow : DefaultWindow, IEntityEventSubscriber
 {
     private readonly LanguageSystem _clientLanguageSystem;
     private readonly List<EntryState> _entries = new();
@@ -17,6 +18,14 @@ public LanguageMenuWindow()
     {
         RobustXamlLoader.Load(this);
         _clientLanguageSystem = IoCManager.Resolve<IEntitySystemManager>().GetEntitySystem<LanguageSystem>();
+
+        _clientLanguageSystem.OnLanguagesChanged += OnUpdateState;
+    }
+
+    protected override void Dispose(bool disposing)
+    {
+        base.Dispose(disposing);
+        _clientLanguageSystem.OnLanguagesChanged -= OnUpdateState;
     }
 
     protected override void Opened()
@@ -28,6 +37,11 @@ protected override void Opened()
     }
 
 
+    private void OnUpdateState(object? sender, LanguagesUpdatedMessage args)
+    {
+        UpdateState(args.CurrentLanguage, args.Spoken);
+    }
+
     public void UpdateState(string currentLanguage, List<string> spokenLanguages)
     {
         var langName = Loc.GetString($"language-{currentLanguage}-name");

Content.Client/Language/Systems/LanguageSystem.cs

@@ -29,6 +29,8 @@ public sealed class LanguageSystem : SharedLanguageSystem
     /// </summary>
     public List<string> UnderstoodLanguages { get; private set; } = new();
 
+    public event EventHandler<LanguagesUpdatedMessage>? OnLanguagesChanged;
+
     public override void Initialize()
     {
         base.Initialize();
@@ -39,9 +41,13 @@ public override void Initialize()
 
     private void OnLanguagesUpdated(LanguagesUpdatedMessage message)
     {
+        // TODO this entire thing is horrible. If someone is willing to refactor this, LanguageSpeakerComponent should become shared with SendOnlyToOwner = true
+        // That way, this system will be able to use the existing networking infrastructure instead of relying on this makeshift... whatever this is.
         CurrentLanguage = message.CurrentLanguage;
         SpokenLanguages = message.Spoken;
         UnderstoodLanguages = message.Understood;
+
+        OnLanguagesChanged?.Invoke(this, message);
     }
 
     private void OnRunLevelChanged(object? sender, RunLevelChangedEventArgs args)

Content.Client/UserInterface/Systems/Language/LanguageMenuUIController.cs

@@ -2,7 +2,6 @@
 using Content.Client.Gameplay;
 using Content.Client.UserInterface.Controls;
 using Content.Shared.Input;
-using Content.Shared.Language.Events;
 using Robust.Client.UserInterface.Controllers;
 using Robust.Client.UserInterface.Controls;
 using Robust.Shared.Input.Binding;
@@ -18,19 +17,24 @@ public sealed class LanguageMenuUIController : UIController, IOnStateEntered<Gam
     public LanguageMenuWindow? LanguageWindow;
     private MenuButton? LanguageButton => UIManager.GetActiveUIWidgetOrNull<MenuBar.Widgets.GameTopMenuBar>()?.LanguageButton;
 
-    public override void Initialize()
-    {
-        SubscribeNetworkEvent((LanguagesUpdatedMessage message, EntitySessionEventArgs _) =>
-            LanguageWindow?.UpdateState(message.CurrentLanguage, message.Spoken));
-    }
-
     public void OnStateEntered(GameplayState state)
     {
         DebugTools.Assert(LanguageWindow == null);
 
         LanguageWindow = UIManager.CreateWindow<LanguageMenuWindow>();
         LayoutContainer.SetAnchorPreset(LanguageWindow, LayoutContainer.LayoutPreset.CenterTop);
 
+        LanguageWindow.OnClose += () =>
+        {
+            if (LanguageButton != null)
+                LanguageButton.Pressed = false;
+        };
+        LanguageWindow.OnOpen += () =>
+        {
+            if (LanguageButton != null)
+                LanguageButton.Pressed = true;
+        };
+
         CommandBinds.Builder.Bind(ContentKeyFunctions.OpenLanguageMenu,
             InputCmdHandler.FromDelegate(_ => ToggleWindow())).Register<LanguageMenuUIController>();
     }
@@ -60,12 +64,6 @@ public void LoadButton()
             return;
 
         LanguageButton.OnPressed += LanguageButtonPressed;
-
-        if (LanguageWindow == null)
-            return;
-
-        LanguageWindow.OnClose += () => LanguageButton.Pressed = false;
-        LanguageWindow.OnOpen += () => LanguageButton.Pressed = true;
     }
 
     private void LanguageButtonPressed(ButtonEventArgs args)

Content.Server/Language/TranslatorSystem.cs

@@ -9,6 +9,7 @@
 using Content.Shared.PowerCell;
 using Content.Shared.Language.Components.Translators;
 using Robust.Shared.Containers;
+using Robust.Shared.Timing;
 
 namespace Content.Server.Language;
 
@@ -27,7 +28,7 @@ public override void Initialize()
         SubscribeLocalEvent<HoldsTranslatorComponent, DetermineEntityLanguagesEvent>(OnProxyDetermineLanguages);
 
         SubscribeLocalEvent<HandheldTranslatorComponent, EntGotInsertedIntoContainerMessage>(OnTranslatorInserted);
-        SubscribeLocalEvent<HandheldTranslatorComponent, EntGotRemovedFromContainerMessage>(OnTranslatorRemoved);
+        SubscribeLocalEvent<HandheldTranslatorComponent, EntParentChangedMessage>(OnTranslatorParentChanged);
         SubscribeLocalEvent<HandheldTranslatorComponent, ActivateInWorldEvent>(OnTranslatorToggle);
         SubscribeLocalEvent<HandheldTranslatorComponent, PowerCellSlotEmptyEvent>(OnPowerCellSlotEmpty);
     }
@@ -65,8 +66,7 @@ private void OnProxyDetermineLanguages(EntityUid uid, HoldsTranslatorComponent c
 
     private void OnTranslatorInserted(EntityUid translator, HandheldTranslatorComponent component, EntGotInsertedIntoContainerMessage args)
     {
-        if (args.Container.Owner is not {Valid: true} holder
-            || !EntityManager.HasComponent<LanguageSpeakerComponent>(holder))
+        if (args.Container.Owner is not {Valid: true} holder || !HasComp<LanguageSpeakerComponent>(holder))
             return;
 
         var intrinsic = EnsureComp<HoldsTranslatorComponent>(holder);
@@ -75,14 +75,19 @@ private void OnTranslatorInserted(EntityUid translator, HandheldTranslatorCompon
         _language.UpdateEntityLanguages(holder);
     }
 
-    private void OnTranslatorRemoved(EntityUid translator, HandheldTranslatorComponent component, EntGotRemovedFromContainerMessage args)
+    private void OnTranslatorParentChanged(EntityUid translator, HandheldTranslatorComponent component, EntParentChangedMessage args)
     {
-        if (args.Container.Owner is not {Valid: true} holder
-            || !EntityManager.TryGetComponent<HoldsTranslatorComponent>(holder, out var intrinsic))
+        if (!HasComp<HoldsTranslatorComponent>(args.OldParent))
             return;
 
-        intrinsic.Translators.RemoveWhere(it => it.Owner == translator);
-        _language.UpdateEntityLanguages(holder);
+        // Update the translator on the next tick - this is necessary because there's a good chance the removal from a container
+        // Was caused by the player moving the translator within their inventory rather than removing it.
+        // If that is not the case, then OnProxyDetermineLanguages will remove this translator from HoldsTranslatorComponent.Translators.
+        Timer.Spawn(0, () =>
+        {
+            if (Exists(args.OldParent) && TryComp<LanguageSpeakerComponent>(args.OldParent, out var speaker))
+                _language.UpdateEntityLanguages(args.OldParent.Value, speaker);
+        });
     }
 
     private void OnTranslatorToggle(EntityUid translator, HandheldTranslatorComponent translatorComp, ActivateInWorldEvent args)
@@ -125,6 +130,9 @@ private void OnPowerCellSlotEmpty(EntityUid translator, HandheldTranslatorCompon
         component.Enabled = false;
         _powerCell.SetPowerCellDrawEnabled(translator, false);
         OnAppearanceChange(translator, component);
+
+        if (_containers.TryGetContainingContainer(translator, out var holderCont) && TryComp<LanguageSpeakerComponent>(holderCont.Owner, out var languageComp))
+            _language.UpdateEntityLanguages(holderCont.Owner, languageComp);
     }
 
     private void CopyLanguages(BaseTranslatorComponent from, DetermineEntityLanguagesEvent to, LanguageKnowledgeComponent knowledge)