Fix combat action prediction (#19152)

* Fix action predictions

* Add ActionsAddedTest

Content.Client/Actions/ActionsSystem.cs

@@ -126,9 +126,6 @@ namespace Content.Client.Actions
 
         public override void AddAction(EntityUid uid, ActionType action, EntityUid? provider, ActionsComponent? comp = null, bool dirty = true)
         {
-            if (GameTiming.ApplyingState && !action.ClientExclusive)
-                return;
-
             if (!Resolve(uid, ref comp, false))
                 return;
 

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

@@ -390,7 +390,7 @@ public sealed class ActionUIController : UIController, IOnStateChanged<GameplayS
 
     private void OnActionAdded(ActionType action)
     {
-        // if the action is toggled when we add it, start targetting
+        // if the action is toggled when we add it, start targeting
         if (action is TargetedAction targetAction && action.Toggled)
             StartTargeting(targetAction);
 

Content.IntegrationTests/Tests/Actions/ActionsAddedTest.cs

@@ -0,0 +1,64 @@
+using System.Linq;
+using Content.Shared.Actions;
+using Content.Shared.Actions.ActionTypes;
+using Content.Shared.CombatMode;
+using Robust.Server.Player;
+using Robust.Shared.GameObjects;
+
+namespace Content.IntegrationTests.Tests.Actions;
+
+/// <summary>
+/// This tests checks that actions properly get added to an entity's actions component..
+/// </summary>
+[TestFixture]
+public sealed class ActionsAddedTest
+{
+    // TODO add magboot test (inventory action)
+    // TODO add ghost toggle-fov test (client-side action)
+
+    [Test]
+    public async Task TestCombatActionsAdded()
+    {
+        await using var pair = await PoolManager.GetServerClient(new PoolSettings { Connected = true, DummyTicker = false});
+        var server = pair.Server;
+        var client = pair.Client;
+        var sEntMan = server.ResolveDependency<IEntityManager>();
+        var cEntMan = client.ResolveDependency<IEntityManager>();
+        var session = server.ResolveDependency<IPlayerManager>().ServerSessions.Single();
+
+        // Dummy ticker is disabled - client should be in control of a normal mob.
+        Assert.NotNull(session.AttachedEntity);
+        var ent = session.AttachedEntity!.Value;
+        Assert.That(sEntMan.EntityExists(ent));
+        Assert.That(cEntMan.EntityExists(ent));
+        Assert.That(sEntMan.HasComponent<ActionsComponent>(ent));
+        Assert.That(cEntMan.HasComponent<ActionsComponent>(ent));
+        Assert.That(sEntMan.HasComponent<CombatModeComponent>(ent));
+        Assert.That(cEntMan.HasComponent<CombatModeComponent>(ent));
+
+        var sComp = sEntMan.GetComponent<ActionsComponent>(ent);
+        var cComp = cEntMan.GetComponent<ActionsComponent>(ent);
+
+        // Mob should have a combat-mode action.
+        // This action should have a non-null event both on the server & client.
+        var evType = typeof(ToggleCombatActionEvent);
+
+        var sActions = sComp!.Actions.Where(
+            x => x is InstantAction act && act.Event?.GetType() == evType).ToArray();
+        var cActions = cComp!.Actions.Where(
+            x => x is InstantAction act && act.Event?.GetType() == evType).ToArray();
+
+        Assert.That(sActions.Length, Is.EqualTo(1));
+        Assert.That(cActions.Length, Is.EqualTo(1));
+
+        var sAct = (InstantAction) sActions[0];
+        var cAct = (InstantAction) cActions[0];
+
+        // Finally, these two actions are not the same object
+        // required, because integration tests do not respect the [NonSerialized] attribute and will simply events by reference.
+        Assert.That(ReferenceEquals(sAct, cAct), Is.False);
+        Assert.That(ReferenceEquals(sAct.Event, cAct.Event), Is.False);
+
+        await pair.CleanReturnAsync();
+    }
+}

Content.Shared/Actions/ActionTypes/ActionType.cs

@@ -249,6 +249,19 @@ public abstract class ActionType : IEquatable<ActionType>, IComparable, ICloneab
         return CompareTo(other) == 0;
     }
 
+    public static bool operator ==(ActionType? left, ActionType? right)
+    {
+        if (left is null)
+            return right is null;
+
+        return left.Equals(right);
+    }
+
+    public static bool operator !=(ActionType? left, ActionType? right)
+    {
+        return !(left == right);
+    }
+
     public override int GetHashCode()
     {
         unchecked

Content.Shared/CombatMode/SharedCombatModeSystem.cs

@@ -2,6 +2,7 @@ using Content.Shared.Actions;
 using Content.Shared.Actions.ActionTypes;
 using Content.Shared.Popups;
 using Content.Shared.Targeting;
+using Robust.Shared.Network;
 using Robust.Shared.Prototypes;
 using Robust.Shared.Serialization;
 using Robust.Shared.Timing;
@@ -14,6 +15,7 @@ public abstract class SharedCombatModeSystem : EntitySystem
     [Dependency] private readonly SharedActionsSystem _actionsSystem = default!;
     [Dependency] private readonly SharedPopupSystem _popup = default!;
     [Dependency] private readonly IGameTiming _timing = default!;
+    [Dependency] private readonly INetManager _netMan = default!;
 
     public override void Initialize()
     {
@@ -50,7 +52,10 @@ public abstract class SharedCombatModeSystem : EntitySystem
         args.Handled = true;
         SetInCombatMode(uid, !component.IsInCombatMode, component);
 
-        if (!_timing.IsFirstTimePredicted)
+        // TODO better handling of predicted pop-ups.
+        // This probably breaks if the client has prediction disabled.
+
+        if (!_netMan.IsClient || !_timing.IsFirstTimePredicted)
             return;
 
         var msg = component.IsInCombatMode ? "action-popup-combat-enabled" : "action-popup-combat-disabled";