fix(exp): never allow level overflowing over maximum

src/GameLogic/Player.cs

@@ -50,6 +50,7 @@ public class Player : AsyncDisposable, IBucketMapObserver, IAttackable, IAttacke
     };
 
     private readonly AsyncLock _moveLock = new();
+    private readonly AsyncLock _experienceLock = new();
 
     private readonly Walker _walker;
 
@@ -1214,6 +1215,12 @@ public async ValueTask<int> AddExpAfterKillAsync(IAttackable killedObject)
     /// <param name="experience">The experience which should be added.</param>
     /// <param name="killedObject">The killed object which caused the experience gain.</param>
     public async ValueTask AddMasterExperienceAsync(int experience, IAttackable? killedObject)
+    {
+        using var d = await this._experienceLock.LockAsync().ConfigureAwait(false);
+        await this.AddMasterExperienceCoreAsync(experience, killedObject).ConfigureAwait(false);
+    }
+
+    private async ValueTask AddMasterExperienceCoreAsync(int experience, IAttackable? killedObject)
     {
         if (this.Attributes![Stats.MasterLevel] >= this.GameContext.Configuration.MaximumMasterLevel)
         {
@@ -1262,30 +1269,41 @@ public async ValueTask AddMasterExperienceAsync(int experience, IAttackable? kil
     /// <param name="killedObject">The killed object which caused the experience gain.</param>
     public async ValueTask AddExperienceAsync(int experience, IAttackable? killedObject)
     {
-        if (this.Attributes![Stats.Level] >= this.GameContext.Configuration.MaximumLevel)
-        {
-            await this.InvokeViewPlugInAsync<IAddExperiencePlugIn>(p => p.AddExperienceAsync(0, killedObject, ExperienceType.MaxLevelReached)).ConfigureAwait(false);
-            return;
-        }
+        using var d = await this._experienceLock.LockAsync().ConfigureAwait(false);
+        await this.AddExperienceCoreAsync(experience, killedObject).ConfigureAwait(false);
+    }
 
-        long exp = experience;
-        bool isLevelUp = false;
-        var expTable = this.GameContext.ExperienceTable;
-        var expForNextLevel = expTable[(int)this.Attributes[Stats.Level] + 1];
-        if (expForNextLevel - this.SelectedCharacter!.Experience < exp)
+    private async ValueTask AddExperienceCoreAsync(int experience, IAttackable? killedObject)
+    {
+        var remainingExperience = experience;
+        while (remainingExperience > 0)
         {
-            exp = expForNextLevel - this.SelectedCharacter.Experience;
-            isLevelUp = true;
-        }
+            if (this.Attributes![Stats.Level] >= this.GameContext.Configuration.MaximumLevel)
+            {
+                await this.InvokeViewPlugInAsync<IAddExperiencePlugIn>(p => p.AddExperienceAsync(0, killedObject, ExperienceType.MaxLevelReached)).ConfigureAwait(false);
+                return;
+            }
 
-        this.SelectedCharacter.Experience += exp;
+            long gainedExperience = remainingExperience;
+            bool isLevelUp = false;
+            var expTable = this.GameContext.ExperienceTable;
+            var expForNextLevel = expTable[(int)this.Attributes[Stats.Level] + 1];
+            if (expForNextLevel - this.SelectedCharacter!.Experience < gainedExperience)
+            {
+                gainedExperience = expForNextLevel - this.SelectedCharacter.Experience;
+                isLevelUp = true;
+            }
 
-        // Tell it to the Player
-        await this.InvokeViewPlugInAsync<IAddExperiencePlugIn>(p => p.AddExperienceAsync((int)exp, killedObject, ExperienceType.Normal)).ConfigureAwait(false);
+            this.SelectedCharacter.Experience += gainedExperience;
+
+            // Tell it to the Player
+            await this.InvokeViewPlugInAsync<IAddExperiencePlugIn>(p => p.AddExperienceAsync((int)gainedExperience, killedObject, ExperienceType.Normal)).ConfigureAwait(false);
+
+            if (!isLevelUp)
+            {
+                return;
+            }
 
-        // Check the lvl up
-        if (isLevelUp)
-        {
             this.Attributes[Stats.Level]++;
             this.SelectedCharacter.LevelUpPoints += (int)this.Attributes[Stats.PointsPerLevelUp];
             this.SetReclaimableAttributesToMaximum();
@@ -1296,14 +1314,12 @@ public async ValueTask AddExperienceAsync(int experience, IAttackable? killedObj
             await this.InvokeViewPlugInAsync<IUpdateLevelPlugIn>(p => p.UpdateLevelAsync()).ConfigureAwait(false);
             await this.ForEachWorldObserverAsync<IShowEffectPlugIn>(p => p.ShowEffectAsync(this, IShowEffectPlugIn.EffectType.LevelUp), true).ConfigureAwait(false);
 
-            var remainingExp = experience - exp;
-            if (remainingExp > 0 && this.Attributes![Stats.Level] < this.GameContext.Configuration.MaximumLevel)
+            remainingExperience -= (int)gainedExperience;
+            if (remainingExperience <= 0
+                || this.Attributes[Stats.Level] >= this.GameContext.Configuration.MaximumLevel
+                || this.GameContext.Configuration.PreventExperienceOverflow)
             {
-                // Only apply overflow if the configuration allows it
-                if (!this.GameContext.Configuration.PreventExperienceOverflow)
-                {
-                    await this.AddExperienceAsync((int)remainingExp, killedObject).ConfigureAwait(false);
-                }
+                return;
             }
         }
     }
@@ -2848,4 +2864,4 @@ public void RaiseAppearanceChanged()
             this.AppearanceChanged?.Invoke(this, EventArgs.Empty);
         }
     }
-}
+}

tests/MUnique.OpenMU.Tests/ExperienceRateSplitTest.cs

@@ -113,6 +113,107 @@ public async ValueTask PartyDistributionUsesMasterExperienceRateForMasterMembers
         Assert.That(masterGained, Is.GreaterThan(normalGained * 3));
     }
 
+    [Test]
+    public async ValueTask ConcurrentNormalExperienceCantExceedMaximumLevelAsync()
+    {
+        var context = this.CreateGameServerContext(
+            normalExperienceRate: 1.0f,
+            globalMasterExperienceRate: 1.0f,
+            maximumLevel: 2,
+            maximumMasterLevel: 200);
+
+        var player = await this.CreatePlayerAsync(context, level: 1, totalLevel: 1, isMasterClass: false).ConfigureAwait(false);
+        player.SelectedCharacter!.Experience = context.ExperienceTable[2] - 1;
+
+        var initialLevelUpPoints = player.SelectedCharacter.LevelUpPoints;
+        var pointsPerLevelUp = (int)player.Attributes![Stats.PointsPerLevelUp];
+
+        await Task.WhenAll(
+            player.AddExperienceAsync(10, null).AsTask(),
+            player.AddExperienceAsync(10, null).AsTask()).ConfigureAwait(false);
+
+        Assert.That((int)player.Attributes[Stats.Level], Is.EqualTo(2));
+        Assert.That(player.SelectedCharacter.LevelUpPoints, Is.EqualTo(initialLevelUpPoints + pointsPerLevelUp));
+    }
+
+    [Test]
+    public async ValueTask ConcurrentMasterExperienceStaysWithinConfiguredMaximumBoundsAsync()
+    {
+        var context = this.CreateGameServerContext(
+            normalExperienceRate: 1.0f,
+            globalMasterExperienceRate: 1.0f,
+            maximumLevel: 400,
+            maximumMasterLevel: 1);
+
+        var player = await this.CreatePlayerAsync(context, level: 400, totalLevel: 400, isMasterClass: true).ConfigureAwait(false);
+        player.Attributes![Stats.MasterLevel] = 0;
+        player.SelectedCharacter!.MasterExperience = context.MasterExperienceTable[1] - 1;
+        var maxMasterExperience = context.MasterExperienceTable[context.Configuration.MaximumMasterLevel];
+
+        await Task.WhenAll(
+            player.AddMasterExperienceAsync(10, null).AsTask(),
+            player.AddMasterExperienceAsync(10, null).AsTask()).ConfigureAwait(false);
+
+        Assert.That((int)player.Attributes[Stats.MasterLevel], Is.LessThanOrEqualTo(context.Configuration.MaximumMasterLevel));
+        Assert.That(player.SelectedCharacter.MasterExperience, Is.LessThanOrEqualTo(maxMasterExperience));
+    }
+
+    [Test]
+    public async ValueTask OverflowIsAppliedBelowMaxWhenNotPreventedAsync()
+    {
+        var context = this.CreateGameServerContext(
+            normalExperienceRate: 1.0f,
+            globalMasterExperienceRate: 1.0f,
+            maximumLevel: 10,
+            maximumMasterLevel: 200);
+
+        var player = await this.CreatePlayerAsync(context, level: 1, totalLevel: 1, isMasterClass: false).ConfigureAwait(false);
+        var requiredForLevel2 = context.ExperienceTable[2] - player.SelectedCharacter!.Experience;
+
+        await player.AddExperienceAsync((int)requiredForLevel2 + 10, null).ConfigureAwait(false);
+
+        Assert.That((int)player.Attributes![Stats.Level], Is.EqualTo(2));
+        Assert.That(player.SelectedCharacter.Experience, Is.EqualTo(context.ExperienceTable[2] + 10));
+    }
+
+    [Test]
+    public async ValueTask OverflowIsDiscardedBelowMaxWhenPreventedAsync()
+    {
+        var context = this.CreateGameServerContext(
+            normalExperienceRate: 1.0f,
+            globalMasterExperienceRate: 1.0f,
+            maximumLevel: 10,
+            maximumMasterLevel: 200,
+            preventExperienceOverflow: true);
+
+        var player = await this.CreatePlayerAsync(context, level: 1, totalLevel: 1, isMasterClass: false).ConfigureAwait(false);
+        var requiredForLevel2 = context.ExperienceTable[2] - player.SelectedCharacter!.Experience;
+
+        await player.AddExperienceAsync((int)requiredForLevel2 + 10, null).ConfigureAwait(false);
+
+        Assert.That((int)player.Attributes![Stats.Level], Is.EqualTo(2));
+        Assert.That(player.SelectedCharacter.Experience, Is.EqualTo(context.ExperienceTable[2]));
+    }
+
+    [TestCase(false)]
+    [TestCase(true)]
+    public async ValueTask ExperienceAlwaysStopsAtMaximumLevelRegardlessOfOverflowSettingAsync(bool preventExperienceOverflow)
+    {
+        var context = this.CreateGameServerContext(
+            normalExperienceRate: 1.0f,
+            globalMasterExperienceRate: 1.0f,
+            maximumLevel: 2,
+            maximumMasterLevel: 200,
+            preventExperienceOverflow);
+
+        var player = await this.CreatePlayerAsync(context, level: 1, totalLevel: 1, isMasterClass: false).ConfigureAwait(false);
+
+        await player.AddExperienceAsync(int.MaxValue, null).ConfigureAwait(false);
+        await player.AddExperienceAsync(int.MaxValue, null).ConfigureAwait(false);
+
+        Assert.That((int)player.Attributes![Stats.Level], Is.EqualTo(2));
+    }
+
     private static Mock<IAttackable> CreateKilledObject(float level)
     {
         var attributes = new Mock<IAttributeSystem>();
@@ -129,6 +230,9 @@ private async ValueTask<Player> CreatePlayerAsync(IGameContext context, short le
         var player = await PlayerTestHelper.CreatePlayerAsync(context).ConfigureAwait(false);
         player.SelectedCharacter!.CharacterClass!.IsMasterClass = isMasterClass;
         player.Attributes![Stats.Level] = level;
+        player.Attributes[Stats.MasterLevel] = 0;
+        player.Attributes[Stats.PointsPerLevelUp] = 1;
+        player.Attributes[Stats.MasterPointsPerLevelUp] = 1;
         player.Attributes.AddElement(new SimpleElement(1.0f, AggregateType.AddRaw), Stats.ExperienceRate);
         player.Attributes.AddElement(new SimpleElement(1.0f, AggregateType.AddRaw), Stats.MasterExperienceRate);
         player.Attributes.AddElement(new SimpleElement(totalLevel, AggregateType.AddRaw), Stats.TotalLevel);
@@ -137,13 +241,19 @@ private async ValueTask<Player> CreatePlayerAsync(IGameContext context, short le
         return player;
     }
 
-    private IGameServerContext CreateGameServerContext(float normalExperienceRate, float globalMasterExperienceRate, short maximumLevel, short maximumMasterLevel)
+    private IGameServerContext CreateGameServerContext(float normalExperienceRate, float globalMasterExperienceRate, short maximumLevel, short maximumMasterLevel, bool preventExperienceOverflow = false)
     {
         var contextProvider = new InMemoryPersistenceContextProvider();
         var gameConfiguration = contextProvider.CreateNewContext().CreateNew<GameConfiguration>();
+        if (gameConfiguration.CharacterClasses is null)
+        {
+            typeof(GameConfiguration).GetProperty(nameof(GameConfiguration.CharacterClasses))?.SetValue(gameConfiguration, new List<CharacterClass>());
+        }
+
         gameConfiguration.RecoveryInterval = int.MaxValue;
         gameConfiguration.MaximumLevel = maximumLevel;
         gameConfiguration.MaximumMasterLevel = maximumMasterLevel;
+        gameConfiguration.PreventExperienceOverflow = preventExperienceOverflow;
         gameConfiguration.MinimumMonsterLevelForMasterExperience = 0;
         gameConfiguration.ExperienceRate = 1.0f;
         gameConfiguration.MasterExperienceRate = globalMasterExperienceRate;

tests/MUnique.OpenMU.Tests/PlayerTestHelper.cs

@@ -131,6 +131,7 @@ public static async ValueTask<Player> CreatePlayerAsync(IGameContext gameContext
 
         var accountMock = new Mock<Account>();
         accountMock.Setup(mock => mock.Attributes).Returns(new List<StatAttribute>());
+        accountMock.Setup(mock => mock.UnlockedCharacterClasses).Returns(new List<CharacterClass>());
         var player = new TestPlayer(gameContext) { Account = accountMock.Object };
         await player.PlayerState.TryAdvanceToAsync(PlayerState.LoginScreen).ConfigureAwait(false);
         await player.PlayerState.TryAdvanceToAsync(PlayerState.Authenticated).ConfigureAwait(false);