From 01bbb16c8a041d51682ce04d305944a391c854f3 Mon Sep 17 00:00:00 2001 From: Enn3Developer Date: Mon, 31 Aug 2026 20:22:17 +0200 Subject: [PATCH 1/2] fix: latent issues in lobby management + added tests --- OpenPolytopia.Common/LobbyData.cs | 7 + OpenPolytopia.Common/LobbyRules.cs | 61 +++ .../Network/Packets/LobbyPackets.cs | 7 + OpenPolytopia.Server/GameServer.cs | 20 +- OpenPolytopia.Server/LobbyManager.cs | 38 +- OpenPolytopia.UnitTest/LobbyManagerTest.cs | 352 ++++++++++++++++++ OpenPolytopia.UnitTest/LobbyRulesTest.cs | 63 ++++ .../OpenPolytopia.UnitTest.csproj | 1 + OpenPolytopia.UnitTest/PacketTest.cs | 9 +- 9 files changed, 531 insertions(+), 27 deletions(-) create mode 100644 OpenPolytopia.Common/LobbyRules.cs create mode 100644 OpenPolytopia.UnitTest/LobbyManagerTest.cs create mode 100644 OpenPolytopia.UnitTest/LobbyRulesTest.cs diff --git a/OpenPolytopia.Common/LobbyData.cs b/OpenPolytopia.Common/LobbyData.cs index c03d20d..ee70341 100644 --- a/OpenPolytopia.Common/LobbyData.cs +++ b/OpenPolytopia.Common/LobbyData.cs @@ -55,6 +55,11 @@ public class LobbyData : INetworkSerializable { /// public uint MaxPlayers; + /// + /// Size of the world the game will be played on + /// + public uint WorldSize; + /// /// If the game in the lobby has started /// @@ -89,6 +94,7 @@ public class LobbyData : INetworkSerializable { public void Serialize(List bytes) { Id.Serialize(bytes); MaxPlayers.Serialize(bytes); + WorldSize.Serialize(bytes); Started.Serialize(bytes); Starting.Serialize(bytes); Players.Serialize(bytes); @@ -97,6 +103,7 @@ public void Serialize(List bytes) { public void Deserialize(byte[] bytes, ref uint index) { Id.Deserialize(bytes, ref index); MaxPlayers.Deserialize(bytes, ref index); + WorldSize.Deserialize(bytes, ref index); Started.Deserialize(bytes, ref index); Starting.Deserialize(bytes, ref index); Players.Deserialize(bytes, ref index); diff --git a/OpenPolytopia.Common/LobbyRules.cs b/OpenPolytopia.Common/LobbyRules.cs new file mode 100644 index 0000000..a6db5cf --- /dev/null +++ b/OpenPolytopia.Common/LobbyRules.cs @@ -0,0 +1,61 @@ +namespace OpenPolytopia.Common; + +/// +/// Rules a lobby must respect to be valid +/// +/// +/// Lives in the common assembly so the client can check a lobby before asking the server for it +/// +public static class LobbyRules { + /// + /// Smallest world a game can be played on + /// + public const uint MIN_WORLD_SIZE = 11; + + /// + /// Biggest world a game can be played on + /// + public const uint MAX_WORLD_SIZE = 30; + + /// + /// Minimum players needed to start a game; solo games aren't allowed + /// + public const uint MIN_PLAYERS = 2; + + /// + /// World size from which a lobby can hold players + /// + private const uint BIG_WORLD_SIZE = 14; + + /// + /// Max players a world smaller than can hold + /// + private const uint MAX_PLAYERS_SMALL_WORLD = 9; + + /// + /// Max players a world of at least can hold + /// + private const uint MAX_PLAYERS_BIG_WORLD = 16; + + /// + /// Max players a lobby on a given world can hold + /// + /// the size of the world + /// the max players allowed on that world + public static uint MaxPlayersFor(uint worldSize) => + worldSize < BIG_WORLD_SIZE ? MAX_PLAYERS_SMALL_WORLD : MAX_PLAYERS_BIG_WORLD; + + /// + /// Checks if a world can host a game + /// + /// the size of the world + public static bool IsValidWorldSize(uint worldSize) => worldSize is >= MIN_WORLD_SIZE and <= MAX_WORLD_SIZE; + + /// + /// Checks if a lobby of a given size can be created on a given world + /// + /// max players that can join the lobby + /// the size of the world + public static bool IsValidLobby(uint maxPlayers, uint worldSize) => + IsValidWorldSize(worldSize) && maxPlayers >= MIN_PLAYERS && maxPlayers <= MaxPlayersFor(worldSize); +} diff --git a/OpenPolytopia.Common/Network/Packets/LobbyPackets.cs b/OpenPolytopia.Common/Network/Packets/LobbyPackets.cs index 37620ea..72e9dd1 100644 --- a/OpenPolytopia.Common/Network/Packets/LobbyPackets.cs +++ b/OpenPolytopia.Common/Network/Packets/LobbyPackets.cs @@ -35,6 +35,11 @@ public class CreateLobbyPacket : IPacket { /// public uint MaxPlayers; + /// + /// Size of the world the game will be played on + /// + public uint WorldSize; + /// /// Tribe chosen by the player creating the lobby /// @@ -42,11 +47,13 @@ public class CreateLobbyPacket : IPacket { public void Serialize(List bytes) { MaxPlayers.Serialize(bytes); + WorldSize.Serialize(bytes); Tribe.Serialize(bytes); } public void Deserialize(byte[] bytes, ref uint index) { MaxPlayers.Deserialize(bytes, ref index); + WorldSize.Deserialize(bytes, ref index); Tribe.Deserialize(bytes, ref index); } } diff --git a/OpenPolytopia.Server/GameServer.cs b/OpenPolytopia.Server/GameServer.cs index 22d6bcb..3a0bd3b 100644 --- a/OpenPolytopia.Server/GameServer.cs +++ b/OpenPolytopia.Server/GameServer.cs @@ -167,7 +167,7 @@ private async Task ManageCreateLobbyAsync(NetworkConnection connection, CreateLo if (!_playerNames.TryGetValue(connection.Id, out var name)) { result = LobbyActionResult.NotRegistered; } - else if (packet.MaxPlayers is < 2 or > 16 || !Enum.IsDefined((TribeType)packet.Tribe)) { + else if (!Enum.IsDefined((TribeType)packet.Tribe)) { result = LobbyActionResult.InvalidParameters; } // one lobby per player and a global cap, or a client could flood the server @@ -178,9 +178,9 @@ private async Task ManageCreateLobbyAsync(NetworkConnection connection, CreateLo result = LobbyActionResult.TooManyLobbies; } else { - result = LobbyActionResult.Ok; - lobby = _lobbyManager.CreateLobby(packet.MaxPlayers, - new LobbyPlayerData { PlayerId = connection.Id, Name = name, Tribe = packet.Tribe }); + // the lobby rules themselves are checked by the manager, so a lobby is never half-valid + result = _lobbyManager.CreateLobby(packet.MaxPlayers, packet.WorldSize, + new LobbyPlayerData { PlayerId = connection.Id, Name = name, Tribe = packet.Tribe }, out lobby); } _server.SendTo(connection.Id, new CreateLobbyResponsePacket { Result = result, LobbyId = lobby?.Id ?? 0 }); @@ -232,14 +232,14 @@ private async Task ManageLeaveLobbyAsync(NetworkConnection connection, LeaveLobb _server.SendTo(connection.Id, new LeaveLobbyResponsePacket { Result = result, LobbyId = packet.LobbyId }); - if (result == LobbyActionResult.Ok && _lobbyManager[packet.LobbyId] is { } lobby) { - // remove the lobby if it became empty - if (lobby.PlayersCount == 0) { - _lobbyManager.RemoveLobby(lobby.Id); - _server.Broadcast(new LobbyDeletedPacket { LobbyId = lobby.Id }); + if (result == LobbyActionResult.Ok) { + // the manager drops a lobby as soon as its last player leaves, so a missing + // lobby here means it became empty + if (_lobbyManager[packet.LobbyId] is { } lobby) { + _server.Broadcast(new LobbyUpdatedPacket { Lobby = lobby }); } else { - _server.Broadcast(new LobbyUpdatedPacket { Lobby = lobby }); + _server.Broadcast(new LobbyDeletedPacket { LobbyId = packet.LobbyId }); } } } diff --git a/OpenPolytopia.Server/LobbyManager.cs b/OpenPolytopia.Server/LobbyManager.cs index e43d074..6d68b67 100644 --- a/OpenPolytopia.Server/LobbyManager.cs +++ b/OpenPolytopia.Server/LobbyManager.cs @@ -10,15 +10,6 @@ namespace OpenPolytopia.Server; /// This class isn't thread-safe, serializes every access through its own lock /// public class LobbyManager { - /// - /// Minimum players needed to start a game; solo games aren't allowed - /// - /// - /// already refuses to create a lobby smaller than this, - /// this keeps the rule from depending on a check living in another class - /// - private const uint MIN_PLAYERS_TO_START = 2; - private readonly Dictionary _lobbies = new(); private ulong _nextId; @@ -41,13 +32,26 @@ public class LobbyManager { /// /// Creates a new lobby and adds the creator to it /// + /// + /// A lobby is never created in an invalid state: the world has to be able to host every player + /// /// max players that can join the lobby + /// size of the world the game will be played on /// the player creating the lobby - /// the new lobby - public LobbyData CreateLobby(uint maxPlayers, LobbyPlayerData creator) { - var lobby = new LobbyData { Id = ++_nextId, MaxPlayers = maxPlayers, Players = [creator] }; + /// the new lobby, or if it couldn't be created + /// the result of the operation + public LobbyActionResult CreateLobby(uint maxPlayers, uint worldSize, LobbyPlayerData creator, + out LobbyData? lobby) { + if (!LobbyRules.IsValidLobby(maxPlayers, worldSize)) { + lobby = null; + return LobbyActionResult.InvalidParameters; + } + + lobby = new LobbyData { + Id = ++_nextId, MaxPlayers = maxPlayers, WorldSize = worldSize, Players = [creator] + }; _lobbies[lobby.Id] = lobby; - return lobby; + return LobbyActionResult.Ok; } /// @@ -100,6 +104,12 @@ public LobbyActionResult LeaveLobby(ulong lobbyId, uint playerId) { } lobby.Players.Remove(player); + + // an empty lobby has nothing left to wait for + if (lobby.PlayersCount == 0) { + _lobbies.Remove(lobby.Id); + } + return LobbyActionResult.Ok; } @@ -143,7 +153,7 @@ public LobbyActionResult SetReady(ulong lobbyId, uint playerId, bool ready) { /// the lobby to check private static void TryMarkStarting(LobbyData lobby) { if (lobby.PlayersCount == lobby.MaxPlayers - && lobby.PlayersCount >= MIN_PLAYERS_TO_START + && lobby.PlayersCount >= LobbyRules.MIN_PLAYERS && lobby.ReadyCount == lobby.PlayersCount) { lobby.Starting = true; } diff --git a/OpenPolytopia.UnitTest/LobbyManagerTest.cs b/OpenPolytopia.UnitTest/LobbyManagerTest.cs new file mode 100644 index 0000000..a2529e0 --- /dev/null +++ b/OpenPolytopia.UnitTest/LobbyManagerTest.cs @@ -0,0 +1,352 @@ +namespace OpenPolytopia; + +using Common; +using Common.Network.Packets; +using Server; +using Shouldly; + +public class LobbyManagerTest { + private const uint WORLD_SIZE = 16; + + private readonly LobbyManager _lobbyManager = new(); + + private static LobbyPlayerData Player(uint playerId, string name = "Tester", uint tribe = 0) => + new() { PlayerId = playerId, Name = name, Tribe = tribe }; + + /// + /// Creates a lobby, failing the test if the parameters aren't valid + /// + private LobbyData NewLobby(uint maxPlayers, LobbyPlayerData creator, uint worldSize = WORLD_SIZE) { + _lobbyManager.CreateLobby(maxPlayers, worldSize, creator, out var lobby).ShouldBe(LobbyActionResult.Ok); + return lobby.ShouldNotBeNull(); + } + + /// + /// Creates a full lobby whose players are all ready but the last one + /// + private LobbyData ReadyLobby(uint maxPlayers) { + var lobby = NewLobby(maxPlayers, Player(1)); + for (var playerId = 2u; playerId <= maxPlayers; playerId++) { + _lobbyManager.JoinLobby(lobby.Id, Player(playerId)).ShouldBe(LobbyActionResult.Ok); + } + + for (var playerId = 1u; playerId < maxPlayers; playerId++) { + _lobbyManager.SetReady(lobby.Id, playerId, true).ShouldBe(LobbyActionResult.Ok); + } + + return lobby; + } + + [Fact] + public void TestCreateLobby() { + var lobby = NewLobby(4, Player(1, "Creator")); + + lobby.MaxPlayers.ShouldBe(4u); + lobby.WorldSize.ShouldBe(WORLD_SIZE); + lobby.PlayersCount.ShouldBe(1u); + lobby.Players[0].Name.ShouldBe("Creator"); + lobby.Starting.ShouldBeFalse(); + lobby.Started.ShouldBeFalse(); + + _lobbyManager.LobbiesCount.ShouldBe(1); + _lobbyManager[lobby.Id].ShouldBe(lobby); + } + + [Fact] + public void TestLobbyIdsAreUnique() { + // ids start at 1, so 0 can be used as "no lobby" + NewLobby(2, Player(1)).Id.ShouldBe(1u); + NewLobby(2, Player(2)).Id.ShouldBe(2u); + _lobbyManager.LobbiesCount.ShouldBe(2); + } + + [Theory] + [InlineData(1u, 16u)] + [InlineData(17u, 16u)] + [InlineData(10u, 13u)] + [InlineData(2u, 10u)] + [InlineData(2u, 31u)] + public void TestCreateInvalidLobby(uint maxPlayers, uint worldSize) { + _lobbyManager.CreateLobby(maxPlayers, worldSize, Player(1), out var lobby) + .ShouldBe(LobbyActionResult.InvalidParameters); + + // an invalid lobby is never created, not even a half-built one + lobby.ShouldBeNull(); + _lobbyManager.LobbiesCount.ShouldBe(0); + } + + [Fact] + public void TestInvalidLobbyDoesntConsumeId() { + _lobbyManager.CreateLobby(99, WORLD_SIZE, Player(1), out _).ShouldBe(LobbyActionResult.InvalidParameters); + + NewLobby(2, Player(1)).Id.ShouldBe(1u); + } + + [Fact] + public void TestUnknownLobby() { + _lobbyManager[42].ShouldBeNull(); + _lobbyManager.JoinLobby(42, Player(1)).ShouldBe(LobbyActionResult.LobbyNotFound); + _lobbyManager.LeaveLobby(42, 1).ShouldBe(LobbyActionResult.LobbyNotFound); + _lobbyManager.SetReady(42, 1, true).ShouldBe(LobbyActionResult.LobbyNotFound); + } + + [Fact] + public void TestJoinLobby() { + var lobby = NewLobby(2, Player(1)); + + _lobbyManager.JoinLobby(lobby.Id, Player(2, "Joiner")).ShouldBe(LobbyActionResult.Ok); + lobby.PlayersCount.ShouldBe(2u); + lobby[2].ShouldNotBeNull().Name.ShouldBe("Joiner"); + } + + [Fact] + public void TestJoinTwice() { + var lobby = NewLobby(4, Player(1)); + + _lobbyManager.JoinLobby(lobby.Id, Player(1)).ShouldBe(LobbyActionResult.AlreadyJoinedLobby); + lobby.PlayersCount.ShouldBe(1u); + } + + [Fact] + public void TestJoinFullLobby() { + var lobby = NewLobby(2, Player(1)); + _lobbyManager.JoinLobby(lobby.Id, Player(2)).ShouldBe(LobbyActionResult.Ok); + + _lobbyManager.JoinLobby(lobby.Id, Player(3)).ShouldBe(LobbyActionResult.LobbyFull); + lobby.PlayersCount.ShouldBe(2u); + } + + [Fact] + public void TestLeaveLobby() { + var lobby = NewLobby(4, Player(1)); + _lobbyManager.JoinLobby(lobby.Id, Player(2)); + + _lobbyManager.LeaveLobby(lobby.Id, 2).ShouldBe(LobbyActionResult.Ok); + lobby.PlayersCount.ShouldBe(1u); + lobby[2].ShouldBeNull(); + _lobbyManager[lobby.Id].ShouldNotBeNull(); + } + + [Fact] + public void TestLeaveLobbyNotJoined() { + var lobby = NewLobby(4, Player(1)); + + _lobbyManager.LeaveLobby(lobby.Id, 2).ShouldBe(LobbyActionResult.NotInLobby); + lobby.PlayersCount.ShouldBe(1u); + } + + [Fact] + public void TestLeaveLobbyRemovesEmptyLobby() { + var lobby = NewLobby(4, Player(1)); + + // the last player leaving takes the lobby with him + _lobbyManager.LeaveLobby(lobby.Id, 1).ShouldBe(LobbyActionResult.Ok); + _lobbyManager.LobbiesCount.ShouldBe(0); + _lobbyManager[lobby.Id].ShouldBeNull(); + } + + [Fact] + public void TestLeaveLobbyKeepsOtherLobbies() { + var emptied = NewLobby(4, Player(1)); + var other = NewLobby(4, Player(2)); + + _lobbyManager.LeaveLobby(emptied.Id, 1).ShouldBe(LobbyActionResult.Ok); + + _lobbyManager[emptied.Id].ShouldBeNull(); + _lobbyManager[other.Id].ShouldNotBeNull(); + _lobbyManager.LobbiesCount.ShouldBe(1); + } + + [Fact] + public void TestSetReady() { + var lobby = NewLobby(4, Player(1)); + + _lobbyManager.SetReady(lobby.Id, 1, true).ShouldBe(LobbyActionResult.Ok); + lobby[1].ShouldNotBeNull().Ready.ShouldBeTrue(); + lobby.ReadyCount.ShouldBe(1u); + + _lobbyManager.SetReady(lobby.Id, 1, false).ShouldBe(LobbyActionResult.Ok); + lobby[1].ShouldNotBeNull().Ready.ShouldBeFalse(); + lobby.ReadyCount.ShouldBe(0u); + } + + [Fact] + public void TestSetReadyNotJoined() { + var lobby = NewLobby(4, Player(1)); + + _lobbyManager.SetReady(lobby.Id, 2, true).ShouldBe(LobbyActionResult.NotInLobby); + lobby.ReadyCount.ShouldBe(0u); + } + + [Fact] + public void TestLobbyStartsWhenFullAndReady() { + var lobby = ReadyLobby(3); + lobby.Starting.ShouldBeFalse(); + + _lobbyManager.SetReady(lobby.Id, 3, true).ShouldBe(LobbyActionResult.Ok); + lobby.Starting.ShouldBeTrue(); + } + + [Fact] + public void TestLobbyDoesntStartWhenNotFull() { + var lobby = NewLobby(3, Player(1)); + _lobbyManager.JoinLobby(lobby.Id, Player(2)); + + _lobbyManager.SetReady(lobby.Id, 1, true); + _lobbyManager.SetReady(lobby.Id, 2, true); + + // everyone is ready, but the lobby still has a free slot + lobby.ReadyCount.ShouldBe(lobby.PlayersCount); + lobby.Starting.ShouldBeFalse(); + } + + [Fact] + public void TestJoiningDoesntStartLobby() { + var lobby = NewLobby(2, Player(1)); + _lobbyManager.SetReady(lobby.Id, 1, true); + + // only SetReady can start a lobby, even if the joining player says he's ready + var joiner = Player(2); + joiner.Ready = true; + _lobbyManager.JoinLobby(lobby.Id, joiner).ShouldBe(LobbyActionResult.Ok); + + lobby.PlayersCount.ShouldBe(lobby.MaxPlayers); + lobby.ReadyCount.ShouldBe(lobby.PlayersCount); + lobby.Starting.ShouldBeFalse(); + } + + [Fact] + public void TestStartingLobbyIsLocked() { + var lobby = ReadyLobby(2); + _lobbyManager.SetReady(lobby.Id, 2, true); + lobby.Starting.ShouldBeTrue(); + + _lobbyManager.JoinLobby(lobby.Id, Player(3)).ShouldBe(LobbyActionResult.LobbyAlreadyStarted); + _lobbyManager.LeaveLobby(lobby.Id, 1).ShouldBe(LobbyActionResult.LobbyAlreadyStarted); + _lobbyManager.SetReady(lobby.Id, 1, false).ShouldBe(LobbyActionResult.LobbyAlreadyStarted); + lobby.PlayersCount.ShouldBe(2u); + } + + [Fact] + public void TestIsPlayerInAnyLobby() { + var lobby = NewLobby(4, Player(1)); + NewLobby(4, Player(2)); + + _lobbyManager.IsPlayerInAnyLobby(1).ShouldBeTrue(); + _lobbyManager.IsPlayerInAnyLobby(2).ShouldBeTrue(); + _lobbyManager.IsPlayerInAnyLobby(3).ShouldBeFalse(); + + _lobbyManager.LeaveLobby(lobby.Id, 1); + _lobbyManager.IsPlayerInAnyLobby(1).ShouldBeFalse(); + } + + [Fact] + public void TestRenamePlayerInLobbies() { + var first = NewLobby(4, Player(1, "Old")); + var second = NewLobby(4, Player(2)); + _lobbyManager.JoinLobby(second.Id, Player(1, "Old")); + + List updated = []; + _lobbyManager.RenamePlayerInLobbies(1, "New", updated); + + updated.Count.ShouldBe(2); + first[1].ShouldNotBeNull().Name.ShouldBe("New"); + second[1].ShouldNotBeNull().Name.ShouldBe("New"); + second[2].ShouldNotBeNull().Name.ShouldBe("Tester"); + } + + [Fact] + public void TestRenameUnknownPlayer() { + NewLobby(4, Player(1)); + + List updated = []; + _lobbyManager.RenamePlayerInLobbies(2, "New", updated); + + updated.ShouldBeEmpty(); + } + + [Fact] + public void TestRemoveLobby() { + var lobby = NewLobby(4, Player(1)); + + _lobbyManager.RemoveLobby(lobby.Id); + _lobbyManager.LobbiesCount.ShouldBe(0); + _lobbyManager[lobby.Id].ShouldBeNull(); + + // removing an unknown lobby is a no-op + _lobbyManager.RemoveLobby(lobby.Id); + _lobbyManager.LobbiesCount.ShouldBe(0); + } + + [Fact] + public void TestRemovePlayerFromAllLobbies() { + var shared = NewLobby(4, Player(1)); + _lobbyManager.JoinLobby(shared.Id, Player(2)); + var owned = NewLobby(4, Player(3)); + var untouched = NewLobby(4, Player(4)); + + List updated = []; + List deleted = []; + _lobbyManager.RemovePlayerFromAllLobbies(1, updated, deleted); + _lobbyManager.RemovePlayerFromAllLobbies(3, updated, deleted); + + // the shared lobby survives with one player left, the one-player lobby is removed + updated.ShouldBe([shared]); + deleted.ShouldBe([owned.Id]); + shared.PlayersCount.ShouldBe(1u); + _lobbyManager[owned.Id].ShouldBeNull(); + _lobbyManager[untouched.Id].ShouldNotBeNull(); + } + + [Fact] + public void TestRemovePlayerStopsStartingLobby() { + var lobby = ReadyLobby(2); + _lobbyManager.SetReady(lobby.Id, 2, true); + lobby.Starting.ShouldBeTrue(); + + List updated = []; + List deleted = []; + _lobbyManager.RemovePlayerFromAllLobbies(2, updated, deleted); + + // the lobby isn't full anymore, so its game must not start + lobby.Starting.ShouldBeFalse(); + updated.ShouldBe([lobby]); + deleted.ShouldBeEmpty(); + } + + [Fact] + public void TestTakeStartingLobbies() { + var starting = ReadyLobby(2); + _lobbyManager.SetReady(starting.Id, 2, true); + var waiting = NewLobby(2, Player(3)); + + var taken = _lobbyManager.TakeStartingLobbies(); + + taken.ShouldBe([starting]); + starting.Started.ShouldBeTrue(); + + // a started lobby leaves the manager, the waiting one stays + _lobbyManager[starting.Id].ShouldBeNull(); + _lobbyManager.LobbiesCount.ShouldBe(1); + _lobbyManager[waiting.Id].ShouldNotBeNull(); + } + + [Fact] + public void TestTakeStartingLobbiesWithNoneStarting() { + NewLobby(2, Player(1)); + + _lobbyManager.TakeStartingLobbies().ShouldBeEmpty(); + _lobbyManager.LobbiesCount.ShouldBe(1); + } + + [Fact] + public void TestTakeStartingLobbiesIsIdempotent() { + var lobby = ReadyLobby(2); + _lobbyManager.SetReady(lobby.Id, 2, true); + + _lobbyManager.TakeStartingLobbies().Count.ShouldBe(1); + + // the lobby already left the manager, so it can't be started twice + _lobbyManager.TakeStartingLobbies().ShouldBeEmpty(); + } +} diff --git a/OpenPolytopia.UnitTest/LobbyRulesTest.cs b/OpenPolytopia.UnitTest/LobbyRulesTest.cs new file mode 100644 index 0000000..9857ddd --- /dev/null +++ b/OpenPolytopia.UnitTest/LobbyRulesTest.cs @@ -0,0 +1,63 @@ +namespace OpenPolytopia; + +using Common; +using Shouldly; + +public class LobbyRulesTest { + [Theory] + [InlineData(11u, 9u)] + [InlineData(12u, 9u)] + [InlineData(13u, 9u)] + [InlineData(14u, 16u)] + [InlineData(16u, 16u)] + [InlineData(18u, 16u)] + [InlineData(20u, 16u)] + [InlineData(30u, 16u)] + public void TestMaxPlayersFor(uint worldSize, uint expected) => + LobbyRules.MaxPlayersFor(worldSize).ShouldBe(expected); + + [Fact] + public void TestMaxPlayersNeverShrinks() { + // a bigger world can always host at least as many players as a smaller one + for (var worldSize = LobbyRules.MIN_WORLD_SIZE; worldSize < LobbyRules.MAX_WORLD_SIZE; worldSize++) { + LobbyRules.MaxPlayersFor(worldSize + 1).ShouldBeGreaterThanOrEqualTo(LobbyRules.MaxPlayersFor(worldSize)); + } + } + + [Fact] + public void TestValidWorldSize() { + LobbyRules.IsValidWorldSize(LobbyRules.MIN_WORLD_SIZE).ShouldBeTrue(); + LobbyRules.IsValidWorldSize(LobbyRules.MAX_WORLD_SIZE).ShouldBeTrue(); + LobbyRules.IsValidWorldSize(LobbyRules.MIN_WORLD_SIZE - 1).ShouldBeFalse(); + LobbyRules.IsValidWorldSize(LobbyRules.MAX_WORLD_SIZE + 1).ShouldBeFalse(); + LobbyRules.IsValidWorldSize(0).ShouldBeFalse(); + } + + [Fact] + public void TestValidLobby() { + LobbyRules.IsValidLobby(2, 11).ShouldBeTrue(); + LobbyRules.IsValidLobby(9, 11).ShouldBeTrue(); + LobbyRules.IsValidLobby(16, 14).ShouldBeTrue(); + } + + [Fact] + public void TestSoloLobbyIsInvalid() { + LobbyRules.IsValidLobby(0, 16).ShouldBeFalse(); + LobbyRules.IsValidLobby(1, 16).ShouldBeFalse(); + LobbyRules.IsValidLobby(LobbyRules.MIN_PLAYERS, 16).ShouldBeTrue(); + } + + [Fact] + public void TestCrowdedSmallWorldIsInvalid() { + // a world smaller than 14x14 can't host more than 9 players + LobbyRules.IsValidLobby(10, 13).ShouldBeFalse(); + LobbyRules.IsValidLobby(10, 14).ShouldBeTrue(); + LobbyRules.IsValidLobby(17, 30).ShouldBeFalse(); + } + + [Fact] + public void TestLobbyOnInvalidWorldIsInvalid() { + LobbyRules.IsValidLobby(2, LobbyRules.MIN_WORLD_SIZE - 1).ShouldBeFalse(); + LobbyRules.IsValidLobby(2, LobbyRules.MAX_WORLD_SIZE + 1).ShouldBeFalse(); + } +} diff --git a/OpenPolytopia.UnitTest/OpenPolytopia.UnitTest.csproj b/OpenPolytopia.UnitTest/OpenPolytopia.UnitTest.csproj index a43f8a8..a8867b8 100644 --- a/OpenPolytopia.UnitTest/OpenPolytopia.UnitTest.csproj +++ b/OpenPolytopia.UnitTest/OpenPolytopia.UnitTest.csproj @@ -21,6 +21,7 @@ + \ No newline at end of file diff --git a/OpenPolytopia.UnitTest/PacketTest.cs b/OpenPolytopia.UnitTest/PacketTest.cs index 7430466..79c8337 100644 --- a/OpenPolytopia.UnitTest/PacketTest.cs +++ b/OpenPolytopia.UnitTest/PacketTest.cs @@ -64,12 +64,13 @@ public void TestGetLobbies() { [Fact] public void TestGetLobbiesResponse() { - var lobby = new LobbyData { Id = 123, MaxPlayers = 4 }; + var lobby = new LobbyData { Id = 123, MaxPlayers = 4, WorldSize = 16 }; lobby.Players.Add(new LobbyPlayerData { PlayerId = 7, Name = "Test", Tribe = 2, Ready = true }); var packet = RoundTrip(new GetLobbiesResponsePacket { Lobbies = [lobby] }); packet.Lobbies.Count.ShouldBe(1); packet.Lobbies[0].Id.ShouldBe(123u); packet.Lobbies[0].MaxPlayers.ShouldBe(4u); + packet.Lobbies[0].WorldSize.ShouldBe(16u); packet.Lobbies[0].Players.Count.ShouldBe(1); packet.Lobbies[0].Players[0].PlayerId.ShouldBe(7u); packet.Lobbies[0].Players[0].Name.ShouldBe("Test"); @@ -80,8 +81,9 @@ public void TestGetLobbiesResponse() { [Fact] public void TestCreateLobby() { - var packet = RoundTrip(new CreateLobbyPacket { MaxPlayers = 8, Tribe = 3 }); + var packet = RoundTrip(new CreateLobbyPacket { MaxPlayers = 8, WorldSize = 18, Tribe = 3 }); packet.MaxPlayers.ShouldBe(8u); + packet.WorldSize.ShouldBe(18u); packet.Tribe.ShouldBe(3u); } @@ -135,11 +137,12 @@ public void TestSetReadyResponse() { [Fact] public void TestLobbyUpdated() { - var lobby = new LobbyData { Id = 55, MaxPlayers = 2 }; + var lobby = new LobbyData { Id = 55, MaxPlayers = 2, WorldSize = 11 }; lobby.Players.Add(new LobbyPlayerData { PlayerId = 9, Name = "Test", Tribe = 1 }); var packet = RoundTrip(new LobbyUpdatedPacket { Lobby = lobby }); packet.Lobby.Id.ShouldBe(55u); packet.Lobby.MaxPlayers.ShouldBe(2u); + packet.Lobby.WorldSize.ShouldBe(11u); packet.Lobby.Players.Count.ShouldBe(1); packet.Lobby.Players[0].Name.ShouldBe("Test"); } From 0a28ab5a9c1bd8a946391f093005e410803c86a3 Mon Sep 17 00:00:00 2001 From: Enn3Developer Date: Mon, 31 Aug 2026 20:25:53 +0200 Subject: [PATCH 2/2] fix: renamed test methods --- OpenPolytopia.UnitTest/LobbyManagerTest.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/OpenPolytopia.UnitTest/LobbyManagerTest.cs b/OpenPolytopia.UnitTest/LobbyManagerTest.cs index a2529e0..787b48d 100644 --- a/OpenPolytopia.UnitTest/LobbyManagerTest.cs +++ b/OpenPolytopia.UnitTest/LobbyManagerTest.cs @@ -76,7 +76,7 @@ public void TestCreateInvalidLobby(uint maxPlayers, uint worldSize) { } [Fact] - public void TestInvalidLobbyDoesntConsumeId() { + public void TestInvalidLobbyDoesNotConsumeId() { _lobbyManager.CreateLobby(99, WORLD_SIZE, Player(1), out _).ShouldBe(LobbyActionResult.InvalidParameters); NewLobby(2, Player(1)).Id.ShouldBe(1u); @@ -188,7 +188,7 @@ public void TestLobbyStartsWhenFullAndReady() { } [Fact] - public void TestLobbyDoesntStartWhenNotFull() { + public void TestLobbyDoesNotStartWhenNotFull() { var lobby = NewLobby(3, Player(1)); _lobbyManager.JoinLobby(lobby.Id, Player(2)); @@ -201,7 +201,7 @@ public void TestLobbyDoesntStartWhenNotFull() { } [Fact] - public void TestJoiningDoesntStartLobby() { + public void TestJoiningDoesNotStartLobby() { var lobby = NewLobby(2, Player(1)); _lobbyManager.SetReady(lobby.Id, 1, true);