From 078d576383db0d5caf6d9004e597d917ce0d9a7d Mon Sep 17 00:00:00 2001 From: Alexa <53471801+Alexa-RR@users.noreply.github.com> Date: Tue, 28 Jul 2026 11:20:39 +0300 Subject: [PATCH] =?UTF-8?q?fix:=20stop=20sending=20a=20room-level=20DataBl?= =?UTF-8?q?ob=20=E2=80=94=20it=20broke=20the=20dorm?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding DataBlob/DataBlobHash at room level for reader parity was wrong, and the dorm is what proved it: rooms came back "the data in the room you are trying to load is corrupt", and every room loaded fine before the key existed. Which blob a room loads is decided per INSTANCE and per SUB-ROOM, not per room. The matchmake response carries it (a dorm's is per-player, dorm_p{id}_v*.dat) and each sub-room carries its own. A room-scoped answer to the same question is one the client preferred, and for a dorm it named a file that was never written. A registered slot does not oblige us to fill it: Newtonsoft leaves an absent key at its default, which is the state every room loaded in. The parity test records both keys as deliberately omitted with the reason, so restoring them takes a decision rather than a tidy-up. Also drops the room-level/sub-room DataBlob invariant test added an hour ago — there is no room-level DataBlob left for it to check. Note for the clone bug: the captured response now shows the clone carries all 41 keys the reader registers and the client still rejects it, so key parity is NOT that failure. It is a value the reader refuses, not a missing name. --- DorkNet.Server.Tests/ClientDtoParityTests.cs | 82 +++++-------------- .../API/Rooms/V2/RoomsController.cs | 24 ++++-- .../Startup/RequestTracingExtensions.cs | 2 +- 3 files changed, 38 insertions(+), 70 deletions(-) diff --git a/DorkNet.Server.Tests/ClientDtoParityTests.cs b/DorkNet.Server.Tests/ClientDtoParityTests.cs index 5eea65f..6105420 100644 --- a/DorkNet.Server.Tests/ClientDtoParityTests.cs +++ b/DorkNet.Server.Tests/ClientDtoParityTests.cs @@ -38,6 +38,25 @@ public sealed class ClientDtoParityTests : IClassFixture /// POST thread/withmembers and GET thread/{id}. private const string ChatThreadReader = "EKEFFNBIOHJ"; + /// Keys the reader registers that we deliberately do NOT send. + /// + /// A registered slot does not oblige us to fill it — Newtonsoft leaves an + /// absent key at its default. These two were added for parity and broke + /// the dorm: which blob a room loads is decided per INSTANCE (the matchmake + /// response carries it, and a dorm's is per-player, dorm_p{id}_v*.dat) and + /// per SUB-ROOM. A room-scoped answer to the same question is one the + /// client preferred, and for a dorm it named a file that was never written, + /// so the room came back "corrupt". + /// + /// Every room loaded fine while these were absent. Their absence is the + /// fix, not a gap to close later — do not "restore parity" here without a + /// dorm join to prove it. + private static readonly HashSet<(string Reader, string Key)> DeliberatelyOmitted = + [ + (RoomDetailsReader, "DataBlob"), + (RoomDetailsReader, "DataBlobHash"), + ]; + [Fact] public async Task Room_details_carries_exactly_the_keys_the_client_reads() { @@ -91,72 +110,15 @@ public sealed class ClientDtoParityTests : IClassFixture AssertKeyParity(ChatThreadReader, body, "POST thread/withmembers"); } - /// The room-level DataBlob must be the SAME file sub-room 0 points - /// at. The client loads a room from one blob, and if the two disagree it - /// fetches a file that was never written and rejects the whole room with - /// "the data in the room you are trying to load is corrupt" — which reads - /// like a broken save rather than a mismatched payload. - /// - /// A dorm is where this bites: its data is per-PLAYER - /// (dorm_p{id}_v*.dat) and reaches the builder as an override, while - /// the room-level name is per-ROOM (room_116_*). Reporting the - /// room-level one hands the client another player's dorm. - [Fact] - public async Task Room_level_data_blob_matches_sub_room_zero() - { - using var client = RoomsClient(); - var session = await GameClientSessionFactory.CreateAsync(client, _factory.ApexDomain); - client.DefaultRequestHeaders.Authorization = - new AuthenticationHeaderValue("Bearer", session.AccessToken); - - var roomId = 9_900_000 + Random.Shared.Next(1, 99_999); - await using (var scope = _factory.Services.CreateAsyncScope()) - { - var db = scope.ServiceProvider.GetRequiredService(); - db.Rooms.Add(new RoomEntity - { - Id = roomId, - Name = $"Blob{Guid.NewGuid():N}"[..18], - CreatorPlayerId = session.PlayerId, - CurrentDataBlobName = $"room_{roomId}_v1.dat", - }); - db.RoomScenes.Add(new RoomSceneEntity - { - RoomId = roomId, - Name = "Home", - OrderIndex = 0, - DataBlobName = $"room_{roomId}_home_v3.dat", - }); - await db.SaveChangesAsync(); - } - - using var response = await client.GetAsync($"/rooms/{roomId}"); - var body = await response.Content.ReadAsStringAsync(); - Assert.True(response.IsSuccessStatusCode, $"GET /rooms/{roomId} -> {(int)response.StatusCode}: {body}"); - - var details = JsonDocument.Parse(body).RootElement; - var roomLevel = details.GetProperty("DataBlob").GetString(); - var subRoomZero = details.GetProperty("SubRooms").EnumerateArray() - .First(e => e.GetProperty("SubRoomId").GetInt64() == 0) - .GetProperty("DataBlob").GetString(); - - Assert.True(roomLevel == subRoomZero, - $""" - The room reports one data blob and sub-room 0 reports another, so - which file the client loads depends on which key it reads first: - - room level : {roomLevel} - sub-room 0 : {subRoomZero} - """); - } - /// Compare one response object's keys against the reader's set. /// Matching is case-insensitive because the readers register three spellings /// per field (Pascal, camel, lower) and accept any of them — casing is the /// one thing they are NOT strict about. private static void AssertKeyParity(string readerType, string body, string what) { - var expected = LoadReaderKeys(readerType); + var expected = LoadReaderKeys(readerType) + .Where(k => !DeliberatelyOmitted.Contains((readerType, k))) + .ToList(); var actual = JsonDocument.Parse(body).RootElement .EnumerateObject().Select(p => p.Name) .ToHashSet(StringComparer.OrdinalIgnoreCase); diff --git a/DorkNet.Server/Controllers/API/Rooms/V2/RoomsController.cs b/DorkNet.Server/Controllers/API/Rooms/V2/RoomsController.cs index 18c9ec9..23bfe49 100644 --- a/DorkNet.Server/Controllers/API/Rooms/V2/RoomsController.cs +++ b/DorkNet.Server/Controllers/API/Rooms/V2/RoomsController.cs @@ -2158,15 +2158,21 @@ public class RoomsController( // already answered 200 to. Kept last so the additions are visible // against the reader's own list; key ORDER does not matter to a // name-keyed reader, presence does. - // Must be the SAME blob sub-room 0 resolves to, override included. - // A dorm's data is per-player (dorm_p{id}_v*.dat) and arrives here - // as overrideDataBlobName; the room-level name is per-ROOM - // (room_116_*), so reporting that instead hands the client another - // player's dorm — or one that was never written — and it rejects - // the load outright with "the data in the room you are trying to - // load is corrupt". - DataBlob = subRooms.Length > 0 ? primarySubRoomDataBlob : dataBlobName, - DataBlobHash = (string?)null, + // DataBlob / DataBlobHash are deliberately NOT sent at room level, + // even though the reader registers them. + // + // The room a player actually loads is chosen per INSTANCE: the + // matchmake response carries the blob (a dorm's is per-player, + // dorm_p{id}_v*.dat), and each sub-room carries its own. Adding a + // room-level name on top gave the client a second, room-scoped + // answer for the same question, and it loaded that one — a file + // that for a dorm was never written — then rejected the room with + // "the data in the room you are trying to load is corrupt". + // + // A registered slot does NOT oblige us to fill it: Newtonsoft + // leaves an absent key at its default, which is the state every + // room loaded fine in. Sending it is what broke the dorm, so the + // absence here is the fix, not an omission to tidy up later. // The room-level cap is the room's own advertised capacity, which is // what RoomEntity.MaxCapacity exists for and what matchmaking hands // to RoomInstance.MaxCapacity. Each sub-room carries its own diff --git a/DorkNet.Server/Startup/RequestTracingExtensions.cs b/DorkNet.Server/Startup/RequestTracingExtensions.cs index 8a308e9..d3653b7 100644 --- a/DorkNet.Server/Startup/RequestTracingExtensions.cs +++ b/DorkNet.Server/Startup/RequestTracingExtensions.cs @@ -264,7 +264,7 @@ public static class RequestTracingExtensions { var configured = Environment.GetEnvironmentVariable("Trace__SuccessBodyPaths"); if (configured is null) - return ["/player", "/thread", "/clone"]; + return ["/player", "/thread", "/clone", "/rooms/", "/matchmake/"]; return configured .Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)