fix(servers): filter empty-name A2S entries so first-player ctxmenu works (#1396) - #1401
Merged
Merged
Conversation
…orks (#1396) Some Source-engine variants and SourceMod plugins emit a "host slot" / "console" entry at the start of `GetPlayers` with `Name = ''`, `Frags = 0`, `Time = 0`. Pre-fix `api_servers_host_players` passed the entry through to `player_list` verbatim, and the JS rendered it as a phantom `<li data-testid="server-player">` above the first named player — an invisible thin row with a misleading "0 · " meta on the right and no `data-context-menu` hooks (the empty name fails the SteamID match gate). Users perceived the next real player as "the first player of the list", and right-clicks landing in the phantom row's area silently no-op'd. The user's screenshot showed "Fletcher" as the first visible player with no menu opening; the menu opened normally on every other player below him. The fix is one line inside the `$playerList` build loop: skip entries with `name === ''`, matching the same gate the SteamID- by-name lookup already applies upstream. The redundant `&& $name !== ''` guard in the SteamID match gate is removed since the loop now skips empty names entirely. The JS contract stays simple: every row in `player_list` has a displayable name. Regression coverage: - `ServersTest::testHostPlayersFiltersEmptyNameEntries` — handler receives an A2S response with an empty-name entry first; asserts `player_list` has 2 rows (not 3), Fletcher is at index 0 with the steamid the menu needs, no row has an empty name. - `ServersTest::testHostPlayersFiltersAllEmptyNameEntries` — all-empty-name A2S response yields an empty `player_list`; the A2S-reported `players` count is independent of the rendered list. - `server-player-context-menu.spec.ts` — new E2E test using real `page.mouse.click({button: 'right'})` against the visual centre of the first rendered row, asserting the menu opens with Fletcher's SteamID in the kick URL (not the second player's). Both PHP tests fail on 568c46b (proved before applying the fix: "actual size 3 matches expected size 2" / "expected size 0").
…-context-menu # Conflicts: # AGENTS.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1396.
Summary
GetPlayerswithName = '',Frags = 0,Time = 0. Pre-fixapi_servers_host_playerspassed it through verbatim toplayer_list, and the JS rendered a phantom<li data-testid="server-player">above the first named player (the empty name fails the SteamID-match gate, so nodata-context-menuhooks; visually a thin border-bottom strip with a misleading "0 · " meta on the right). Users perceived the next real player as "the first player of the list" and right-clicks landing in the phantom row silently no-op'd. The reporter's screenshot showed "Fletcher" as the first visible player with the menu only opening on "kovka" below him.continueinside the$playerListbuild loop inapi_servers_host_players, matching the same$name === ''skip the SteamID-by-name lookup already applies upstream. The redundant&& $name !== ''guard in the SteamID match gate is removed (the loop now skips empty names entirely). The JS contract stays simple: every row inplayer_listhas a displayable name. Bots, real players whose A2S name didn't match the RCON status output, and anonymous callers all still render — the filter is strictly "name is empty string".player_listcontract under the existing "Add or extend the server-player right-click context menu" row in "Where to find what".Test plan
Quality gates run locally before pushing:
./sbpp.sh phpstan— OK, no errors./sbpp.sh test— 643 tests, 2610 assertions (includes 2 new regression tests + 1 reused, plus 15-testhost_playersgroup all green)./sbpp.sh ts-check— clean./sbpp.sh composer api-contract— no diffCI=1 ./sbpp.sh e2e— 265 passed (full suite; 277 skipped per per-spec project filters)New regression tests
web/tests/api/ServersTest.php::testHostPlayersFiltersEmptyNameEntries— A2S response with an empty-name entry at index 0 followed by Fletcher + kovka. Assertsplayer_listhas 2 rows (not 3), Fletcher is at index 0 with the SteamID the menu needs, no row has an empty name. Fails on 568c46b withactual size 3 matches expected size 2— proved before applying the fix.web/tests/api/ServersTest.php::testHostPlayersFiltersAllEmptyNameEntries— all-empty-name A2S response yields an emptyplayer_list; the A2S-reportedplayerscount is independent of the rendered list (a SourceMod plugin can legitimately inflate the count). Fails on 568c46b withactual size 3 matches expected size 0.web/tests/e2e/specs/flows/server-player-context-menu.spec.ts::first named player accepts right-click (#1396 end-to-end)— new E2E test using realpage.mouse.click({button: 'right'})against the visual centre of the first rendered row. Asserts the menu opens with Fletcher's SteamID in the kick URL (not the second player's). Pins the END-USER outcome — load-bearing coverage for the filter itself lives in the two PHP tests above.Files changed
web/api/handlers/servers.php— the fix (+19 lines, -1 line; +17 lines are the explanatory comment block above the loop).web/tests/api/ServersTest.php— two new regression tests.web/tests/e2e/specs/flows/server-player-context-menu.spec.ts— one new E2E test.AGENTS.md— contract documentation update in the existing right-click-context-menu row.