From 5c7e218c98fd52748eefa33d50a86db60b3bc4d8 Mon Sep 17 00:00:00 2001 From: Cyd Date: Mon, 10 Aug 2026 23:58:27 -0500 Subject: [PATCH] A room cannot mix builds, and the log stops hiding things Three guards, all of them paid for by an actual wasted afternoon. THE BUILD GUARD. kNetRevision is hand-maintained and only bumped when someone judges that a change alters the simulation, so two different builds normally carry the same revision and will race each other happily - which is how a 4.12.184 machine and a 4.12.187 machine sat in the same room all day. The exact build now travels as lobby data and member data, and it is checked in two places: - on JOIN, before the room is ever entered, with a dialog naming both builds. The owner's launch check would have caught it eventually, but only after everyone had picked a loadout and pressed go, and all it can do then is decline to start - which reads as the host's button being broken. - at LAUNCH, beside the revision check, so a member seated before the guard existed still cannot race. The room screen shows BUILD against an offending row, because a host is owed the reason as well as the refusal. An absent value counts as a mismatch: a build older than the key cannot be trusted to match. Both guards stay - the revision still refuses a mix known to simulate differently even where the build strings agree. THE DUPLICATE KEY WARNING. environ.ini is applied line by line, so a second copy of a key silently beats the first. A TARGETFPS added at the top of the file was overridden by the one the template ships further down, and the test it was written for looked as though it had failed rather than never having run. Now: "TARGETFPS is set twice - line 1 and line 8; the LATER one wins", naming the key and both lines, because which and where is the whole value. THE MISSING FACTS. TARGETFPS appeared nowhere in the log, so no run could be checked afterwards against what it was actually asked for, and interpolation only announced itself when switched off - there was no way to confirm from a log that it was on. Both are now stated outright, with a warning when a frame target differs from the physics rate AND interpolation is off, which is the combination that steps. Verified: the duplicate warning names lines 1 and 8 of a file carrying both, the frame line reads "144 fps, drawing on exact physics steps" with the mismatch note when interpolation is off, and "60 fps, drawing interpolated across the physics step" with no note when it is on. Co-Authored-By: Claude Opus 5 (1M context) --- RP_L4/RPL4.CPP | 31 +++++++++++++ RP_L4/RPL4ENVIRON.cpp | 51 +++++++++++++++++++++ RP_L4/RPL4LOBBY.cpp | 102 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 182 insertions(+), 2 deletions(-) diff --git a/RP_L4/RPL4.CPP b/RP_L4/RPL4.CPP index 95cc489..79a5bff 100644 --- a/RP_L4/RPL4.CPP +++ b/RP_L4/RPL4.CPP @@ -276,6 +276,37 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine DEBUG_STREAM << "L4CONTROLS=" << getenv("L4CONTROLS") << std::endl << std::flush; + // + // The frame target and whether drawing interpolates, stated outright. + // Both were previously invisible: TARGETFPS appeared nowhere in the log, + // so a run could not be checked afterwards against what it was actually + // asked for, and interpolation only announced itself when switched OFF. + // Between them that cost a wasted test run and an ambiguous one. + // + // The mismatch note matters because a frame target that is not the + // physics rate used to mean visibly stepped motion - it no longer does + // while interpolation is on, which is exactly why the log should say + // which of the two states it is in. + // + { + const char *interp = getenv("RP412INTERP"); + Logical blending = !(interp != NULL && atoi(interp) == 0); + const char *physics = getenv("RP412PHYSICSHZ"); + int fps = atoi(getenv("TARGETFPS")); + int hz = (physics != NULL) ? atoi(physics) : 0; + + DEBUG_STREAM << "Video: frame target " << fps << " fps, drawing " + << (blending ? "interpolated across the physics step" : "on exact physics steps") + << std::endl << std::flush; + + if (!blending && hz > 0 && fps != hz) + { + DEBUG_STREAM << "Video: " << fps << " fps against a " << hz + << " Hz physics step with interpolation off - motion will step" + << std::endl << std::flush; + } + } + #ifdef RP412_STEAM // // RP412STEAM=1 (environ.ini or a Steam launch) swaps the wire to diff --git a/RP_L4/RPL4ENVIRON.cpp b/RP_L4/RPL4ENVIRON.cpp index d449b8a..4d77597 100644 --- a/RP_L4/RPL4ENVIRON.cpp +++ b/RP_L4/RPL4ENVIRON.cpp @@ -687,9 +687,27 @@ void // int applied = 0; char line[1024]; + + // + // Keys already applied, so a second copy of one can be reported. The + // file is applied line by line, so a later line silently beats an + // earlier one - which is a genuinely expensive way to lose an evening: + // a TARGETFPS added at the top of this file was overridden by the one + // the template ships further down, and the test it was written for + // looked like it had failed rather than never having run. + // + // Named rather than counted: knowing WHICH key and which lines is the + // whole value. + // + char seen_keys[4096]; + int seen_length = 0; + seen_keys[0] = '\0'; + + int line_number = 0; const char *cursor = text; while (*cursor != '\0') { + ++line_number; int length = 0; while (cursor[length] != '\0' && cursor[length] != '\n' && length < (int) sizeof(line) - 1) @@ -721,6 +739,39 @@ void { continue; } + // + // Warn on a repeat before applying it, naming the key and both + // lines. The later value is the one that survives, which is worth + // stating outright rather than leaving to be deduced. + // + { + char key[128]; + int key_length = 0; + while (setting[key_length] != '\0' && setting[key_length] != '=' && + key_length < (int) sizeof(key) - 1) + { + key[key_length] = setting[key_length]; + ++key_length; + } + key[key_length] = '\0'; + + // entries are stored as "KEY\tLINE\n" + char needle[132]; + sprintf(needle, "\n%s\t", key); + const char *found = (seen_length > 0) ? strstr(seen_keys, needle) : NULL; + if (found != NULL) + { + DEBUG_STREAM << "Environ: " << key << " is set twice - line " + << atoi(found + strlen(needle)) << " and line " << line_number + << "; the LATER one wins\n" << std::flush; + } + else if (seen_length + key_length + 16 < (int) sizeof(seen_keys)) + { + seen_length += sprintf(seen_keys + seen_length, "\n%s\t%d", + key, line_number); + } + } + putenv(setting); ++applied; } diff --git a/RP_L4/RPL4LOBBY.cpp b/RP_L4/RPL4LOBBY.cpp index 48d5729..2c2d98a 100644 --- a/RP_L4/RPL4LOBBY.cpp +++ b/RP_L4/RPL4LOBBY.cpp @@ -23,6 +23,7 @@ void RPL4Lobby_PullRaceResults() { } #include "rpl4fe.h" #include "rpl4console.h" +#include "rpl4build.h" // generated: RP412_VERSION, for the build guard #include "..\munga_l4\l4steamtransport.h" #pragma pack(push, 8) @@ -68,6 +69,23 @@ namespace // this member picked Live Cam rather than a grid slot const char kCamKey[] = "cam"; + // + // The exact build, so a room cannot mix them. + // + // kNetRevision above is hand-maintained and only bumped when someone + // decides a change alters the simulation - which means two DIFFERENT + // builds normally carry the same revision and will happily race each + // other. That is fine when the difference really is cosmetic and + // disastrous when the judgement was wrong, and it is not a judgement + // anyone should have to make correctly every time. The patch number is + // the repository's commit count, so this compares the actual binary. + // + // Both guards stay: the revision still refuses a mix that is known to + // simulate differently even between builds that agree here, which + // matters for anyone hand-editing a version. + // + const char kBuildKey[] = "bld"; + // the owner's mission setup, shown to everyone in the room const char kMapKey[] = "mp"; const char kTimeKey[] = "td"; @@ -212,6 +230,9 @@ namespace // what this build simulates like, so a mismatched room cannot launch SteamMatchmaking()->SetLobbyMemberData(gLobby, kNetRevKey, kNetRevision); + // and which build it actually IS - see kBuildKey + SteamMatchmaking()->SetLobbyMemberData(gLobby, kBuildKey, RP412_VERSION); + //--------------------------------------------------------------- // Only the owner's menu decides the mission, so the owner also // publishes what it picked: the scenario (members need it to know @@ -220,8 +241,10 @@ namespace //--------------------------------------------------------------- if (IsOwner()) { - // members check this before they act on the owner's go + // members check these before they act on the owner's go, and + // a joiner checks the build before it even sits down SteamMatchmaking()->SetLobbyData(gLobby, kNetRevKey, kNetRevision); + SteamMatchmaking()->SetLobbyData(gLobby, kBuildKey, RP412_VERSION); SteamMatchmaking()->SetLobbyData(gLobby, kScenarioKey, RPL4FrontEnd_IsFootballSelected() ? "football" : "race"); SteamMatchmaking()->SetLobbyData(gLobby, kMapKey, @@ -277,6 +300,7 @@ namespace char team[32]; // football pick char position[16]; char netRev[8]; // simulation protocol revision + char build[24]; // the exact build, see kBuildKey Logical camera; // picked Live Cam (acted on for the host) Logical published; }; @@ -321,6 +345,9 @@ namespace strncpy(member->netRev, SteamMatchmaking()->GetLobbyMemberData(gLobby, member->id, kNetRevKey), sizeof(member->netRev) - 1); + strncpy(member->build, + SteamMatchmaking()->GetLobbyMemberData(gLobby, member->id, kBuildKey), + sizeof(member->build) - 1); member->camera = (atoi( SteamMatchmaking()->GetLobbyMemberData(gLobby, member->id, kCamKey)) != 0) ? True : False; @@ -608,7 +635,21 @@ namespace // who set Live Cam is still going to race - saying otherwise // here would be the room screen lying about the grid. // - if (member->camera && is_owner_row) + // + // A build the room cannot race with is worth saying before + // anything about loadouts. A joiner is now turned away at the + // door, so this should only ever show for a member who was + // already seated when the guard arrived - but the launch check + // still refuses on it, and a host is owed the reason. + // + if (member->published && member->build[0] != '\0' && + strcmp(member->build, RP412_VERSION) != 0) + { + sprintf(text, "BUILD %s", member->build); + DrawTextA(mem, text, -1, &row, + DT_RIGHT | DT_VCENTER | DT_SINGLELINE); + } + else if (member->camera && is_owner_row) { DrawTextA(mem, "LIVE CAM", -1, &row, DT_RIGHT | DT_VCENTER | DT_SINGLELINE); @@ -881,6 +922,19 @@ namespace << " simulates like rev '" << room.members[i].netRev << "', we are rev '" << kNetRevision << "'\n" << std::flush; } + // + // The exact build too. An empty string is an older + // build that predates the key and cannot be trusted to + // match either. + // + if (strcmp(room.members[i].build, RP412_VERSION) != 0) + { + all_same_build = False; + DEBUG_STREAM << "Lobby: " << room.members[i].name + << " is build '" + << (room.members[i].build[0] ? room.members[i].build : "(older)") + << "', we are '" << RP412_VERSION << "'\n" << std::flush; + } } if (all_published && all_same_build && room.memberCount >= 1) { @@ -1101,6 +1155,50 @@ int } gLobby = gCallLobby; gInLobby = True; + + // + // Refuse a room running a different build, and refuse it HERE rather + // than at launch. The owner's launch check would catch it, but only + // after everyone has picked a loadout and pressed go, and all it can do + // then is silently decline to start - which reads as the host's button + // being broken. Far better to say so on the way in. + // + // The owner publishes its build as lobby data, so this costs one read. + // An empty string means a host older than the key, which is equally a + // mismatch: it cannot be trusted to be this build. + // + { + const char *host_build = SteamMatchmaking()->GetLobbyData(gLobby, kBuildKey); + if (host_build == NULL || strcmp(host_build, RP412_VERSION) != 0) + { + DEBUG_STREAM << "Lobby: room is build '" + << ((host_build != NULL && host_build[0]) ? host_build : "(older)") + << "', we are '" << RP412_VERSION + << "' - leaving, everyone must run the same build\n" << std::flush; + SteamMatchmaking()->LeaveLobby(gLobby); + gInLobby = False; + + // + // Say so on screen, not just in the log. A player who is bounced + // out of a room with no explanation will try again, and again, + // and then report that joining is broken - which is precisely + // the afternoon this guard exists to prevent. + // + char said[256]; + sprintf(said, + "That race is running Red Planet %s.\n" + "You are running %s.\n\n" + "Everyone in a race has to be on the same build - the\n" + "simulation has to agree exactly. Swap to a matching\n" + "build and join again.", + (host_build != NULL && host_build[0]) ? host_build : "an older build", + RP412_VERSION); + MessageBoxA(main_window, said, "Different build", + MB_OK | MB_ICONINFORMATION); + return LobbyRoomLeft; + } + } + // answer only launches newer than anything already in the lobby const char *go = SteamMatchmaking()->GetLobbyData(gLobby, kGoKey); gLastGoNonce = (go != NULL) ? atoi(go) : 0;