Fix prediction and popups for actions (#11871)

Content.Client/Actions/ActionsSystem.cs

@@ -1,12 +1,16 @@
 using System.IO;
+using System.Linq;
+using Content.Client.Popups;
 using Content.Shared.Actions;
 using Content.Shared.Actions.ActionTypes;
 using JetBrains.Annotations;
 using Robust.Client.GameObjects;
 using Robust.Client.Player;
+using Robust.Shared.Audio;
 using Robust.Shared.ContentPack;
 using Robust.Shared.GameStates;
 using Robust.Shared.Input.Binding;
+using Robust.Shared.Player;
 using Robust.Shared.Serialization.Manager;
 using Robust.Shared.Serialization.Markdown;
 using Robust.Shared.Serialization.Markdown.Mapping;
@@ -19,14 +23,19 @@ namespace Content.Client.Actions
     [UsedImplicitly]
     public sealed class ActionsSystem : SharedActionsSystem
     {
+        public delegate void OnActionReplaced(ActionType existing, ActionType action);
+
         [Dependency] private readonly IPlayerManager _playerManager = default!;
         [Dependency] private readonly IResourceManager _resources = default!;
         [Dependency] private readonly ISerializationManager _serialization = default!;
 
-        public event Action<ActionType>? OnActionAdded;
+        [Dependency] private readonly PopupSystem _popupSystem = default!;
-        public event Action<ActionType>? OnActionRemoved;
+
-        public event Action<ActionsComponent>? OnLinkActions;
+        public event Action<ActionType>? ActionAdded;
-        public event Action? OnUnlinkActions;
+        public event Action<ActionType>? ActionRemoved;
+        public event OnActionReplaced? ActionReplaced;
+        public event Action<ActionsComponent>? LinkActions;
+        public event Action? UnlinkActions;
         public event Action? ClearAssignments;
         public event Action<List<SlotAssignment>>? AssignSlot;
 
@@ -42,43 +51,128 @@ namespace Content.Client.Actions
 
         private void HandleComponentState(EntityUid uid, ActionsComponent component, ref ComponentHandleState args)
         {
-            if (args.Current is not ActionsComponentState currentState)
+            if (args.Current is not ActionsComponentState state)
                 return;
 
-            List<ActionType> added = new();
+            var serverActions = new SortedSet<ActionType>(state.Actions);
-            List<ActionType> removed = new();
+            var removed = new List<ActionType>();
 
-            foreach (var actionType in component.Actions)
+            foreach (var act in component.Actions.ToList())
             {
-                if (!currentState.Actions.Contains(actionType))
+                if (act.ClientExclusive)
+                    continue;
+
+                if (!serverActions.TryGetValue(act, out var serverAct))
                 {
-                    removed.Add(actionType);
+                    component.Actions.Remove(act);
+                    if (act.AutoRemove)
+                        removed.Add(act);
+
+                    continue;
+                }
+
+                act.CopyFrom(serverAct);
+                serverActions.Remove(serverAct);
+            }
+
+            var added = new List<ActionType>();
+
+            // Anything that remains is a new action
+            foreach (var newAct in serverActions)
+            {
+                // We create a new action, not just sorting a reference to the state's action.
+                var action = (ActionType) newAct.Clone();
+                component.Actions.Add(action);
+                added.Add(action);
+            }
+
+            foreach (var action in removed)
+            {
+                ActionRemoved?.Invoke(action);
+            }
+
+            foreach (var action in added)
+            {
+                ActionAdded?.Invoke(action);
             }
         }
 
-            foreach (var serverAction in currentState.Actions)
+        protected override void AddActionInternal(ActionsComponent comp, ActionType action)
         {
-                if (!component.Actions.TryGetValue(serverAction, out var clientAction))
+            // Sometimes the client receives actions from the server, before predicting that newly added components will add
+            // their own shared actions. Just in case those systems ever decided to directly access action properties (e.g.,
+            // action.Toggled), we will remove duplicates:
+            if (comp.Actions.TryGetValue(action, out var existing))
             {
-                    added.Add((ActionType) serverAction.Clone());
+                comp.Actions.Remove(existing);
+                ActionReplaced?.Invoke(existing, action);
             }
-                else
+
+            comp.Actions.Add(action);
+        }
+
+        public override void AddAction(EntityUid uid, ActionType action, EntityUid? provider, ActionsComponent? comp = null, bool dirty = true)
         {
-                    clientAction.CopyFrom(serverAction);
+            if (!Resolve(uid, ref comp, false))
+                return;
+
+            base.AddAction(uid, action, provider, comp, dirty);
+
+            if (uid == _playerManager.LocalPlayer?.ControlledEntity)
+                ActionAdded?.Invoke(action);
+        }
+
+        public override void RemoveActions(EntityUid uid, IEnumerable<ActionType> actions, ActionsComponent? comp = null, bool dirty = true)
+        {
+            if (uid != _playerManager.LocalPlayer?.ControlledEntity)
+                return;
+
+            if (!Resolve(uid, ref comp, false))
+                return;
+
+            var actionList = actions.ToList();
+            base.RemoveActions(uid, actionList, comp, dirty);
+
+            foreach (var act in actionList)
+            {
+                if (act.AutoRemove)
+                    ActionRemoved?.Invoke(act);
             }
         }
 
-            foreach (var actionType in added)
+        /// <summary>
+        ///     Execute convenience functionality for actions (pop-ups, sound, speech)
+        /// </summary>
+        protected override bool PerformBasicActions(EntityUid user, ActionType action)
         {
-                component.Actions.Add(actionType);
+            var performedAction = action.Sound != null
-                OnActionAdded?.Invoke(actionType);
+                                  || !string.IsNullOrWhiteSpace(action.UserPopup)
+                                  || !string.IsNullOrWhiteSpace(action.Popup);
+
+            if (!GameTiming.IsFirstTimePredicted)
+                return performedAction;
+
+            if (!string.IsNullOrWhiteSpace(action.UserPopup))
+            {
+                var msg = (!action.Toggled || string.IsNullOrWhiteSpace(action.PopupToggleSuffix))
+                    ? Loc.GetString(action.UserPopup)
+                    : Loc.GetString(action.UserPopup + action.PopupToggleSuffix);
+
+                _popupSystem.PopupEntity(msg, user);
+            }
+            else if (!string.IsNullOrWhiteSpace(action.Popup))
+            {
+                var msg = (!action.Toggled || string.IsNullOrWhiteSpace(action.PopupToggleSuffix))
+                    ? Loc.GetString(action.Popup)
+                    : Loc.GetString(action.Popup + action.PopupToggleSuffix);
+
+                _popupSystem.PopupEntity(msg, user);
             }
 
-            foreach (var actionType in removed)
+            if (action.Sound != null)
-            {
+                SoundSystem.Play(action.Sound.GetSound(), Filter.Local(), user, action.AudioParams);
-                component.Actions.Remove(actionType);
+
-                OnActionRemoved?.Invoke(actionType);
+            return performedAction;
-            }
         }
 
         private void OnPlayerAttached(EntityUid uid, ActionsComponent component, PlayerAttachedEvent args)
@@ -86,7 +180,7 @@ namespace Content.Client.Actions
             if (uid != _playerManager.LocalPlayer?.ControlledEntity)
                 return;
 
-            OnLinkActions?.Invoke(component);
+            LinkActions?.Invoke(component);
             PlayerActions = component;
         }
 
@@ -95,7 +189,7 @@ namespace Content.Client.Actions
             if (uid != _playerManager.LocalPlayer?.ControlledEntity)
                 return;
 
-            OnUnlinkActions?.Invoke();
+            UnlinkActions?.Invoke();
             PlayerActions = null;
         }
 

Content.Client/UserInterface/Systems/Actions/ActionUIController.cs

@@ -85,21 +85,25 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
     public void OnStateEntered(GameplayState state)
     {
         DebugTools.Assert(_window == null);
+
         _window = UIManager.CreateWindow<ActionsWindow>();
         _actionButton = UIManager.GetActiveUIWidget<MenuBar.Widgets.GameTopMenuBar>().ActionButton;
+        _actionsBar = UIManager.GetActiveUIWidget<ActionsBar>();
         LayoutContainer.SetAnchorPreset(_window, LayoutContainer.LayoutPreset.CenterTop);
-        _window.OnClose += () => { _actionButton.Pressed = false; };
+
-        _window.OnOpen += () => { _actionButton.Pressed = true; };
+        _window.OnOpen += OnWindowOpened;
+        _window.OnClose += OnWindowClosed;
         _window.ClearButton.OnPressed += OnClearPressed;
         _window.SearchBar.OnTextChanged += OnSearchChanged;
         _window.FilterButton.OnItemSelected += OnFilterSelected;
+        _actionButton.OnPressed += ActionButtonPressed;
+        _actionsBar.PageButtons.LeftArrow.OnPressed += OnLeftArrowPressed;
+        _actionsBar.PageButtons.RightArrow.OnPressed += OnRightArrowPressed;
+        _actionsSystem.ActionReplaced += OnActionReplaced;
+
         UpdateFilterLabel();
         SearchAndDisplay();
 
-        _actionsBar = UIManager.GetActiveUIWidget<ActionsBar>();
-        _actionsBar.PageButtons.LeftArrow.OnPressed += OnLeftArrowPressed;
-        _actionsBar.PageButtons.RightArrow.OnPressed += OnRightArrowPressed;
-        _actionButton.OnPressed += ActionButtonPressed;
         _dragShadow.Orphan();
         UIManager.PopupRoot.AddChild(_dragShadow);
 
@@ -134,10 +138,28 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
             .Register<ActionUIController>();
     }
 
+    private void OnWindowOpened()
+    {
+        if (_actionButton != null)
+            _actionButton.Pressed = true;
+    }
+
+    private void OnWindowClosed()
+    {
+        if (_actionButton != null)
+            _actionButton.Pressed = false;
+    }
+
     public void OnStateExited(GameplayState state)
     {
         if (_window != null)
         {
+            _window.OnOpen -= OnWindowOpened;
+            _window.OnClose -= OnWindowClosed;
+            _window.ClearButton.OnPressed -= OnClearPressed;
+            _window.SearchBar.OnTextChanged -= OnSearchChanged;
+            _window.FilterButton.OnItemSelected -= OnFilterSelected;
+
             _window.Dispose();
             _window = null;
         }
@@ -194,6 +216,18 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
         ChangePage(_currentPageIndex + 1);
     }
 
+    private void OnActionReplaced(ActionType existing, ActionType action)
+    {
+        if (_container == null)
+            return;
+
+        foreach (var button in _container.GetButtons())
+        {
+            if (button.Action == existing)
+                button.UpdateData(_entities, action);
+        }
+    }
+
     private void ActionButtonPressed(ButtonEventArgs args)
     {
         ToggleWindow();
@@ -293,14 +327,14 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
             if (action.Keywords.Any(keyword => search.Contains(keyword, StringComparison.OrdinalIgnoreCase)))
                 return true;
 
-            if (action.DisplayName.Contains((string) search, StringComparison.OrdinalIgnoreCase))
+            if (action.DisplayName.Contains(search, StringComparison.OrdinalIgnoreCase))
                 return true;
 
             if (action.Provider == null || action.Provider == _actionsSystem.PlayerActions?.Owner)
                 return false;
 
             var name = _entities.GetComponent<MetaDataComponent>(action.Provider.Value).EntityName;
-            return name.Contains((string) search, StringComparison.OrdinalIgnoreCase);
+            return name.Contains(search, StringComparison.OrdinalIgnoreCase);
         });
 
         PopulateActions(actions);
@@ -492,16 +526,16 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
 
     public void OnSystemLoaded(ActionsSystem system)
     {
-        _actionsSystem.OnLinkActions += OnComponentLinked;
+        _actionsSystem.LinkActions += OnComponentLinked;
-        _actionsSystem.OnUnlinkActions += OnComponentUnlinked;
+        _actionsSystem.UnlinkActions += OnComponentUnlinked;
         _actionsSystem.ClearAssignments += ClearActions;
         _actionsSystem.AssignSlot += AssignSlots;
     }
 
     public void OnSystemUnloaded(ActionsSystem system)
     {
-        _actionsSystem.OnLinkActions -= OnComponentLinked;
+        _actionsSystem.LinkActions -= OnComponentLinked;
-        _actionsSystem.OnUnlinkActions -= OnComponentUnlinked;
+        _actionsSystem.UnlinkActions -= OnComponentUnlinked;
         _actionsSystem.ClearAssignments -= ClearActions;
         _actionsSystem.AssignSlot -= AssignSlots;
     }