Merge pull request #836 from nolt/speedhack-exempt-offline-players

Don't check offline players for speedhacking

(cherry picked from commit 6380b269770f0c949b03794ca701876c13636513)

src/GameLogic/PlugIns/SpeedHackDetectPlugIn.cs

@@ -36,7 +36,7 @@ public class SpeedHackDetectPlugIn : IFeaturePlugIn, ISupportCustomConfiguration
     /// <inheritdoc/>
     public async ValueTask WalkCheatCheckAsync(Player player, Memory<WalkingStep> steps, SpeedHackCheckEventArgs eventArgs)
     {
-        if (steps.IsEmpty)
+        if (steps.IsEmpty || IsServerControlled(player))
         {
             return;
         }
@@ -139,7 +139,7 @@ public class SpeedHackDetectPlugIn : IFeaturePlugIn, ISupportCustomConfiguration
     /// <inheritdoc/>
     public async ValueTask AttackCheatCheckAsync(Player player, SpeedHackCheckEventArgs eventArgs)
     {
-        if (player.Attributes is not { } attributes)
+        if (player.Attributes is not { } attributes || IsServerControlled(player))
         {
             return;
         }
@@ -229,6 +229,19 @@ public class SpeedHackDetectPlugIn : IFeaturePlugIn, ISupportCustomConfiguration
         }
     }
 
+    /// <summary>
+    /// Determines whether the player's actions originate on the server instead of a game client.
+    /// These checks validate what a client claims about its own timing, so there is nothing to
+    /// validate for an offline player: the server itself paces its walks and attacks. Its MU Helper
+    /// tick performs the same work cycle as the original client helper (recover, then attack), which
+    /// draws two attack tokens from one 500 ms tick and eventually empties the bucket - so leaving
+    /// these players in would only ever produce false positives, and with the default configuration
+    /// those are answered with a persisted account ban.
+    /// </summary>
+    /// <param name="player">The player.</param>
+    /// <returns><c>true</c> if the player is controlled by the server; otherwise, <c>false</c>.</returns>
+    private static bool IsServerControlled(Player player) => player is Offline.OfflinePlayer;
+
     private SpeedHackState GetState(Player player)
     {
         return this._playerStates.GetValue(player, p => new SpeedHackState(this.Configuration?.MaxAttackTokens ?? 5.0));

tests/MUnique.OpenMU.Tests/SpeedHackAntiCheatTests.cs

@@ -16,6 +16,7 @@ using MUnique.OpenMU.DataModel.Configuration.Items;
 using MUnique.OpenMU.DataModel.Entities;
 using MUnique.OpenMU.GameLogic;
 using MUnique.OpenMU.GameLogic.Attributes;
+using MUnique.OpenMU.GameLogic.Offline;
 using MUnique.OpenMU.GameLogic.Views;
 using MUnique.OpenMU.GameLogic.Views.World;
 using MUnique.OpenMU.Pathfinding;
@@ -389,6 +390,70 @@ public class SpeedHackAntiCheatTests
         Assert.That(player.GameContext.FeaturePlugIns.GetPlugIn<SpeedHackDetectPlugIn>()!.GetWarningCount(player), Is.GreaterThan(5));
     }
 
+    /// <summary>
+    /// Tests that an offline player - whose walks are issued by the server itself - is not checked at all.
+    /// </summary>
+    [Test]
+    public async Task TestOfflinePlayerIsNotCheckedOnWalkAsync()
+    {
+        var player = await CreatePlayerWithSpeedAttributesAsync().ConfigureAwait(false);
+        var offlinePlayer = await CreateOfflinePlayerAsync(player).ConfigureAwait(false);
+        var plugin = player.GameContext.FeaturePlugIns.GetPlugIn<SpeedHackDetectPlugIn>()!;
+
+        // The same walk pattern which bans a regular player in TestWalkSpeedHackDetectionBansAccountAsync.
+        for (int i = 0; i < 12; i++)
+        {
+            var nextFrom = new Point((byte)(StartPoint.X + (i * 2)), StartPoint.Y);
+            var nextTo = new Point((byte)(StartPoint.X + (i * 2) + 2), StartPoint.Y);
+
+            offlinePlayer.Position = nextFrom;
+            plugin.SetLastAlertTime(offlinePlayer, DateTime.MinValue);
+
+            WalkingStep[] steps = [new() { From = nextFrom, To = nextTo, Direction = Direction.East }];
+
+            await offlinePlayer.WalkToAsync(nextTo, steps).ConfigureAwait(false);
+        }
+
+        Assert.That(plugin.GetWarningCount(offlinePlayer), Is.EqualTo(0));
+        Assert.That(offlinePlayer.Account?.State, Is.EqualTo(AccountState.Normal));
+    }
+
+    /// <summary>
+    /// Tests that an offline player is not checked on attacks. Its MU Helper tick performs the same work
+    /// cycle as the original client helper (recover, then attack), which draws more than one token from a
+    /// single tick and would eventually empty the bucket.
+    /// </summary>
+    [Test]
+    public async Task TestOfflinePlayerIsNotCheckedOnAttackAsync()
+    {
+        var player = await CreatePlayerWithSpeedAttributesAsync().ConfigureAwait(false);
+        var offlinePlayer = await CreateOfflinePlayerAsync(player).ConfigureAwait(false);
+        offlinePlayer.Attributes![Stats.AttackSpeed] = 20;
+
+        var speedCheck = player.GameContext.PlugInManager.GetPlugInPoint<ISpeedHackCheatCheckPlugIn>()!;
+
+        // The same burst which is detected for a regular player in TestAttackSpeedHackDetectionAsync.
+        for (int i = 0; i < 20; i++)
+        {
+            var args = new SpeedHackCheckEventArgs();
+            await speedCheck.AttackCheatCheckAsync(offlinePlayer, args).ConfigureAwait(false);
+            Assert.That(args.IsCheatDetected, Is.False);
+        }
+
+        Assert.That(player.GameContext.FeaturePlugIns.GetPlugIn<SpeedHackDetectPlugIn>()!.GetWarningCount(offlinePlayer), Is.EqualTo(0));
+        Assert.That(offlinePlayer.Account?.State, Is.EqualTo(AccountState.Normal));
+    }
+
+    private static async ValueTask<OfflinePlayer> CreateOfflinePlayerAsync(Player regularPlayer)
+    {
+        var offlinePlayer = new OfflinePlayer(regularPlayer.GameContext) { Account = regularPlayer.Account };
+        await offlinePlayer.PlayerState.TryAdvanceToAsync(PlayerState.LoginScreen).ConfigureAwait(false);
+        await offlinePlayer.PlayerState.TryAdvanceToAsync(PlayerState.Authenticated).ConfigureAwait(false);
+        await offlinePlayer.PlayerState.TryAdvanceToAsync(PlayerState.CharacterSelection).ConfigureAwait(false);
+        await offlinePlayer.SetSelectedCharacterAsync(regularPlayer.SelectedCharacter!).ConfigureAwait(false);
+        return offlinePlayer;
+    }
+
     private static async ValueTask<Player> CreatePlayerWithSpeedAttributesAsync(List<Item>? inventoryItems = null)
     {
         var gameConfig = new Mock<GameConfiguration>();