Apply monster attribute changes instantly on the game server

src/GameLogic/Attributes/MonsterAttributeHolder.cs

@@ -35,10 +35,10 @@ public class MonsterAttributeHolder : IAttributeSystem
 
     private readonly AttackableNpcBase _monster;
 
-    private readonly IDictionary<AttributeDefinition, float> _statAttributes;
-
     private readonly object _attributesLock = new();
 
+    private IDictionary<AttributeDefinition, float> _statAttributes;
+
     /// <summary>
     /// Attribute dictionary of a monster instance.
     /// Most monster instances don't have additional attributes, so we just instantiate one if needed.
@@ -158,11 +158,36 @@ public IElement GetOrCreateAttribute(AttributeDefinition attributeDefinition)
         throw new NotImplementedException();
     }
 
+    /// <summary>
+    /// Reloads the stat attributes from the (possibly changed) <see cref="MonsterDefinition"/>,
+    /// so that configuration changes take effect on the running game server.
+    /// </summary>
+    public void ApplyChanges()
+    {
+        // When many instances of the same monster are spawned, all of them get notified about the
+        // same change. The first one rebuilds the shared cache; the others just adopt the new one.
+        if (MonsterStatAttributesCache.TryGetValue(this._monster.Definition, out var cached)
+            && !ReferenceEquals(cached, this._statAttributes))
+        {
+            this._statAttributes = cached;
+            return;
+        }
+
+        var statAttributes = BuildStatAttributes(this._monster.Definition);
+        MonsterStatAttributesCache[this._monster.Definition] = statAttributes;
+        this._statAttributes = statAttributes;
+    }
+
     private static IDictionary<AttributeDefinition, float> GetStatAttributeOfMonster(MonsterDefinition monsterDefinition)
     {
-        return MonsterStatAttributesCache.GetOrAdd(monsterDefinition, monsterDef => monsterDef.Attributes.ToDictionary(
-                m => m.AttributeDefinition ?? throw Error.NotInitializedProperty(m, nameof(m.AttributeDefinition)),
-                m => m.Value));
+        return MonsterStatAttributesCache.GetOrAdd(monsterDefinition, BuildStatAttributes);
+    }
+
+    private static IDictionary<AttributeDefinition, float> BuildStatAttributes(MonsterDefinition monsterDefinition)
+    {
+        return monsterDefinition.Attributes.ToDictionary(
+            m => m.AttributeDefinition ?? throw Error.NotInitializedProperty(m, nameof(m.AttributeDefinition)),
+            m => m.Value);
     }
 
     private IDictionary<AttributeDefinition, IComposableAttribute> GetAttributeDictionary()

src/GameLogic/MapInitializer.cs

@@ -270,6 +270,22 @@ protected virtual GameMap InternalCreateGameMap(GameMapDefinition definition)
 
     private void RegisterForConfigChanges(GameMap createdMap, MonsterSpawnArea spawnArea, NonPlayerCharacter spawnedObject)
     {
+        // Apply changes of the monster definition (e.g. its attributes) instantly to the
+        // already spawned instance, without having to re-spawn it. The registration is owned
+        // by the NPC, so it gets disposed whenever the NPC is disposed.
+        if (spawnedObject is AttackableNpcBase attackableNpc
+            && this._configurationChangeMediator?.RegisterObject(
+                spawnedObject.Definition,
+                attackableNpc,
+                (_, _, o) =>
+                {
+                    o.ReloadAttributes();
+                    return ValueTask.CompletedTask;
+                }) is { } definitionRegistration)
+        {
+            attackableNpc.RegisterDisposable(definitionRegistration);
+        }
+
         this._configurationChangeMediator?.RegisterObject(
             spawnArea,
             spawnedObject,

src/GameLogic/NPC/AttackableNpcBase.cs

@@ -24,6 +24,7 @@ public abstract class AttackableNpcBase : NonPlayerCharacter, IAttackable
     private readonly IEventStateProvider? _eventStateProvider;
     private readonly IDropGenerator _dropGenerator;
     private readonly PlugInManager _plugInManager;
+    private readonly List<IDisposable> _registrations = new();
 
     private int _health;
 
@@ -161,13 +162,38 @@ public override void Initialize()
         this.IsAlive = true;
     }
 
+    /// <summary>
+    /// Reloads the attributes from the <see cref="NonPlayerCharacter.Definition"/>, so that changes
+    /// to the monster definition take effect on this already spawned instance.
+    /// </summary>
+    public void ReloadAttributes()
+    {
+        (this.Attributes as MonsterAttributeHolder)?.ApplyChanges();
+    }
+
+    /// <summary>
+    /// Registers a disposable (e.g. a configuration change registration) to be disposed
+    /// together with this instance.
+    /// </summary>
+    /// <param name="disposable">The disposable.</param>
+    public void RegisterDisposable(IDisposable disposable)
+    {
+        this._registrations.Add(disposable);
+    }
+
     /// <inheritdoc/>
     protected override void Dispose(bool managed)
     {
         if (managed)
         {
             this.Died = null;
             this.IsAlive = false;
+            foreach (var registration in this._registrations)
+            {
+                registration.Dispose();
+            }
+
+            this._registrations.Clear();
         }
 
         base.Dispose(managed);

tests/MUnique.OpenMU.Tests/MonsterAttributeReloadTests.cs

@@ -0,0 +1,132 @@
+// <copyright file="MonsterAttributeReloadTests.cs" company="MUnique">
+// Licensed under the MIT License. See LICENSE file in the project root for full license information.
+// </copyright>
+
+namespace MUnique.OpenMU.Tests;
+
+using Moq;
+using MUnique.OpenMU.AttributeSystem;
+using MUnique.OpenMU.DataModel.Configuration;
+using MUnique.OpenMU.GameLogic;
+using MUnique.OpenMU.GameLogic.Attributes;
+using MUnique.OpenMU.GameLogic.NPC;
+using MUnique.OpenMU.Pathfinding;
+using MonsterDefinition = MUnique.OpenMU.Persistence.BasicModel.MonsterDefinition;
+using MonsterAttribute = MUnique.OpenMU.Persistence.BasicModel.MonsterAttribute;
+
+/// <summary>
+/// Tests for applying changes of a <see cref="MonsterDefinition"/> to an already spawned
+/// <see cref="AttackableNpcBase"/> via <see cref="AttackableNpcBase.ReloadAttributes"/>.
+/// </summary>
+[TestFixture]
+public class MonsterAttributeReloadTests
+{
+    private IGameContext _gameContext = null!;
+
+    /// <summary>
+    /// Sets up a fresh game context before each test.
+    /// </summary>
+    [SetUp]
+    public void SetUp()
+    {
+        this._gameContext = GameContextTestHelper.CreateGameContext();
+    }
+
+    /// <summary>
+    /// Tests that changing a value of a <see cref="MonsterAttribute"/> takes effect on an
+    /// already spawned monster after <see cref="AttackableNpcBase.ReloadAttributes"/> is called.
+    /// </summary>
+    [Test]
+    public async ValueTask ReloadAttributesAppliesChangedValueAsync()
+    {
+        var monster = await this.CreateMonsterAsync().ConfigureAwait(false);
+        var maximumHealthAttribute = monster.Definition.Attributes.First(a => a.AttributeDefinition == Stats.MaximumHealth);
+
+        Assert.That(monster.Attributes[Stats.MaximumHealth], Is.EqualTo(1000));
+
+        maximumHealthAttribute.Value = 2000;
+        monster.ReloadAttributes();
+
+        Assert.That(monster.Attributes[Stats.MaximumHealth], Is.EqualTo(2000));
+    }
+
+    /// <summary>
+    /// Tests that adding a new <see cref="MonsterAttribute"/> takes effect on an already spawned
+    /// monster after <see cref="AttackableNpcBase.ReloadAttributes"/> is called.
+    /// </summary>
+    [Test]
+    public async ValueTask ReloadAttributesAppliesAddedAttributeAsync()
+    {
+        var monster = await this.CreateMonsterAsync().ConfigureAwait(false);
+
+        Assert.That(monster.Attributes[Stats.AttackRatePvm], Is.EqualTo(0));
+
+        monster.Definition.Attributes.Add(new MonsterAttribute { AttributeDefinition = Stats.AttackRatePvm, Value = 50 });
+        monster.ReloadAttributes();
+
+        Assert.That(monster.Attributes[Stats.AttackRatePvm], Is.EqualTo(50));
+    }
+
+    /// <summary>
+    /// Tests that all spawned instances of the same monster definition pick up an attribute change.
+    /// </summary>
+    [Test]
+    public async ValueTask ReloadAttributesAppliesToAllInstancesOfDefinitionAsync()
+    {
+        var monsterDefinition = CreateMonsterDefinition();
+        var monster1 = await this.CreateMonsterAsync(monsterDefinition).ConfigureAwait(false);
+        var monster2 = await this.CreateMonsterAsync(monsterDefinition).ConfigureAwait(false);
+        var maximumHealthAttribute = monsterDefinition.Attributes.First(a => a.AttributeDefinition == Stats.MaximumHealth);
+
+        maximumHealthAttribute.Value = 2000;
+        monster1.ReloadAttributes();
+        monster2.ReloadAttributes();
+
+        Assert.That(monster1.Attributes[Stats.MaximumHealth], Is.EqualTo(2000));
+        Assert.That(monster2.Attributes[Stats.MaximumHealth], Is.EqualTo(2000));
+    }
+
+    private static MonsterDefinition CreateMonsterDefinition()
+    {
+        var monsterDefinition = new MonsterDefinition
+        {
+            ObjectKind = NpcObjectKind.Monster,
+        };
+        monsterDefinition.Attributes.Add(new MonsterAttribute { AttributeDefinition = Stats.MaximumHealth, Value = 1000 });
+        monsterDefinition.Attributes.Add(new MonsterAttribute { AttributeDefinition = Stats.DefenseBase, Value = 100 });
+        return monsterDefinition;
+    }
+
+    private ValueTask<Monster> CreateMonsterAsync()
+    {
+        return this.CreateMonsterAsync(CreateMonsterDefinition());
+    }
+
+    private async ValueTask<Monster> CreateMonsterAsync(MonsterDefinition monsterDefinition)
+    {
+        var map = await this._gameContext.GetMapAsync(0).ConfigureAwait(false);
+        var spawnArea = new MonsterSpawnArea
+        {
+            MonsterDefinition = monsterDefinition,
+            GameMap = map!.Definition,
+            X1 = 100,
+            Y1 = 100,
+            X2 = 100,
+            Y2 = 100,
+            Quantity = 1,
+        };
+
+        var monster = new Monster(
+            spawnArea,
+            monsterDefinition,
+            map,
+            NullDropGenerator.Instance,
+            new Mock<INpcIntelligence>().Object,
+            this._gameContext.PlugInManager,
+            this._gameContext.PathFinderPool);
+
+        monster.Initialize();
+
+        return monster;
+    }
+}