Remove speech & popups from actions (#15747)

Content.Client/Actions/ActionsSystem.cs

@@ -161,39 +161,6 @@ namespace Content.Client.Actions
                 ActionRemoved?.Invoke(action);
         }
 
-        /// <summary>
-        ///     Execute convenience functionality for actions (pop-ups, sound, speech)
-        /// </summary>
-        protected override bool PerformBasicActions(EntityUid user, ActionType action, bool predicted)
-        {
-            var performedAction = action.Sound != null
-                                  || !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);
-            }
-
-            _audio.Play(action.Sound, Filter.Local(), user, false);
-            return performedAction;
-        }
-
         private void OnPlayerAttached(EntityUid uid, ActionsComponent component, PlayerAttachedEvent args)
         {
             LinkAllActions(component);

Content.Client/Ghost/GhostComponent.cs

@@ -16,7 +16,6 @@ namespace Content.Client.Ghost
             Icon = new SpriteSpecifier.Texture(new ("Interface/VerbIcons/light.svg.192dpi.png")),
             DisplayName = "ghost-gui-toggle-lighting-manager-name",
             Description = "ghost-gui-toggle-lighting-manager-desc",
-            UserPopup = "ghost-gui-toggle-lighting-manager-popup",
             ClientExclusive = true,
             CheckCanInteract = false,
             Event = new ToggleLightingActionEvent(),
@@ -27,7 +26,6 @@ namespace Content.Client.Ghost
             Icon = new SpriteSpecifier.Texture(new ("Interface/VerbIcons/vv.svg.192dpi.png")),
             DisplayName = "ghost-gui-toggle-fov-name",
             Description = "ghost-gui-toggle-fov-desc",
-            UserPopup = "ghost-gui-toggle-fov-popup",
             ClientExclusive = true,
             CheckCanInteract = false,
             Event = new ToggleFoVActionEvent(),
@@ -38,7 +36,6 @@ namespace Content.Client.Ghost
             Icon = new SpriteSpecifier.Rsi(new ("Mobs/Ghosts/ghost_human.rsi"), "icon"),
             DisplayName = "ghost-gui-toggle-ghost-visibility-name",
             Description = "ghost-gui-toggle-ghost-visibility-desc",
-            UserPopup = "ghost-gui-toggle-ghost-visibility-popup",
             ClientExclusive = true,
             CheckCanInteract = false,
             Event = new ToggleGhostsActionEvent(),

Content.Client/Ghost/GhostSystem.cs

@@ -1,6 +1,7 @@
 using Content.Client.Movement.Systems;
 using Content.Shared.Actions;
 using Content.Shared.Ghost;
+using Content.Shared.Popups;
 using JetBrains.Annotations;
 using Robust.Client.Console;
 using Robust.Client.GameObjects;
@@ -17,6 +18,7 @@ namespace Content.Client.Ghost
         [Dependency] private readonly IPlayerManager _playerManager = default!;
         [Dependency] private readonly SharedActionsSystem _actions = default!;
         [Dependency] private readonly ILightManager _lightManager = default!;
+        [Dependency] private readonly SharedPopupSystem _popup = default!;
         [Dependency] private readonly ContentEyeSystem _contentEye = default!;
 
         public int AvailableGhostRoleCount { get; private set; }
@@ -90,6 +92,7 @@ namespace Content.Client.Ghost
             if (args.Handled)
                 return;
 
+            _popup.PopupEntity(Loc.GetString("ghost-gui-toggle-lighting-manager-popup"), args.Performer);
             _lightManager.Enabled = !_lightManager.Enabled;
             args.Handled = true;
         }
@@ -99,6 +102,7 @@ namespace Content.Client.Ghost
             if (args.Handled)
                 return;
 
+            _popup.PopupEntity(Loc.GetString("ghost-gui-toggle-fov-popup"), args.Performer);
             _contentEye.RequestToggleFov(uid);
             args.Handled = true;
         }
@@ -108,6 +112,7 @@ namespace Content.Client.Ghost
             if (args.Handled)
                 return;
 
+            _popup.PopupEntity(Loc.GetString("ghost-gui-toggle-ghost-visibility-popup"), args.Performer);
             ToggleGhostVisibility();
             args.Handled = true;
         }

Content.Server/Actions/ActionsSystem.cs

@@ -10,20 +10,5 @@ namespace Content.Server.Actions
     [UsedImplicitly]
     public sealed class ActionsSystem : SharedActionsSystem
     {
-        [Dependency] private readonly ChatSystem _chat = default!;
-        [Dependency] private readonly MetaDataSystem _metaSystem = default!;
-
-        protected override bool PerformBasicActions(EntityUid user, ActionType action, bool predicted)
-        {
-            var result = base.PerformBasicActions(user, action, predicted);
-
-            if (!string.IsNullOrWhiteSpace(action.Speech))
-            {
-                _chat.TrySendInGameICMessage(user, Loc.GetString(action.Speech), InGameICChatType.Speak, false);
-                result = true;
-            }
-
-            return result;
-        }
     }
 }

Content.Server/Magic/Events/ChangeComponentsSpellEvent.cs

@@ -6,7 +6,7 @@ namespace Content.Server.Magic.Events;
 /// <summary>
 ///     Spell that uses the magic of ECS to add & remove components. Components are first removed, then added.
 /// </summary>
-public sealed class ChangeComponentsSpellEvent : EntityTargetActionEvent
+public sealed class ChangeComponentsSpellEvent : EntityTargetActionEvent, ISpeakSpell
 {
     // TODO allow it to set component data-fields?
     // for now a Hackish way to do that is to remove & add, but that doesn't allow you to selectively set specific data fields.
@@ -18,4 +18,7 @@ public sealed class ChangeComponentsSpellEvent : EntityTargetActionEvent
     [DataField("toRemove")]
     [AlwaysPushInheritance]
     public HashSet<string> ToRemove = new();
+
+    [DataField("speech")]
+    public string? Speech { get; }
 }

Content.Server/Magic/Events/ISpeakSpell.cs

@@ -0,0 +1,10 @@
+namespace Content.Server.Magic.Events;
+
+public interface ISpeakSpell // The speak n spell interface
+{
+    /// <summary>
+    /// Localized string spoken by the caster when casting this spell.
+    /// </summary>
+    public string? Speech { get; }
+}
+

Content.Server/Magic/Events/InstantSpawnSpellEvent.cs

@@ -4,7 +4,7 @@ using Robust.Shared.Serialization.TypeSerializers.Implementations.Custom.Prototy
 
 namespace Content.Server.Magic.Events;
 
-public sealed class InstantSpawnSpellEvent : InstantActionEvent
+public sealed class InstantSpawnSpellEvent : InstantActionEvent, ISpeakSpell
 {
     /// <summary>
     /// What entity should be spawned.
@@ -15,6 +15,9 @@ public sealed class InstantSpawnSpellEvent : InstantActionEvent
     [DataField("preventCollide")]
     public bool PreventCollideWithCaster = true;
 
+    [DataField("speech")]
+    public string? Speech { get; }
+
     /// <summary>
     /// Gets the targeted spawn positons; may lead to multiple entities being spawned.
     /// </summary>

Content.Server/Magic/Events/KnockSpellEvent.cs

@@ -3,7 +3,7 @@ using Robust.Shared.Audio;
 
 namespace Content.Server.Magic.Events;
 
-public sealed class KnockSpellEvent : InstantActionEvent
+public sealed class KnockSpellEvent : InstantActionEvent, ISpeakSpell
 {
     /// <summary>
     /// The range this spell opens doors in
@@ -20,4 +20,7 @@ public sealed class KnockSpellEvent : InstantActionEvent
     /// </summary>
     [DataField("knockVolume")]
     public float KnockVolume = 5f;
+
+    [DataField("speech")]
+    public string? Speech { get; }
 }

Content.Server/Magic/Events/ProjectileSpellEvent.cs

@@ -5,7 +5,7 @@ using Robust.Shared.Serialization.TypeSerializers.Implementations.Custom.Prototy
 
 namespace Content.Server.Magic.Events;
 
-public sealed class ProjectileSpellEvent : WorldTargetActionEvent
+public sealed class ProjectileSpellEvent : WorldTargetActionEvent, ISpeakSpell
 {
     /// <summary>
     /// What entity should be spawned.
@@ -17,4 +17,7 @@ public sealed class ProjectileSpellEvent : WorldTargetActionEvent
     /// Gets the targeted spawn positions; may lead to multiple entities being spawned.
     /// </summary>
     [DataField("posData")] public MagicSpawnData Pos = new TargetCasterPos();
+
+    [DataField("speech")]
+    public string? Speech { get; }
 }

Content.Server/Magic/Events/SmiteSpellEvent.cs

@@ -2,11 +2,14 @@
 
 namespace Content.Server.Magic.Events;
 
-public sealed class SmiteSpellEvent : EntityTargetActionEvent
+public sealed class SmiteSpellEvent : EntityTargetActionEvent, ISpeakSpell
 {
     /// <summary>
     ///     Should this smite delete all parts/mechanisms gibbed except for the brain?
     /// </summary>
     [DataField("deleteNonBrainParts")]
     public bool DeleteNonBrainParts = true;
+
+    [DataField("speech")]
+    public string? Speech { get; }
 }

Content.Server/Magic/Events/TeleportSpellEvent.cs

@@ -3,11 +3,13 @@ using Robust.Shared.Audio;
 
 namespace Content.Server.Magic.Events;
 
-public sealed class TeleportSpellEvent : WorldTargetActionEvent
+public sealed class TeleportSpellEvent : WorldTargetActionEvent, ISpeakSpell
 {
     [DataField("blinkSound")]
     public SoundSpecifier BlinkSound = new SoundPathSpecifier("/Audio/Magic/blink.ogg");
 
+    [DataField("speech")]
+    public string? Speech { get; }
 
     /// <summary>
     /// Volume control for the spell.

Content.Server/Magic/Events/WorldSpawnSpellEvent.cs

@@ -3,7 +3,7 @@ using Content.Shared.Storage;
 
 namespace Content.Server.Magic.Events;
 
-public sealed class WorldSpawnSpellEvent : WorldTargetActionEvent
+public sealed class WorldSpawnSpellEvent : WorldTargetActionEvent, ISpeakSpell
 {
     // TODO:This class needs combining with InstantSpawnSpellEvent
 
@@ -25,5 +25,8 @@ public sealed class WorldSpawnSpellEvent : WorldTargetActionEvent
     /// Lifetime to set for the entities to self delete
     /// </summary>
     [DataField("lifetime")] public float? Lifetime;
+
+    [DataField("speech")]
+    public string? Speech { get; }
 }
 

Content.Server/Magic/MagicSystem.cs

@@ -1,5 +1,6 @@
 using Content.Server.Body.Components;
 using Content.Server.Body.Systems;
+using Content.Server.Chat.Systems;
 using Content.Server.Coordinates.Helpers;
 using Content.Server.Doors.Systems;
 using Content.Server.Magic.Events;
@@ -23,6 +24,7 @@ using Robust.Shared.Player;
 using Robust.Shared.Prototypes;
 using Robust.Shared.Random;
 using Robust.Shared.Serialization.Manager;
+using Robust.Shared.Serialization.Manager.Exceptions;
 
 namespace Content.Server.Magic;
 
@@ -46,6 +48,7 @@ public sealed class MagicSystem : EntitySystem
     [Dependency] private readonly PhysicsSystem _physics = default!;
     [Dependency] private readonly SharedTransformSystem _transformSystem = default!;
     [Dependency] private readonly SharedAudioSystem _audio = default!;
+    [Dependency] private readonly ChatSystem _chat = default!;
 
     public override void Initialize()
     {
@@ -144,6 +147,7 @@ public sealed class MagicSystem : EntitySystem
             }
         }
 
+        Speak(args);
         args.Handled = true;
     }
 
@@ -152,6 +156,9 @@ public sealed class MagicSystem : EntitySystem
         if (ev.Handled)
             return;
 
+        ev.Handled = true;
+        Speak(ev);
+
         var xform = Transform(ev.Performer);
         var userVelocity = _physics.GetMapLinearVelocity(ev.Performer);
 
@@ -170,6 +177,11 @@ public sealed class MagicSystem : EntitySystem
 
     private void OnChangeComponentsSpell(ChangeComponentsSpellEvent ev)
     {
+        if (ev.Handled)
+            return;
+        ev.Handled = true;
+        Speak(ev);
+
         foreach (var toRemove in ev.ToRemove)
         {
             if (_compFact.TryGetRegistration(toRemove, out var registration))
@@ -263,6 +275,7 @@ public sealed class MagicSystem : EntitySystem
         _transformSystem.SetCoordinates(args.Performer, args.Target);
         transform.AttachToGridOrMap();
         _audio.PlayPvs(args.BlinkSound, args.Performer, AudioParams.Default.WithVolume(args.BlinkVolume));
+        Speak(args);
         args.Handled = true;
     }
 
@@ -275,6 +288,9 @@ public sealed class MagicSystem : EntitySystem
         if (args.Handled)
             return;
 
+        args.Handled = true;
+        Speak(args);
+
         //Get the position of the player
         var transform = Transform(args.Performer);
         var coords = transform.Coordinates;
@@ -290,8 +306,6 @@ public sealed class MagicSystem : EntitySystem
             if (TryComp<DoorComponent>(entity, out var doorComp) && doorComp.State is not DoorState.Open)
                 _doorSystem.StartOpening(doorComp.Owner);
         }
-
-        args.Handled = true;
     }
 
     private void OnSmiteSpell(SmiteSpellEvent ev)
@@ -299,6 +313,9 @@ public sealed class MagicSystem : EntitySystem
         if (ev.Handled)
             return;
 
+        ev.Handled = true;
+        Speak(ev);
+
         var direction = Transform(ev.Target).MapPosition.Position - Transform(ev.Performer).MapPosition.Position;
         var impulseVector = direction * 10000;
 
@@ -337,7 +354,7 @@ public sealed class MagicSystem : EntitySystem
         var targetMapCoords = args.Target;
 
         SpawnSpellHelper(args.Contents, targetMapCoords, args.Lifetime, args.Offset);
-
+        Speak(args);
         args.Handled = true;
     }
 
@@ -373,4 +390,13 @@ public sealed class MagicSystem : EntitySystem
     }
 
     #endregion
+
+    private void Speak(BaseActionEvent args)
+    {
+        if (args is not ISpeakSpell speak || string.IsNullOrWhiteSpace(speak.Speech))
+            return;
+
+        _chat.TrySendInGameICMessage(args.Performer, Loc.GetString(speak.Speech),
+            InGameICChatType.Speak, false);
+    }
 }

Content.Shared/Actions/ActionTypes/ActionType.cs

@@ -157,39 +157,12 @@ public abstract class ActionType : IEquatable<ActionType>, IComparable, ICloneab
     [DataField("itemIconStyle")]
     public ItemActionIconStyle ItemIconStyle;
 
-    /// <summary>
-    ///     If not null, the user will speak these words when performing the action. Convenient feature to have for some
-    ///     actions. Gets passed through localization.
-    /// </summary>
-    [DataField("speech")]
-    public string? Speech;
-
     /// <summary>
     ///     If not null, this sound will be played when performing this action.
     /// </summary>
     [DataField("sound")]
     public SoundSpecifier? Sound;
 
-    /// <summary>
-    ///     A pop-up to show the user when performing this action. Gets passed through localization.
-    /// </summary>
-    [DataField("userPopup")]
-    public string? UserPopup;
-
-    /// <summary>
-    ///     A pop-up to show to all players when performing this action. Gets passed through localization.
-    /// </summary>
-    [DataField("popup")]
-    public string? Popup;
-
-    /// <summary>
-    ///     If not null, this string will be appended to the pop-up localization strings when the action was toggled on
-    ///     after execution. Exists to make it easy to have a different pop-up for turning the action on or off (e.g.,
-    ///     combat mode toggle).
-    /// </summary>
-    [DataField("popupToggleSuffix")]
-    public string? PopupToggleSuffix = null;
-
     /// <summary>
     ///     Compares two actions based on their properties. This is used to determine equality when the client requests the
     ///     server to perform some action. Also determines the order in which actions are automatically added to the action bar.
@@ -256,12 +229,8 @@ public abstract class ActionType : IEquatable<ActionType>, IComparable, ICloneab
         AutoRemove = toClone.AutoRemove;
         ItemIconStyle = toClone.ItemIconStyle;
         CheckCanInteract = toClone.CheckCanInteract;
-        Speech = toClone.Speech;
         UseDelay = toClone.UseDelay;
         Sound = toClone.Sound;
-        UserPopup = toClone.UserPopup;
-        Popup = toClone.Popup;
-        PopupToggleSuffix = toClone.PopupToggleSuffix;
         ItemIconStyle = toClone.ItemIconStyle;
         _entityIcon = toClone._entityIcon;
     }

Content.Shared/Actions/SharedActionsSystem.cs

@@ -284,8 +284,8 @@ public abstract class SharedActionsSystem : EntitySystem
             handled = actionEvent.Handled;
         }
 
-        // Execute convenience functionality (pop-ups, sound, speech)
+        _audio.PlayPredicted(action.Sound, performer,predicted ? performer : null);
-        handled |= PerformBasicActions(performer, action, predicted);
+        handled |= action.Sound != null;
 
         if (!handled)
             return; // no interaction occurred.
@@ -312,30 +312,6 @@ public abstract class SharedActionsSystem : EntitySystem
         if (dirty && component != null)
             Dirty(component);
     }
-
-    /// <summary>
-    ///     Execute convenience functionality for actions (pop-ups, sound, speech)
-    /// </summary>
-    protected virtual bool PerformBasicActions(EntityUid performer, ActionType action, bool predicted)
-    {
-        if (action.Sound == null && string.IsNullOrWhiteSpace(action.Popup))
-            return false;
-
-        var filter = predicted ? Filter.PvsExcept(performer) : Filter.Pvs(performer);
-
-        _audio.Play(action.Sound, filter, performer, true);
-
-        if (string.IsNullOrWhiteSpace(action.Popup))
-            return true;
-
-        var msg = (!action.Toggled || string.IsNullOrWhiteSpace(action.PopupToggleSuffix))
-            ? Loc.GetString(action.Popup)
-            : Loc.GetString(action.Popup + action.PopupToggleSuffix);
-
-        _popupSystem.PopupEntity(msg, performer, filter, true);
-
-        return true;
-    }
     #endregion
 
     #region AddRemoveActions

Content.Shared/CombatMode/SharedCombatModeSystem.cs

@@ -1,8 +1,10 @@
 using Content.Shared.Actions;
 using Content.Shared.Actions.ActionTypes;
+using Content.Shared.Popups;
 using Content.Shared.Targeting;
 using Robust.Shared.Prototypes;
 using Robust.Shared.Serialization;
+using Robust.Shared.Timing;
 
 namespace Content.Shared.CombatMode
 {
@@ -10,6 +12,8 @@ namespace Content.Shared.CombatMode
     {
         [Dependency] private readonly IPrototypeManager _protoMan = default!;
         [Dependency] private readonly SharedActionsSystem _actionsSystem = default!;
+        [Dependency] private readonly SharedPopupSystem _popup = default!;
+        [Dependency] private readonly IGameTiming _timing = default!;
 
         public override void Initialize()
         {
@@ -43,8 +47,14 @@ namespace Content.Shared.CombatMode
             if (args.Handled)
                 return;
 
-            SetInCombatMode(uid, !component.IsInCombatMode, component);
             args.Handled = true;
+            SetInCombatMode(uid, !component.IsInCombatMode, component);
+
+            if (!_timing.IsFirstTimePredicted)
+                return;
+
+            var msg = component.IsInCombatMode ? "action-popup-combat" : "action-popup-combat-enabled";
+            _popup.PopupEntity(Loc.GetString(msg), args.Performer);
         }
 
         public void SetCanDisarm(EntityUid entity, bool canDisarm, CombatModeComponent? component = null)

Resources/Prototypes/Actions/types.yml

@@ -73,8 +73,6 @@
   checkCanInteract: false
   icon: Interface/Actions/harmOff.png
   iconOn: Interface/Actions/harm.png
-  userPopup: action-popup-combat
-  popupToggleSuffix: -enabled
   event: !type:ToggleCombatActionEvent
 
 - type: instantAction

Resources/Prototypes/Magic/forcewall_spells.yml

@@ -3,7 +3,6 @@
   name: action-name-spell-forcewall
   description: action-description-spell-forcewall
   useDelay: 10
-  speech: action-speech-spell-forcewall
   itemIconStyle: BigAction
   sound: !type:SoundPathSpecifier
     path: /Audio/Magic/forcewall.ogg
@@ -13,3 +12,4 @@
   serverEvent: !type:InstantSpawnSpellEvent
     prototype: WallForce
     posData: !type:TargetInFront
+    speech: action-speech-spell-forcewall

Resources/Prototypes/Magic/knock_spell.yml

@@ -3,9 +3,9 @@
   name: action-name-spell-knock
   description: action-description-spell-knock
   useDelay: 10
-  speech: action-speech-spell-knock
   itemIconStyle: BigAction
   icon:
     sprite: Objects/Magic/magicactions.rsi
     state: knock
   serverEvent: !type:KnockSpellEvent
+    speech: action-speech-spell-knock

Resources/Prototypes/Magic/projectile_spells.yml

@@ -3,7 +3,6 @@
   name: action-name-spell-fireball
   description: action-description-spell-fireball
   useDelay: 30
-  speech: action-speech-spell-fireball
   itemIconStyle: BigAction
   checkCanAccess: false
   range: 60
@@ -15,3 +14,4 @@
   serverEvent: !type:ProjectileSpellEvent
     prototype: ProjectileFireball
     posData: !type:TargetCasterPos
+    speech: action-speech-spell-fireball

Resources/Prototypes/Magic/smite_spells.yml

@@ -3,7 +3,6 @@
   name: action-name-spell-smite
   description: action-description-spell-smite
   useDelay: 60
-  speech: action-speech-spell-smite
   itemIconStyle: BigAction
   whitelist:
     components:
@@ -16,3 +15,4 @@
     sprite: Objects/Magic/magicactions.rsi
     state: gib
   serverEvent: !type:SmiteSpellEvent
+    speech: action-speech-spell-smite

Resources/Prototypes/Magic/spawn_spells.yml

@@ -4,7 +4,6 @@
   description: action-description-spell-summon-magicarp
   useDelay: 10
   range: 4
-  speech: action-speech-spell-summon-magicarp
   itemIconStyle: BigAction
   icon:
     sprite: Objects/Magic/magicactions.rsi
@@ -14,3 +13,4 @@
       - id: MobCarpMagic
         amount: 3
     offsetVector2: 0, 1
+    speech: action-speech-spell-summon-magicarp