From a30f657964907622145c53120b8cdeabea05ebab Mon Sep 17 00:00:00 2001 From: themuffinator Date: Thu, 3 Sep 2026 12:39:35 +0100 Subject: [PATCH 1/2] Stop a client authoring the clock its own shots are judged against usercmd.gameTime arrives from the client and reaches the game unchanged, where anything that measures a command's age against the server clock becomes a dial the client can turn. Overwriting it with the frame index the server already filed the command under is a no-op for an honest client - that is exactly what idAsyncClient sends - and it removes the dial permanently. Staleness detection is unaffected: a slot that was never received still holds a stamp from a whole backup window ago, so DuplicateUsercmd still fires. Add tools/tests/mp_lag_compensation_contract.py, and register it in the two workflows and the local validation profile. Lag compensation was pinned by nothing in either repository, which is how it came to ship with a rewind that could not measure what it was for. The contract pins three agreements: the estimate must not be derived from the command's age, the hit target must be restored before anything acts on the hit, and a discontinuity must invalidate the history. It also pins net_mpLagCompensation at 0, so the default cannot be flipped by accident - only deliberately, by editing the assertion with it. Pairs with openQ4-game "Make the lag-compensation rewind measure something, and put the target back". Co-Authored-By: Claude Opus 5 --- .github/workflows/commit-validation.yml | 2 + .github/workflows/push-verification.yml | 2 + src/framework/async/AsyncServer.cpp | 7 + tools/tests/mp_lag_compensation_contract.py | 229 ++++++++++++++++++++ tools/validation/openq4_validate.py | 1 + 5 files changed, 241 insertions(+) create mode 100644 tools/tests/mp_lag_compensation_contract.py diff --git a/.github/workflows/commit-validation.yml b/.github/workflows/commit-validation.yml index 6ea5486a0..85289b159 100644 --- a/.github/workflows/commit-validation.yml +++ b/.github/workflows/commit-validation.yml @@ -174,6 +174,7 @@ jobs: tools/tests/mp_bot_navigation.py \ tools/tests/mp_bot_server_menu.py \ tools/tests/mp_client_combat_effects.py \ + tools/tests/mp_lag_compensation_contract.py \ tools/tests/mp_weapon_switch_contract.py \ tools/tests/multiview_demo.py \ tools/tests/mvd_server_api_contract.py \ @@ -338,6 +339,7 @@ jobs: python tools/tests/mp_bot_navigation.py python tools/tests/mp_bot_server_menu.py python tools/tests/mp_client_combat_effects.py + python tools/tests/mp_lag_compensation_contract.py python tools/tests/mp_weapon_switch_contract.py python tools/tests/multiview_demo.py python tools/tests/mvd_server_api_contract.py diff --git a/.github/workflows/push-verification.yml b/.github/workflows/push-verification.yml index f5403cc9c..0173a9ac3 100644 --- a/.github/workflows/push-verification.yml +++ b/.github/workflows/push-verification.yml @@ -174,6 +174,7 @@ jobs: tools/tests/mp_bot_navigation.py \ tools/tests/mp_bot_server_menu.py \ tools/tests/mp_client_combat_effects.py \ + tools/tests/mp_lag_compensation_contract.py \ tools/tests/mp_weapon_switch_contract.py \ tools/tests/multiview_demo.py \ tools/tests/mvd_server_api_contract.py \ @@ -338,6 +339,7 @@ jobs: python tools/tests/mp_bot_navigation.py python tools/tests/mp_bot_server_menu.py python tools/tests/mp_client_combat_effects.py + python tools/tests/mp_lag_compensation_contract.py python tools/tests/mp_weapon_switch_contract.py python tools/tests/multiview_demo.py python tools/tests/mvd_server_api_contract.py diff --git a/src/framework/async/AsyncServer.cpp b/src/framework/async/AsyncServer.cpp index 4afa634c6..db06a8a86 100644 --- a/src/framework/async/AsyncServer.cpp +++ b/src/framework/async/AsyncServer.cpp @@ -1605,6 +1605,13 @@ void idAsyncServer::ProcessUnreliableClientMessage( int clientNum, const idBitMs return; } userCmds[index][clientNum].gameFrame = i; + // openQ4: gameTime is authored by the client and reaches the game unchecked, where + // anything that measures a command's age against the server clock becomes a dial + // the client can turn. An honest client sends exactly this value, so overwriting + // it costs nothing and closes the hole permanently. Staleness detection is + // unaffected: a slot that was never received still holds a stamp from a whole + // backup window ago. + userCmds[index][clientNum].gameTime = i * common->GetUserCmdMSec(); userCmds[index][clientNum].duplicateCount = 0; if ( idAsyncNetwork::UsercmdInputChanged( userCmds[( i - 1 ) & ( MAX_USERCMD_BACKUP - 1 )][clientNum], userCmds[index][clientNum] ) ) { client.lastInputTime = serverTime; diff --git a/tools/tests/mp_lag_compensation_contract.py b/tools/tests/mp_lag_compensation_contract.py new file mode 100644 index 000000000..ce23fb50c --- /dev/null +++ b/tools/tests/mp_lag_compensation_contract.py @@ -0,0 +1,229 @@ +#!/usr/bin/env python3 +"""Pins the agreements that make server-side lag compensation honest. + +Nothing in either repository asserted anything about lag compensation before this file, which is +how the feature came to ship with a rewind that could not measure what it was for. + +Three agreements are pinned here. + + * THE ESTIMATE MUST NOT COME FROM THE COMMAND'S AGE. `time - shooter->usercmd.gameTime` looks + like the end-to-end pipeline delay and is in fact a structural constant: idAsyncServer files a + client's command at THAT client's own frame index and executes the slot for the frame it is + running, and idGameLocal::RunFrame has already advanced `time` past it, so the difference is + one GetMSec() at any ping. A duplicated command is re-stamped with the server's own clock to + the same effect. The rewind must be derived from the client's reported prediction lead, + capped by the round trip the server measured for itself. + + * THE TARGET MUST BE PUT BACK BEFORE ANYTHING ACTS ON THE HIT. The trace resolves against the + rewound world, but the damage, the blood decal, the impact effect, the PVS areas that effect + is broadcast to and the ragdoll a corpse starts from all belong in the present. The gauntlet + and the lightning gun already restored before they damaged; the hit-scan path did not. + + * A DISCONTINUITY MUST INVALIDATE THE HISTORY. Rewinding onto a position a player teleported + away from, or onto the previous occupant of a recycled client slot, aims a shot at a place + nobody could have been. + +The multiplayer tree is the only one that matters here: src/game is game_sp and never runs +multiplayer. Its copy of this code is dead, and is checked only for defaults that would read as a +disagreement between the forks. +""" + +from __future__ import annotations + +import os +import sys +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[2] +GAME_LIBS_ROOT = Path( + os.environ.get("OPENQ4_GAMELIBS_REPO", ROOT.parent / "openQ4-game") +).resolve() + + +def read(root: Path, relative: str) -> str: + path = root / relative + if not path.is_file(): + raise AssertionError(f"Required file not found: {path}") + return path.read_text(encoding="utf-8", errors="replace") + + +def require(text: str, token: str, context: str) -> None: + if token not in text: + raise AssertionError(f"Missing {token!r} in {context}") + + +def reject(text: str, token: str, context: str) -> None: + if token in text: + raise AssertionError(f"Unexpected {token!r} in {context}") + + +def require_before(text: str, first: str, second: str, context: str) -> None: + require(text, first, context) + require(text, second, context) + if text.index(first) >= text.index(second): + raise AssertionError(f"Expected {first!r} before {second!r} in {context}") + + +def function_body(source: str, signature: str, context: str) -> str: + start = source.find(signature) + if start < 0: + raise AssertionError(f"Missing {signature!r} in {context}") + opening = source.find("{", start + len(signature)) + if opening < 0: + raise AssertionError(f"Missing body for {signature!r} in {context}") + depth = 0 + for index in range(opening, len(source)): + if source[index] == "{": + depth += 1 + elif source[index] == "}": + depth -= 1 + if depth == 0: + return source[start : index + 1] + raise AssertionError(f"Unbalanced body for {signature!r} in {context}") + + +def validate_estimator() -> None: + game_local = read(GAME_LIBS_ROOT, "src/mpgame/Game_local.cpp") + estimator = function_body( + game_local, + "bool idGameLocal::ComputeMPLagCompensationRewind(", + "MP lag compensation estimator", + ) + label = "MP lag compensation estimator" + + # The command-age estimate cannot measure anything; see the module docstring. Rejected as the + # assignment rather than as the bare symbol, so the comment explaining why it is wrong is + # allowed to name it. + reject(estimator, "rewindEstimateMS = time - shooter->usercmd.gameTime", label) + reject(estimator, "= pingMS / 2", label) + + require(estimator, "ServerGetClientPrediction", label) + require(estimator, "ServerGetClientPing", label) + # Both report a 99999 sentinel for a slot that is not fully connected, and the reported lead is + # a client-authored short, so it needs clamping as well as the sentinel check. + require(estimator, "99999", label) + require(estimator, "idMath::ClampInt( 0, 1000, reported )", label) + require(estimator, "net_mpLagCompSlackMS", label) + require(estimator, "RemoteExtrapolationMS()", label) + require(estimator, "net_mpLagCompMaxMS", label) + + # Lag compensation and client-side remote extrapolation correct the same error from opposite + # ends. Applying both at full strength inverts the shooter's lead rather than removing it, so + # the coupling has to stay visible in the code. + coupling = function_body( + game_local, + "int idGameLocal::RemoteExtrapolationMS(", + "remote extrapolation coupling", + ) + require(coupling, "return 0;", "remote extrapolation coupling") + require(game_local, "net_mpPredictMode", "remote extrapolation coupling rationale") + + +def validate_hit_is_resolved_in_the_present() -> None: + game_local = read(GAME_LIBS_ROOT, "src/mpgame/Game_local.cpp") + label = "MP lag compensation hit resolution" + + resolve = function_body( + game_local, + "bool idGameLocal::ResolveMPLagCompensationHit(", + label, + ) + require(resolve, "restore.originalOrigin - restore.rewoundOrigin", label) + require(resolve, "tr.c.point += shift;", label) + require(resolve, "tr.endpos += shift;", label) + require(resolve, "tr.c.dist = tr.c.normal * tr.c.point;", label) + require(resolve, "restoreState[ i ] = restoreState[ --restoreCount ];", label) + + hitscan = function_body(game_local, "idEntity* idGameLocal::HitScan(", label) + require(hitscan, "ResolveMPLagCompensationHit(", label) + # The target must be back in the present before ANY of these run against the hit. + for consumer in ( + "collisionArea = pvs.GetPVSArea( tr.c.point )", + "ent->Damage(", + "AddDamageEffect(", + ): + require_before(hitscan, "ResolveMPLagCompensationHit(", consumer, label) + + +def validate_invalidation() -> None: + label = "MP lag compensation history invalidation" + player = read(GAME_LIBS_ROOT, "src/mpgame/Player.cpp") + network = read(GAME_LIBS_ROOT, "src/mpgame/Game_network.cpp") + + # A respawn was the only invalidation site the feature shipped with. + spawn = function_body(player, "void idPlayer::SpawnToPoint(", label) + require(spawn, "InvalidateMPLagCompensationHistory", label) + + # A teleport does not route through SpawnToPoint. + teleport = function_body( + player, + "void idPlayer::Teleport( const idVec3 &origin, const idAngles &angles, idEntity *destination )", + label, + ) + require(teleport, "InvalidateMPLagCompensationHistory", label) + + # A recycled slot must not be rewound onto its previous occupant's positions. + disconnect = function_body( + network, "void idGameLocal::ServerClientDisconnect(", label + ) + require(disconnect, "InvalidateMPLagCompensationHistory", label) + + +def validate_usercmd_stamp_is_server_authored() -> None: + label = "usercmd gameTime sanitisation" + async_server = read(ROOT, "src/framework/async/AsyncServer.cpp") + process = function_body( + async_server, + "void idAsyncServer::ProcessUnreliableClientMessage(", + label, + ) + # gameTime arrives client-authored and reaches the game unchecked. An honest client sends + # exactly this value, so overwriting it costs nothing and removes a client-controlled dial on + # anything that measures a command against the server clock. + require( + process, + "userCmds[index][clientNum].gameTime = i * common->GetUserCmdMSec();", + label, + ) + require_before( + process, + "userCmds[index][clientNum].gameFrame = i;", + "userCmds[index][clientNum].gameTime = i * common->GetUserCmdMSec();", + label, + ) + + +def validate_defaults() -> None: + label = "MP lag compensation defaults" + mp_cvar = read(GAME_LIBS_ROOT, "src/mpgame/gamesys/SysCvar.cpp") + + # Off until the rewind has been measured against a real server, and because it must not be on + # at the same time as full remote extrapolation - the two corrections invert rather than + # compose. A deliberate flip changes this line and this assertion together. + require(mp_cvar, 'net_mpLagCompensation(', label) + line = next( + stripped + for stripped in mp_cvar.splitlines() + if "net_mpLagCompensation(" in stripped + ) + if '"0"' not in line: + raise AssertionError( + f"{label}: net_mpLagCompensation must ship at 0 until the rewind is measured; got: {line.strip()}" + ) + + require(mp_cvar, 'net_mpLagCompSlackMS(', label) + require(mp_cvar, 'net_mpLagCompMaxMS(', label) + + +def main() -> None: + validate_estimator() + validate_hit_is_resolved_in_the_present() + validate_invalidation() + validate_usercmd_stamp_is_server_authored() + validate_defaults() + print("mp_lag_compensation_contract: ok") + + +if __name__ == "__main__": + main() diff --git a/tools/validation/openq4_validate.py b/tools/validation/openq4_validate.py index 295636e7b..5814dc060 100644 --- a/tools/validation/openq4_validate.py +++ b/tools/validation/openq4_validate.py @@ -491,6 +491,7 @@ def run_python_tests(args: argparse.Namespace, root: Path, env: dict[str, str]) root / "tools" / "tests" / "mp_bot_navigation.py", root / "tools" / "tests" / "mp_bot_server_menu.py", root / "tools" / "tests" / "mp_client_combat_effects.py", + root / "tools" / "tests" / "mp_lag_compensation_contract.py", root / "tools" / "tests" / "mp_weapon_switch_contract.py", root / "tools" / "tests" / "multiview_demo.py", root / "tools" / "tests" / "mvd_server_api_contract.py", From 235c645ea1c8e3693a34190899d7ea5815b90c40 Mon Sep 17 00:00:00 2001 From: themuffinator Date: Thu, 3 Sep 2026 13:36:08 +0100 Subject: [PATCH 2/2] Move the CI GameLibs pin to the merged multiplayer work mp_lag_compensation_contract.py asserts code that exists only in openQ4-game 6804902, so the pin has to move with it or the engine's cross-repo tests would assert contracts against a tree that predates them. 6804902 is the tree these engine commits were measured against, and it carries all three merged multiplayer changes: the projectile prestep (#11), remote-player re-simulation (#12) and the lag-compensation rewind (#14). Co-Authored-By: Claude Opus 5 --- .github/workflows/commit-validation.yml | 2 +- .github/workflows/push-verification.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/commit-validation.yml b/.github/workflows/commit-validation.yml index 85289b159..c7faeeb18 100644 --- a/.github/workflows/commit-validation.yml +++ b/.github/workflows/commit-validation.yml @@ -15,7 +15,7 @@ env: # Keep the engine's shared File/FileSystem ABI locked to the companion # revision that implements GAME_API 46, generated-animation cache v3, and # the game-owned transient time-scale interface. - OPENQ4_GAMELIBS_SHA: f5596b6cea3569533375fff6ee4ff1ff8f4d3ff0 + OPENQ4_GAMELIBS_SHA: 680490234f018432d04ef5ce9a9d3e11a06b1f7d jobs: script-smoke: diff --git a/.github/workflows/push-verification.yml b/.github/workflows/push-verification.yml index 0173a9ac3..1a63b8fe9 100644 --- a/.github/workflows/push-verification.yml +++ b/.github/workflows/push-verification.yml @@ -11,7 +11,7 @@ env: # Keep the engine's shared File/FileSystem ABI locked to the companion # revision that implements GAME_API 46, generated-animation cache v3, and # the game-owned transient time-scale interface. - OPENQ4_GAMELIBS_SHA: f5596b6cea3569533375fff6ee4ff1ff8f4d3ff0 + OPENQ4_GAMELIBS_SHA: 680490234f018432d04ef5ce9a9d3e11a06b1f7d concurrency: group: openq4-push-verification-${{ github.ref }}