diff --git a/src/GameLogic/AsyncDisposable.cs b/src/GameLogic/AsyncDisposable.cs index 1ecf28503..484c58df2 100644 --- a/src/GameLogic/AsyncDisposable.cs +++ b/src/GameLogic/AsyncDisposable.cs @@ -32,8 +32,9 @@ public async ValueTask DisposeAsync() try { await this.DisposeAsyncCore().ConfigureAwait(false); - this.Dispose(true); - GC.SuppressFinalize(this); + + // Disposes the managed resources and marks this instance as disposed. + this.Dispose(); } finally { diff --git a/src/GameLogic/NPC/AttackableNpcBase.cs b/src/GameLogic/NPC/AttackableNpcBase.cs index e53820809..1828191d5 100644 --- a/src/GameLogic/NPC/AttackableNpcBase.cs +++ b/src/GameLogic/NPC/AttackableNpcBase.cs @@ -373,6 +373,12 @@ private async ValueTask RespawnAsync() { try { + if (this.IsDisposed || this.IsDisposing) + { + // It got disposed while waiting for the respawn, e.g. because its spawn area was removed. + return; + } + if (!this.ShouldRespawn) { await this.RemoveFromMapAndDisposeAsync().ConfigureAwait(false); diff --git a/tests/MUnique.OpenMU.Tests/AsyncDisposableTest.cs b/tests/MUnique.OpenMU.Tests/AsyncDisposableTest.cs new file mode 100644 index 000000000..759acb483 --- /dev/null +++ b/tests/MUnique.OpenMU.Tests/AsyncDisposableTest.cs @@ -0,0 +1,66 @@ +// +// Licensed under the MIT License. See LICENSE file in the project root for full license information. +// + +namespace MUnique.OpenMU.Tests; + +using MUnique.OpenMU.GameLogic; + +/// +/// Tests for the . +/// +[TestFixture] +public class AsyncDisposableTest +{ + /// + /// Tests that an instance is marked as disposed after . + /// + [Test] + public async Task IsDisposedAfterDisposeAsync() + { + var disposable = new TestDisposable(); + + await disposable.DisposeAsync().ConfigureAwait(false); + + Assert.That(disposable.IsDisposed, Is.True); + Assert.That(disposable.IsDisposing, Is.False); + } + + /// + /// Tests that disposing an instance a second time doesn't release its resources again. + /// + [Test] + public async Task SecondDisposeAsyncDoesNothingAsync() + { + var disposable = new TestDisposable(); + + await disposable.DisposeAsync().ConfigureAwait(false); + await disposable.DisposeAsync().ConfigureAwait(false); + + Assert.That(disposable.AsyncDisposeCount, Is.EqualTo(1)); + Assert.That(disposable.DisposeCount, Is.EqualTo(1)); + } + + private sealed class TestDisposable : AsyncDisposable + { + public int AsyncDisposeCount { get; private set; } + + public int DisposeCount { get; private set; } + + protected override ValueTask DisposeAsyncCore() + { + this.AsyncDisposeCount++; + return base.DisposeAsyncCore(); + } + + protected override void Dispose(bool disposing) + { + if (disposing) + { + this.DisposeCount++; + } + + base.Dispose(disposing); + } + } +} diff --git a/tests/MUnique.OpenMU.Tests/MonsterRespawnTest.cs b/tests/MUnique.OpenMU.Tests/MonsterRespawnTest.cs new file mode 100644 index 000000000..825871004 --- /dev/null +++ b/tests/MUnique.OpenMU.Tests/MonsterRespawnTest.cs @@ -0,0 +1,108 @@ +// +// Licensed under the MIT License. See LICENSE file in the project root for full license information. +// + +namespace MUnique.OpenMU.Tests; + +using Moq; +using MUnique.OpenMU.DataModel.Configuration; +using MUnique.OpenMU.GameLogic; +using MUnique.OpenMU.GameLogic.Attributes; +using MUnique.OpenMU.GameLogic.NPC; +using MUnique.OpenMU.Pathfinding; +using MonsterAttribute = MUnique.OpenMU.Persistence.BasicModel.MonsterAttribute; +using MonsterDefinition = MUnique.OpenMU.Persistence.BasicModel.MonsterDefinition; + +/// +/// Tests for the respawn of a killed . +/// +[TestFixture] +public class MonsterRespawnTest +{ + private static readonly TimeSpan RespawnDelay = TimeSpan.FromMilliseconds(100); + + private static readonly Point SpawnPoint = new(100, 100); + + /// + /// Tests that a killed monster respawns on its map after the respawn delay. + /// + [Test] + public async Task KilledMonsterRespawnsAsync() + { + var gameContext = GameContextTestHelper.CreateGameContext(); + var killer = await PlayerTestHelper.CreatePlayerAsync(gameContext).ConfigureAwait(false); + var monster = await CreateMonsterAsync(gameContext).ConfigureAwait(false); + + await monster.ReflectDamageAsync(killer, 1_000_000).ConfigureAwait(false); + Assert.That(monster.IsAlive, Is.False); + + var deadline = DateTime.UtcNow.AddSeconds(10); + while (!monster.IsAlive && DateTime.UtcNow < deadline) + { + await Task.Delay(50).ConfigureAwait(false); + } + + Assert.That(monster.IsAlive, Is.True); + Assert.That(monster.CurrentMap.GetNpcsInRange(SpawnPoint, 1), Does.Contain(monster)); + } + + /// + /// Tests that a killed monster which is disposed while it waits to respawn, + /// e.g. because its spawn area got deleted, doesn't come back to the map. + /// + [Test] + public async Task KilledMonsterDoesNotRespawnAfterDisposeAsync() + { + var gameContext = GameContextTestHelper.CreateGameContext(); + var killer = await PlayerTestHelper.CreatePlayerAsync(gameContext).ConfigureAwait(false); + var monster = await CreateMonsterAsync(gameContext).ConfigureAwait(false); + var map = monster.CurrentMap; + var monsterId = monster.Id; + + await monster.ReflectDamageAsync(killer, 1_000_000).ConfigureAwait(false); + await monster.DisposeAsync().ConfigureAwait(false); + + await Task.Delay(RespawnDelay * 5).ConfigureAwait(false); + + Assert.That(monster.IsAlive, Is.False); + Assert.That(map.GetNpcsInRange(SpawnPoint, 1), Does.Not.Contain(monster)); + Assert.That(map.GetAttackablesInRange(SpawnPoint, 1), Does.Not.Contain(monster)); + Assert.That(map.GetObject(monsterId), Is.Not.SameAs(monster)); + } + + private static async ValueTask CreateMonsterAsync(IGameContext gameContext) + { + // A unique id, because the attributes of monster definitions are cached by their id. + var monsterDefinition = new MonsterDefinition + { + Id = Guid.NewGuid(), + ObjectKind = NpcObjectKind.Monster, + RespawnDelay = RespawnDelay, + }; + monsterDefinition.Attributes.Add(new MonsterAttribute { AttributeDefinition = Stats.MaximumHealth, Value = 100 }); + var map = await gameContext.GetMapAsync(0).ConfigureAwait(false); + var spawnArea = new MonsterSpawnArea + { + MonsterDefinition = monsterDefinition, + GameMap = map!.Definition, + SpawnTrigger = SpawnTrigger.Automatic, + X1 = SpawnPoint.X, + Y1 = SpawnPoint.Y, + X2 = SpawnPoint.X, + Y2 = SpawnPoint.Y, + Quantity = 1, + }; + + var monster = new Monster( + spawnArea, + monsterDefinition, + map, + NullDropGenerator.Instance, + new Mock().Object, + gameContext.PlugInManager, + gameContext.PathFinderPool); + monster.Initialize(); + await map.AddAsync(monster).ConfigureAwait(false); + return monster; + } +}