Refactor: User/Player distinction - #5178
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
There are verified authorization/spoofing issues in the refactored frontend chat handling that can misattribute/privilege messages before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR refactors network/input handling to consistently distinguish Net users (connected humans/devices) from in-game players/factions, as groundwork for future “archon mode” (multiple users controlling one player).
Changes:
- Replace packet addressing and packet processing paths from “player index” semantics to NetUserId semantics (including new helpers like
get_local_user()/get_local_packet()). - Introduce and use
get_net_user_player_number(NetUserId)to map User → Player, updating many gameplay/front-end call sites accordingly. - Rename/refactor network structures and APIs to use “user” terminology (
net_user_info,max_users,network_user_active/name,network_is_host()).
File summaries
| File | Description |
|---|---|
| src/roomspace.c | Uses user-based packet lookup for roomspace input handling; switches local packet acquisition. |
| src/roomspace_prediction.c | Updates prediction paths to use local user/packet helpers and user-based history. |
| src/player_utils.c | Initializes local player’s user_id for non-network games. |
| src/player_data.h | Replaces packet_num with user_id in PlayerInfo. |
| src/packets.h | Refactors packet API to user-centric functions and updates processing function signatures. |
| src/packets.c | Converts packet processing pipeline from player-indexed to user-indexed processing. |
| src/packets_misc.c | Implements get_local_user() / get_local_packet() and redefines get_packet() to take NetUserId. |
| src/packets_input.c | Refactors dungeon input processing to take NetUserId and use user packets/history. |
| src/packets_cheats.c | Renames global cheats processing entrypoint for consistency. |
| src/net_resync.cpp | Uses network_is_host() for host checks. |
| src/net_matchmaking.c | Switches lobby JSON reporting to user-centric helpers. |
| src/net_main.h | Renames “player” concepts to “user” in net state/public types; adds SOLO_HUMAN_ID. |
| src/net_main.c | Renames internal storage and loops to max_users / user-centric naming. |
| src/net_lobby.c | Updates lobby flows to use max_users and user-centric checks. |
| src/net_input_lag.c | Updates lag logic to host checks and user-based packet history. |
| src/net_game.h | Renames exported network user helpers and adds user→player mapping API. |
| src/net_game.c | Adds network_is_host() and introduces net_user_player_number[] mapping used by get_net_user_player_number(). |
| src/net_exchange_gameplay.h | Refactors packet history and chat message APIs to be user-based. |
| src/net_exchange_gameplay.c | Moves gameplay chat + packet history storage/lookup to NetUserId. |
| src/net_exchange_common.h | Refactors chat send signature to use NetUserId. |
| src/net_exchange_common.c | Writes/reads chat sender as NetUserId and converts loops/host checks to max_users. |
| src/net_checksums.c | Refactors checksum comparison/update to use user packets and local packet helper. |
| src/main.cpp | Updates mouse light logic to use user-based packet/history access. |
| src/local_camera.c | Uses local user history and local packet helper for camera syncing. |
| src/frontmenu_specials.c | Switches UI packet usage to local packet helper. |
| src/frontmenu_options.c | Switches options UI packet usage to local packet helper. |
| src/frontmenu_net.c | Refactors frontend chat send/process to use NetUserId; updates net user info references. |
| src/frontmenu_ingame_tabs.c | Switches various GUI actions to use local packet helper; simplifies a spell helper signature. |
| src/frontmenu_ingame_evnt.c | Switches in-game event GUI packet usage to local packet helper. |
| src/frontend.cpp | Updates message bookkeeping and host-loss logic to user-based structures/mapping. |
| src/front_torture.c | Refactors torture frontend input to use local/user packets. |
| src/front_network.h | Updates frontend chat processing signature to NetUserId. |
| src/front_network.c | Refactors frontend chat flow and host checks to user semantics. |
| src/front_landview_multiplayer.c | Switches host checks and naming to user-centric helpers; refactors loops to NetUserId. |
| src/front_input.c | Updates in-game chat send + various packet accesses to local packet helper; updates host-loss logic. |
| src/console_cmd.h | Changes command execution API to accept NetUserId. |
| src/console_cmd.c | Converts console commands to accept NetUserId and map to PlayerNumber internally. |
Review details
- Files reviewed: 37/37 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Resolved Copilot threads. Ready for review from @rainlizard. |
|
Issues:
(I'm assuming that last one would be handled as a merge conflict but I don't know) |
|
The first one is a small fix (done). The last one is just a merge conflict yeah. Regarding the command API, yeah, it's a good point. I've reverted most of the changes there for now. I'll have to think about how to handle those commands in archon mode, but come to think, most of them will probably just work fine as is. Also, squashed. |
ce9e8b7 to
ba2350b
Compare
ba2350b to
4b49a89
Compare
|
|
That's probably almost everything solved. Did find this one:
Under the current one-user-per-player mapping, there is no behavioral consequence because
So it is a dormant correctness issue: harmless today, but likely to become a hard-to-diagnose multiplayer bug when the many-users-to-one-player mapping is introduced. |
|
This is really the kind of thing I was hoping to handle in follow-up PRs. There are still many instances in the codebase of user being derived from player number. All will be addressed eventually as archon mode materializes. For that to happen, the codebase needs to be in a position to accommodate small fixes that reorient the scattered player->user lookup toward the archon-correct user->player direction. So let's land this PR, then start grafting the smaller changes that will enable archon mode afterward. |
|
merging time @Loobinex |
User: a human/device connected to the game.
Player: a faction such as PLAYER0 (the red keeper), PLAYER1( the blue keeper), PLAYER2, ... the heroes, neutrals, etc.
Sometimes the code is a bit imprecise on the distinction. Users on the network are occasionally referred to as "players". In order to eventually implement "archon mode" (allowing multiple users to control the same player co-operatively), I've gone through the codebase to try to solidify this distinction.
In particular, where possible, I've switched away from ever inferring the user from the player, as future work on archon mode may mean that players can have multiple users. Instead, using a user id is generally preferred to using a player idx anywhere the distinction between two users of the same player might someday be important. To this end, I've added
PlayerNumber get_net_user_player_number(NetUserId user). I'd like to eventually deprecate and remove theuser_idfield inPlayerInfoentirely.TL;DR: prefer the direction
User --> Player, avoidPlayer --> User.Sorry for the largeish diff, but it's mostly a couple consistent find/replace rules, and it should allow for smaller bite-sized work toward archon mode in the future. Hopefully.