From 35c2ad2b351fa9ea27e0c9262d63d7dd647e0595 Mon Sep 17 00:00:00 2001 From: themuffinator Date: Thu, 24 Sep 2026 19:18:56 +0100 Subject: [PATCH] Fix retail spectator key-activity button --- code/client/cl_input.cpp | 3 +- code/qcommon/q_shared.h | 5 ++- docs/fnql/CHANGELOG.md | 2 +- docs/fnql/INPUT_COMPATIBILITY.md | 40 ++++++++++++++++++++++++ docs/fnql/RELEASE_COMPLETION.md | 6 ++++ tests/message_codec_tests.cpp | 53 ++++++++++++++++++++++++++++++++ tests/protocol_layout_tests.cpp | 6 ++++ 7 files changed, 112 insertions(+), 3 deletions(-) diff --git a/code/client/cl_input.cpp b/code/client/cl_input.cpp index 8b24c21..1377f54 100644 --- a/code/client/cl_input.cpp +++ b/code/client/cl_input.cpp @@ -1097,7 +1097,8 @@ static void CL_CmdButtons( usercmd_t *cmd ) { // allow the game to know if any key at all is // currently pressed, even if it isn't bound to anything if ( anykeydown && !gameplayInputCaptured ) { - cmd->buttons |= BUTTON_ANY; + cmd->buttons |= clc.netchan.wireProfile == NETCHAN_WIRE_QL_RETAIL + ? BUTTON_ANY : BUTTON_ANY_Q3; } } diff --git a/code/qcommon/q_shared.h b/code/qcommon/q_shared.h index 8707c0d..a30d8de 100644 --- a/code/qcommon/q_shared.h +++ b/code/qcommon/q_shared.h @@ -1322,7 +1322,10 @@ typedef struct playerState_s { #define BUTTON_PATROL 512 #define BUTTON_FOLLOWME 1024 -#define BUTTON_ANY 2048 // any key whatsoever +// Retail CL_CmdButtons (0x004B5C67) uses bit 12 for key activity. Quake III's +// bit 11 instead requests stop-follow in retail SpectatorThink (0x10033F82). +#define BUTTON_ANY 4096 // any key whatsoever +#define BUTTON_ANY_Q3 2048 // retained Quake III wire profiles #define MOVE_RUN 120 // if forwardmove or rightmove are >= MOVE_RUN, // then BUTTON_WALKING should be set diff --git a/docs/fnql/CHANGELOG.md b/docs/fnql/CHANGELOG.md index ab2f525..ce7159e 100644 --- a/docs/fnql/CHANGELOG.md +++ b/docs/fnql/CHANGELOG.md @@ -28,7 +28,7 @@ release, CI resets `Unreleased` for the next cycle. - _None yet._ ### Fixes -- _None yet._ +- Fixed scoreboard key presses unexpectedly leaving spectator follow mode. ### Documentation and Tooling - _None yet._ diff --git a/docs/fnql/INPUT_COMPATIBILITY.md b/docs/fnql/INPUT_COMPATIBILITY.md index 3344646..7257437 100644 --- a/docs/fnql/INPUT_COMPATIBILITY.md +++ b/docs/fnql/INPUT_COMPATIBILITY.md @@ -71,6 +71,46 @@ unchanged when neither correction applies. ## Usercmd sampling and stateful commands +### Key activity and spectator follow + +Retail Quake Live uses `0x1000` for the engine's `BUTTON_ANY` key-activity +flag. The inherited Quake III value, `0x0800`, is a separate stop-follow +input in retail QL. Sending it for every physical key press made `+scores` +bindings leave follow mode ([issue #5](https://github.com/themuffinator/FnQL/issues/5)). +FnQL now emits the retail key-activity value for protocol 91 connections. +Retained Quake III/ioquake3 wire profiles continue to use `BUTTON_ANY_Q3` +(`0x0800`), preserving their existing key-activity behavior. + +Observed evidence, checked against the legitimate Steam build `1168251` on +2026-09-24 and the [QLSRP reference corpus](https://github.com/themuffinator/QL-SRP): + +- `quakelive_steam.exe` `CL_CmdButtons` at `0x004B5BD0` masks catcher bit + `0x10` out for both `BUTTON_TALK` and key activity. At `0x004B5C67`, it + ORs `0x1000` into `usercmd_t.buttons`. Its SHA-256 is + `c926fe9f6c851e00b3b9332e88903ad01f28fdd60454873891c0158f5ded1299`. +- Retail `bin.pk3`'s `qagamex86.dll` `SpectatorThink` tests a rising `0x0800` + edge at `0x10033F82`/`0x10033F89`, then calls `StopFollowing` at + `0x10033FB1`. Its SHA-256 is + `9bfad1b5df4cbbb3fcfb20781024fc5c0abe73ba8389c2f20be8c0a55552e83d`. +- QLSRP's reconstructed `q_shared.h` still defines `BUTTON_ANY` as `2048`; + the retail instructions above take precedence over that source definition. + +These facts explain the reported distinction between console commands and +physical bindings: console `+scores` does not create a key-activity edge. +Holding another key already asserts the old flag, so the scoreboard key does +not create another rising edge. This explanation is an inference from the +report and the retail instructions, not a Proton runtime reproduction. + +The correction keeps the scoreboard's `0x10` catcher transparent to movement, +preserves held-key state and command routing, and leaves explicit button bits +and their wire encoding intact. The ABI assertion and successive usercmd codec +tests reject the old value and cover press/hold/release, attack/walk chords, +chat, the separate `0x0800` input, and the legacy key-activity value. +Existing scoreboard/input tests protect +the catcher behavior. Interactive Proton/Wayland validation remains pending. + +### Sampling and command delivery + Relative mouse deltas do not carry timestamps in the engine ABI. When usercmd generation is suspended before a gamestate, while disconnected, or by a local pause, FnQL therefore discards deltas collected during that unsampleable gap diff --git a/docs/fnql/RELEASE_COMPLETION.md b/docs/fnql/RELEASE_COMPLETION.md index cfa62ca..29b596c 100644 --- a/docs/fnql/RELEASE_COMPLETION.md +++ b/docs/fnql/RELEASE_COMPLETION.md @@ -23,6 +23,12 @@ only planned. ## Ready For Changelog +- [x] Correct the engine's key-activity button to retail Quake Live's `0x1000`, + preventing scoreboard key presses from emitting the separate `0x0800` + stop-follow input while retaining Quake III wire-profile behavior. + Retail executable/module inspection and ABI/usercmd + regression tests cover the correction; interactive Proton validation is + still pending. See [input compatibility](INPUT_COMPATIBILITY.md#key-activity-and-spectator-follow). - [x] Make pending WebUI avatars use a steady-clock retry backoff and immediate availability hints from Steam, with four distinct image paths per frame. Duplicate requests share provider/PNG work and successful buffers; navigation diff --git a/tests/message_codec_tests.cpp b/tests/message_codec_tests.cpp index d25193e..b183ffc 100644 --- a/tests/message_codec_tests.cpp +++ b/tests/message_codec_tests.cpp @@ -88,6 +88,58 @@ int TestWireProfileUserCommandHash() { return 0; } +int TestRetailAnyKeyButtons() { + struct ButtonSample { + int buttons; + int expected; + }; + // A scoreboard/unbound key must report activity without asserting retail's + // stop-follow bit (0x0800). Exercise presses, holds, releases, chords, and + // an explicit stop-follow bit across successive delta baselines. + const ButtonSample samples[] = { + { 0, 0 }, + { BUTTON_ANY, 0x1000 }, + { BUTTON_ANY, 0x1000 }, + { 0, 0 }, + { BUTTON_ANY | BUTTON_ATTACK, 0x1001 }, + { BUTTON_ANY | BUTTON_WALKING, 0x1010 }, + { BUTTON_ANY, 0x1000 }, + { 0, 0 }, + { 0x0800, 0x0800 }, + { BUTTON_ANY | 0x0800, 0x1800 }, + { BUTTON_TALK, 0x0002 }, + { 0, 0 }, + // Retained Quake III/ioquake3 connections keep their original flag. + { BUTTON_ANY_Q3, 0x0800 }, + { BUTTON_ANY_Q3 | BUTTON_ATTACK, 0x0801 }, + { BUTTON_ANY_Q3 | BUTTON_WALKING, 0x0810 }, + { 0, 0 }, + }; + std::array storage{}; + usercmd_t baseline{}; + for ( const ButtonSample& sample : samples ) { + usercmd_t sent = baseline; + sent.serverTime += 16; + sent.buttons = sample.buttons; + usercmd_t received{}; + msg_t writer{}; + MSG_Init( &writer, storage.data(), static_cast( storage.size() ) ); + MSG_WriteDeltaUsercmdKey( &writer, 0x10203040, &baseline, &sent ); + CHECK( !writer.overflowed ); + + msg_t reader{}; + MSG_Init( &reader, storage.data(), static_cast( storage.size() ) ); + reader.cursize = writer.cursize; + MSG_BeginReading( &reader ); + MSG_ReadDeltaUsercmdKey( &reader, 0x10203040, &baseline, &received ); + CHECK( received.buttons == sample.expected ); + CHECK( received.serverTime == sent.serverTime ); + CHECK( reader.readcount <= reader.cursize ); + baseline = received; + } + return 0; +} + int TestWireProfileCommandStrings() { static const char command[] = { 'p', 'r', 'i', 'n', 't', ' ', '"', 'c', 'a', 'f', '\xc3', '\xa9', ' ', '%', '"', '\0' }; @@ -286,6 +338,7 @@ int TestPlayerStateRoundTrip() { int main() { if ( const int result = TestBounds() ) return result; if ( const int result = TestWireProfileUserCommandHash() ) return result; + if ( const int result = TestRetailAnyKeyButtons() ) return result; if ( const int result = TestWireProfileCommandStrings() ) return result; if ( const int result = TestUserCommandRoundTrip() ) return result; if ( const int result = TestUserCommandClockWrap() ) return result; diff --git a/tests/protocol_layout_tests.cpp b/tests/protocol_layout_tests.cpp index bbe603e..888e205 100644 --- a/tests/protocol_layout_tests.cpp +++ b/tests/protocol_layout_tests.cpp @@ -12,6 +12,12 @@ static_assert( offsetof( usercmd_t, forwardmove ) == 23 ); static_assert( offsetof( usercmd_t, rightmove ) == 24 ); static_assert( offsetof( usercmd_t, upmove ) == 25 ); +// Retail CL_CmdButtons (0x004B5C67) emits bit 12 for key activity. +// Bit 11 is a separate stop-follow input in retail SpectatorThink. +static_assert( BUTTON_ANY == 0x1000 ); +static_assert( ( BUTTON_ANY & 0x0800 ) == 0 ); +static_assert( BUTTON_ANY_Q3 == 0x0800 ); + static_assert( sizeof( trajectory_t ) == 40 ); static_assert( offsetof( trajectory_t, gravity ) == 36 );