feat(game): character select slot positions from DB query rownum (#334)

andreionoie · web-flow · commit 8e9e9bf587d5 · 2025-09-15T20:03:02.000+02:00
* feat(game): character select slot positions from DB Id rownum

* refactor(game): populate Slot within LINQ

* refactor(game): revert + tests + fix for potential GetPlayerAsync bug
diff --git a/src/Data/Game.Persistence/DbPlayerRepository.cs b/src/Data/Game.Persistence/DbPlayerRepository.cs
@@ -17,11 +17,15 @@ public DbPlayerRepository(GameDbContext db)
 
     public async Task<PlayerData[]> GetPlayersAsync(Guid accountId)
     {
-        return await _db.Players
+        var playersWithUnpopulatedSlot = await _db.Players
             .AsNoTracking()
             .Where(x => x.AccountId == accountId)
+            .OrderBy(x => x.Id)
             .SelectPlayerData()
             .ToArrayAsync();
+
+        return playersWithUnpopulatedSlot
+            .AssignIncrementalSlots();
     }
 
     public async Task<bool> IsNameInUseAsync(string name)
@@ -110,9 +114,29 @@ public async Task SetPlayerAsync(PlayerData data)
 
     public async Task<PlayerData?> GetPlayerAsync(uint playerId)
     {
-        return await _db.Players
+        // TODO: optimize/refactor this part while maintaining Slot field correctness
+        // (needed to avoid overwriting `players:..` cache keys when Slot is assigned default zero value for all players)
+        // Note that currently this method is not used in production code
+        var accountId = await _db.Players
+            .AsNoTracking()
             .Where(x => x.Id == playerId)
+            .Select(x => x.AccountId)
+            .SingleOrDefaultAsync();
+
+        if (accountId == Guid.Empty)
+        {
+            return null;
+        }
+
+        var playersWithUnpopulatedSlot = await _db.Players
+            .AsNoTracking()
+            .Where(x => x.AccountId == accountId)
+            .OrderBy(x => x.Id)
             .SelectPlayerData()
-            .FirstOrDefaultAsync();
+            .ToArrayAsync();
+
+        return playersWithUnpopulatedSlot
+            .AssignIncrementalSlots()
+            .SingleOrDefault(x => x.Id == playerId);
     }
 }
diff --git a/src/Data/Game.Persistence/Extensions/QueryExtensions.cs b/src/Data/Game.Persistence/Extensions/QueryExtensions.cs
@@ -41,6 +41,16 @@ public static IQueryable<PlayerData> SelectPlayerData(this IQueryable<Player> qu
             GuildId = x.GuildId
         });
     }
+    
+    public static PlayerData[] AssignIncrementalSlots(this PlayerData[] players)
+    {
+        for (var i = 0; i < players.Length; i++)
+        {
+            players[i].Slot = (byte)i;
+        }
+
+        return players;
+    }
 
     public static IQueryable<Skill> SelectPlayerSkill(this IQueryable<PlayerSkill> query)
     {
diff --git a/src/Libraries/Game.Server/Commands/PhaseSelectCommand.cs b/src/Libraries/Game.Server/Commands/PhaseSelectCommand.cs
@@ -36,7 +36,6 @@ public async Task ExecuteAsync(CommandContext context)
             context.Player.Connection.SetPhase(EPhases.Select);
 
             var characters = new Characters();
-            var i = 0;
             await using var scope = _serviceProvider.CreateAsyncScope();
             var playerManager = scope.ServiceProvider.GetRequiredService<IPlayerManager>();
             var guildManager = scope.ServiceProvider.GetRequiredService<IGuildManager>();
@@ -46,17 +45,15 @@ public async Task ExecuteAsync(CommandContext context)
                 var host = _world.GetMapHost(player.PositionX, player.PositionY);
                 var guild = await guildManager.GetGuildForPlayerAsync(player.Id);
 
-                // todo character slot position
-                characters.CharacterList[i] = player.ToCharacter();
-                characters.CharacterList[i].Ip = BitConverter.ToInt32(host.Ip.GetAddressBytes());
-                characters.CharacterList[i].Port = host.Port;
-                characters.GuildIds[i] = guild?.Id ?? 0;
-                characters.GuildNames[i] = guild?.Name ?? "";
-
-                i++;
+                var slot = (int)player.Slot;
+                characters.CharacterList[slot] = player.ToCharacter();
+                characters.CharacterList[slot].Ip = BitConverter.ToInt32(host.Ip.GetAddressBytes());
+                characters.CharacterList[slot].Port = host.Port;
+                characters.GuildIds[slot] = guild?.Id ?? 0;
+                characters.GuildNames[slot] = guild?.Name ?? "";
             }
 
             context.Player.Connection.Send(characters);
         }
     }
-}
+}
diff --git a/src/Libraries/Game.Server/Extensions/PaketExtensions.cs b/src/Libraries/Game.Server/Extensions/PaketExtensions.cs
@@ -9,7 +9,7 @@ public static Character ToCharacter(this PlayerData player)
     {
         return new Character
         {
-            Id = 1,
+            Id = player.Id,
             Name = player.Name,
             Class = player.PlayerClass,
             Level = player.Level,
@@ -26,4 +26,4 @@ public static Character ToCharacter(this PlayerData player)
             SkillGroup = player.SkillGroup
         };
     }
-}
+}
diff --git a/src/Libraries/Game.Server/PacketHandlers/TokenLoginHandler.cs b/src/Libraries/Game.Server/PacketHandlers/TokenLoginHandler.cs
@@ -81,22 +81,18 @@ public async Task ExecuteAsync(GamePacketContext<TokenLogin> ctx, CancellationTo
 
             // Load players of account
             var characters = new Characters();
-            var i = 0;
             var charactersFromCacheOrDb = await _playerManager.GetPlayers(token.AccountId);
             foreach (var player in charactersFromCacheOrDb)
             {
                 var host = _world.GetMapHost(player.PositionX, player.PositionY);
 
                 var guild = await _guildManager.GetGuildForPlayerAsync(player.Id, cancellationToken);
-                // todo character slot position
-                characters.CharacterList[i] = player.ToCharacter();
-                characters.CharacterList[i].Ip = BitConverter.ToInt32(host.Ip.GetAddressBytes());
-                characters.CharacterList[i].Port = host.Port;
-                characters.GuildIds[i] = guild?.Id ?? 0;
-                characters.GuildNames[i] = guild?.Name ?? "";
-                // todo armor on character select
-
-                i++;
+                var slot = (int)player.Slot;
+                characters.CharacterList[slot] = player.ToCharacter();
+                characters.CharacterList[slot].Ip = BitConverter.ToInt32(host.Ip.GetAddressBytes());
+                characters.CharacterList[slot].Port = host.Port;
+                characters.GuildIds[slot] = guild?.Id ?? 0;
+                characters.GuildNames[slot] = guild?.Name ?? "";
             }
 
             // When there are no characters belonging to the account, the empire status is stored in the cache.
diff --git a/src/Libraries/Game.Server/PlayerManager.cs b/src/Libraries/Game.Server/PlayerManager.cs
@@ -33,16 +33,11 @@ public PlayerManager(IDbPlayerRepository dbPlayerRepository, ICachePlayerReposit
         if (cachedPlayer is null)
         {
             var players = await _dbPlayerRepository.GetPlayersAsync(accountId);
-            for (var i = 0; i < players.Length; i++)
+            var player = players.FirstOrDefault(p => p.Slot == slot);
+            if (player is not null)
             {
-                var player = players[i];
-                player.Slot = (byte)i;
-
-                if (i == slot)
-                {
-                    await _cachePlayerRepository.SetPlayerAsync(player);
-                    return player;
-                }
+                await _cachePlayerRepository.SetPlayerAsync(player);
+                return player;
             }
 
             _logger.LogWarning("Could not find player for account {AccountId} at slot {Slot}", accountId, slot);
diff --git a/src/Tests/Game.Tests/PlayerManagerTests.cs b/src/Tests/Game.Tests/PlayerManagerTests.cs
@@ -218,6 +218,31 @@ public async Task GetPlayerById_NotFound()
         output.Should().BeNull();
     }
 
+    [Fact]
+    public async Task GetPlayerById_WithMultiplePlayers_CachesUnderCorrectSlot()
+    {
+        await _cacheManager.FlushAll();
+
+        var accountId = Guid.NewGuid();
+
+        const uint FirstId = 100u, SecondId = 200u, ThirdId = 300u;
+        await _dbPlayerRepository.CreateAsync(new PlayerData { Id = FirstId, AccountId = accountId, Name = "PlayerA" });
+        await _dbPlayerRepository.CreateAsync(new PlayerData { Id = SecondId, AccountId = accountId, Name = "PlayerB" });
+        await _dbPlayerRepository.CreateAsync(new PlayerData { Id = ThirdId, AccountId = accountId, Name = "PlayerC" });
+
+        var fetched = await _playerManager.GetPlayer(SecondId);
+
+        fetched.Should().NotBeNull();
+        fetched.Id.Should().Be(SecondId);
+
+        var keys = await _cacheManager.Server.Keys("*");
+        keys.Should().HaveCount(2)
+            .And.Contain($"player:{SecondId}")
+            .And.Contain($"players:{accountId}:1");
+        keys.Should().NotContain($"players:{accountId}:0");
+        keys.Should().NotContain($"players:{accountId}:2");
+    }
+
     [Fact]
     public async Task GetPlayerByAccountIdAndSlot_NotFound()
     {
@@ -237,4 +262,45 @@ public async Task DeleteCharacter()
         (await _cacheManager.Server.Keys("*")).Should().BeEquivalentTo([$"temp:empire-selection:{accountId}"]);
         (await _dbPlayerRepository.GetPlayersAsync(accountId)).Should().BeEmpty();
     }
+
+    [Theory]
+    [InlineData(0)]
+    [InlineData(1)]
+    [InlineData(2)]
+    [InlineData(3)]
+    [InlineData(4)]
+    public async Task GetPlayers_ReturnsOrderedAndCachesWithSlots(int charactersCount)
+    {
+        await _cacheManager.FlushAll();
+        await _db.Players.ExecuteDeleteAsync();
+
+        var accountId = Guid.NewGuid();
+        var basePlayerId = 1000u;
+
+        for (uint i = 0; i < charactersCount; i++)
+        {
+            var id = basePlayerId + i;
+            await _dbPlayerRepository.CreateAsync(new PlayerData { Id = id, AccountId = accountId, Name = $"Player{i}" });
+        }
+
+        var players = await _playerManager.GetPlayers(accountId);
+
+        players.Should().HaveCount(charactersCount);
+        for (var i = 0; i < charactersCount; i++)
+        {
+            players[i].Slot.Should().Be((byte)i);
+
+            var expectedId = basePlayerId + (uint)i; 
+            players[i].Id.Should().Be(expectedId);
+        }
+
+        var keys = await _cacheManager.Server.Keys("*");
+        keys.Should().HaveCount(charactersCount * 2);
+        for (var i = 0; i < charactersCount; i++)
+        {
+            var expectedId = basePlayerId + i; 
+            keys.Should().Contain($"player:{expectedId}");
+            keys.Should().Contain($"players:{accountId}:{i}");
+        }
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ public static Character ToCharacter(this PlayerData player)`
`9`	`9`	`{`
`10`	`10`	`return new Character`
`11`	`11`	`{`
`12`		`- Id = 1,`
	`12`	`+ Id = player.Id,`
`13`	`13`	`Name = player.Name,`
`14`	`14`	`Class = player.PlayerClass,`
`15`	`15`	`Level = player.Level,`
`@@ -26,4 +26,4 @@ public static Character ToCharacter(this PlayerData player)`
`26`	`26`	`SkillGroup = player.SkillGroup`
`27`	`27`	`};`
`28`	`28`	`}`
`29`		`-}`
	`29`	`+}`