Bugfix: offlevel fixes

src/GameLogic/GameContext.cs

@@ -343,6 +343,17 @@ public async ValueTask<IList<Player>> GetPlayersAsync()
     /// <param name="player">The player.</param>
     public virtual async ValueTask RemovePlayerAsync(Player player)
     {
+        bool removed;
+        using (await this._playerListLock.WriterLockAsync())
+        {
+            removed = this._playerList.Remove(player);
+        }
+
+        if (!removed)
+        {
+            return;
+        }
+
         PlayerCounter.Add(-1);
         if (player.SelectedCharacter != null)
         {
@@ -351,11 +362,6 @@ public virtual async ValueTask RemovePlayerAsync(Player player)
 
         player.CurrentMap?.RemoveAsync(player);
 
-        using (await this._playerListLock.WriterLockAsync())
-        {
-            this._playerList.Remove(player);
-        }
-
         player.PlayerDisconnected -= this.RemovePlayerAsync;
         player.PlayerEnteredWorld -= this.PlayerEnteredWorldAsync;
         player.PlayerLeftWorld -= this.PlayerLeftWorldAsync;

src/GameLogic/IGameContext.cs

@@ -138,6 +138,12 @@ public interface IGameContext
     /// <param name="player">The player.</param>
     ValueTask AddPlayerAsync(Player player);
 
+    /// <summary>
+    /// Removes the player from the game.
+    /// </summary>
+    /// <param name="player">The player.</param>
+    ValueTask RemovePlayerAsync(Player player);
+
     /// <summary>
     /// Gets the maps which is meant to be hosted by the game.
     /// </summary>

src/GameLogic/Offline/ItemPickupHandler.cs

@@ -16,7 +16,6 @@ namespace MUnique.OpenMU.GameLogic.Offline;
 public sealed class ItemPickupHandler
 {
     private const byte MinPickupRange = 1;
-    private const byte JewelItemGroup = 14;
 
     private static readonly PickupItemAction PickupAction = new();
 
@@ -35,7 +34,7 @@ public ItemPickupHandler(OfflinePlayer player, IMuHelperSettings? config)
     }
 
     /// <summary>
-    /// Scans for and picks up items within configurable range.
+    /// Scans for and picks up items within a configurable range.
     /// </summary>
     public async ValueTask PickupItemsAsync()
     {
@@ -61,6 +60,17 @@ public async ValueTask PickupItemsAsync()
         }
     }
 
+    private static bool IsJewel(Item item)
+    {
+        if (item.Definition is not { } definition)
+        {
+            return false;
+        }
+
+        return (definition.Group == 14 && definition.Number is 13 or 14 or 16 or 22 or 31 or 41 or 42 or 43 or 44)
+               || (definition.Group == 12 && definition.Number == 15);
+    }
+
     private bool ShouldPickUpDrop(IIdentifiable drop)
     {
         if (this._config!.PickAllItems)
@@ -93,7 +103,7 @@ private bool ShouldPickUp(Item item)
             return false;
         }
 
-        if (this._config.PickJewel && item.Definition?.Group == JewelItemGroup)
+        if (this._config.PickJewel && IsJewel(item))
         {
             return true;
         }

src/GameLogic/Offline/MovementHandler.cs

@@ -71,9 +71,9 @@ public async ValueTask<bool> RegroupAsync()
     /// <param name="range">The range to stop within.</param>
     public async ValueTask MoveCloserToTargetAsync(IAttackable target, byte range)
     {
-        if (target.IsInRange(this._originPosition, this.HuntingRange))
+        if (this._player.CurrentMap is { } map && target.IsInRange(this._originPosition, this.HuntingRange))
         {
-            var walkTarget = this._player.CurrentMap!.Terrain.GetRandomCoordinate(target.Position, range);
+            var walkTarget = map.Terrain.GetRandomCoordinate(target.Position, range);
             await this.WalkToAsync(walkTarget).ConfigureAwait(false);
         }
     }

src/GameLogic/Offline/OfflinePlayer.cs

@@ -15,6 +15,7 @@ namespace MUnique.OpenMU.GameLogic.Offline;
 public sealed class OfflinePlayer : Player
 {
     private OfflinePlayerMuHelper? _intelligence;
+    private Task? _intelligenceDisposeTask;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="OfflinePlayer"/> class.
@@ -36,18 +37,32 @@ public OfflinePlayer(IGameContext gameContext)
     public DateTime StartTimestamp { get; internal set; }
 
     /// <summary>
-    /// Initializes the offline player from captured references.
+    /// Initializes the offline player by loading the account fresh from the database.
     /// </summary>
-    /// <param name="account">The account.</param>
-    /// <param name="character">The character.</param>
+    /// <param name="loginName">The account login name.</param>
+    /// <param name="characterName">The character name to continue with.</param>
     /// <returns><c>true</c> if successfully started.</returns>
-    public async ValueTask<bool> InitializeAsync(Account account, Character character)
+    public async ValueTask<bool> InitializeAsync(string loginName, string characterName)
     {
         try
         {
             this.StartTimestamp = DateTime.UtcNow;
+
+            var account = await this.PersistenceContext.GetAccountByLoginNameAsync(loginName).ConfigureAwait(false);
+            if (account is null)
+            {
+                this.Logger.LogError("Failed to load account {LoginName} for offline session.", loginName);
+                return false;
+            }
+
+            var character = account.Characters?.FirstOrDefault(c => c.Name == characterName);
+            if (character is null)
+            {
+                this.Logger.LogError("Character {CharacterName} not found in account {LoginName}.", characterName, loginName);
+                return false;
+            }
+
             this.Account = account;
-            this.PersistenceContext.Attach(account);
 
             await this.AdvanceToCharacterSelectionStateAsync().ConfigureAwait(false);
 
@@ -67,7 +82,7 @@ public async ValueTask<bool> InitializeAsync(Account account, Character characte
         }
         catch (Exception ex)
         {
-            this.Logger.LogError(ex, "Failed to initialize offline player for {player}.", this);
+            this.Logger.LogError(ex, "Failed to initialize offline player for {LoginName}.", loginName);
             return false;
         }
     }
@@ -76,23 +91,41 @@ public async ValueTask<bool> InitializeAsync(Account account, Character characte
     /// Stops the offline player and removes it from the world.
     /// </summary>
     public async ValueTask StopAsync()
+    {
+        await this.DisconnectAsync().ConfigureAwait(false);
+    }
+
+    /// <inheritdoc />
+    protected override async ValueTask InternalDisconnectAsync()
     {
         if (this._intelligence is { } intelligence)
         {
-            await intelligence.DisposeAsync().ConfigureAwait(false);
             this._intelligence = null;
+            this._intelligenceDisposeTask = Task.Run(async () =>
+            {
+                try
+                {
+                    await intelligence.DisposeAsync().ConfigureAwait(false);
+                }
+                catch (Exception ex)
+                {
+                    this.Logger.LogError(ex, "Error disposing intelligence for offline player {AccountLoginName}.", this.AccountLoginName);
+                }
+            });
         }
 
-        try
-        {
-            await this.SaveProgressAsync().ConfigureAwait(false);
-        }
-        catch (Exception ex)
+        await base.InternalDisconnectAsync().ConfigureAwait(false);
+    }
+
+    /// <inheritdoc />
+    protected override async ValueTask DisposeAsyncCore()
+    {
+        if (this._intelligenceDisposeTask is { } disposeTask)
         {
-            this.Logger.LogError(ex, "Failed to save progress of offline player {AccountLoginName}.", this.AccountLoginName);
+            await disposeTask.ConfigureAwait(false);
         }
 
-        await this.DisconnectAsync().ConfigureAwait(false);
+        await base.DisposeAsyncCore().ConfigureAwait(false);
     }
 
     /// <inheritdoc />
@@ -109,14 +142,8 @@ private async ValueTask AdvanceToCharacterSelectionStateAsync()
 
     private async ValueTask SetupCharacterAsync(Character character)
     {
-        // Add to context and set character.
         await this.GameContext.AddPlayerAsync(this).ConfigureAwait(false);
         await this.SetSelectedCharacterAsync(character).ConfigureAwait(false);
-
-        if (this.SelectedCharacter is { } selectedCharacter)
-        {
-            this.PersistenceContext.Attach(selectedCharacter);
-        }
     }
 
     private void StartIntelligence()

src/GameLogic/Offline/OfflinePlayerManager.cs

@@ -6,6 +6,7 @@ namespace MUnique.OpenMU.GameLogic.Offline;
 
 using MUnique.OpenMU.GameLogic.Attributes;
 using MUnique.OpenMU.GameLogic.MuHelper;
+using MUnique.OpenMU.GameLogic.Views.Login;
 
 /// <summary>
 /// Manages active <see cref="OfflinePlayer"/> sessions.
@@ -29,10 +30,9 @@ public IReadOnlyCollection<OfflinePlayer> OfflinePlayers
     /// <returns><c>true</c> if the offline session was started successfully.</returns>
     public async ValueTask<bool> StartAsync(Player realPlayer, string loginName)
     {
-        var account = realPlayer.Account;
-        var character = realPlayer.SelectedCharacter;
+        var characterName = realPlayer.SelectedCharacter?.Name;
 
-        if (account is null || character is null)
+        if (string.IsNullOrEmpty(characterName))
         {
             return false;
         }
@@ -56,17 +56,17 @@ public async ValueTask<bool> StartAsync(Player realPlayer, string loginName)
         {
             await this.TransitionToOfflineAsync(realPlayer, loginName).ConfigureAwait(false);
 
-            if (!await sentinel.InitializeAsync(account, character).ConfigureAwait(false))
+            if (!await sentinel.InitializeAsync(loginName, characterName).ConfigureAwait(false))
             {
-                this._activePlayers.TryRemove(loginName, out _);
+                await this.RemoveAndDisposeAsync(loginName, sentinel).ConfigureAwait(false);
                 return false;
             }
 
             return true;
         }
         catch
         {
-            this._activePlayers.TryRemove(loginName, out _);
+            await this.RemoveAndDisposeAsync(loginName, sentinel).ConfigureAwait(false);
             throw;
         }
     }
@@ -77,10 +77,25 @@ public async ValueTask<bool> StartAsync(Player realPlayer, string loginName)
     /// <param name="loginName">The account login name.</param>
     public async ValueTask StopAsync(string loginName)
     {
-        if (this._activePlayers.TryRemove(loginName, out var offlinePlayer))
+        if (!this._activePlayers.TryRemove(loginName, out var offlinePlayer))
+        {
+            // The session might have been started and stopped outside the manager (e.g. in tests).
+            return;
+        }
+
+        try
         {
             await offlinePlayer.StopAsync().ConfigureAwait(false);
         }
+        catch (Exception ex)
+        {
+            offlinePlayer.Logger.LogError(ex, "Error stopping offline player session for {0}.", loginName);
+        }
+        finally
+        {
+            await offlinePlayer.GameContext.RemovePlayerAsync(offlinePlayer).ConfigureAwait(false);
+            await offlinePlayer.DisposeAsync().ConfigureAwait(false);
+        }
     }
 
     /// <summary>
@@ -102,9 +117,12 @@ private async ValueTask TransitionToOfflineAsync(Player realPlayer, string login
     {
         await this.LogOffFromLoginServerAsync(realPlayer, loginName).ConfigureAwait(false);
 
-        realPlayer.SuppressDisconnectedEvent();
+        // Send a close-game packet so the client exits cleanly without auto-reconnecting.
+        // DisconnectAsync will then fire PlayerDisconnected, which triggers
+        // RemovePlayerAsync (player list cleanup) and OnPlayerDisconnectedAsync (dispose).
+        await realPlayer.InvokeViewPlugInAsync<ILogoutPlugIn>(p => p.LogoutAsync(LogoutType.CloseGame)).ConfigureAwait(false);
+
         await realPlayer.DisconnectAsync().ConfigureAwait(false);
-        realPlayer.PersistenceContext.Dispose();
     }
 
     /// <summary>