Implement LAN play#120
Open
neumannt wants to merge 13 commits into
Open
Conversation
Decouple the mouse force from the single global current-player state so each of the two players can independently drive their own marble. This is a prerequisite for both LAN two-player and any future local split-input two-player mode. MouseForce now holds an array of forces (one per player) and exposes both a per-actor sum (used by ordinary floors) and a per-player accessor (used by YinyangFloor, where the floor color selects the input). Floor::process_mouseforce drops the controlled_by gate because the per-actor sum already encodes it. YinyangFloor queries the per-player force directly so its "floor color selects whose input passes" semantics are preserved. server::Msg_MouseForce and world::SetMouseForce gain a player index. The client passes player::CurrentPlayer() so single-player behavior is unchanged: only one slot is non-zero per tick, and the sum collapses to the previous behavior. The netgame stub hard-codes player 0 for now; a real protocol comes in phase 3. Also add an indexed player::InhibitPickup overload so a future LAN client can inhibit pickup for its own marble without touching the active player.
Introduce client::EventSink, an observer interface with hooks for every cross-cutting server->client effect: the existing Msg_* surface (command, advance-level, jump-back, level-loaded, player-position, sparkle, show-text/document, finished-text, teatime, play-sound, error) plus two per-tick engine taps for actor position/velocity and actor sprite model. Each Msg_* notifies any registered sink before performing its local effect, so the host's display path is unchanged. The engine taps are placed at the mutation choke-points: Actor::set_model and World::move_actors (once per actor per tick, after the sub-stepped integration completes). Msg_ShowDocument now updates the status bar directly instead of calling Msg_ShowText so the OnShowText hook does not double-fire. No sinks are registered yet; this is purely the outbound seam.
Define the CL_*/SV_* tag set for the LAN protocol, a NetworkSink that batches outbound EventSink hooks into reliable and unreliable buffers, and the two dispatcher functions that parse inbound packets on each side. NetworkSink subclasses client::EventSink. Per-tick volume events (actor moves, player position) go unreliable on channel 0; the rest go reliable on channel 1. flush() ships both buffers and clears them; called once per tick from the host's server loop. dispatch_input_from_client tags inputs with the connection's player index so the server simulation knows which marble to drive. dispatch_event_from_server applies events on the remote side: actor positions write ActorInfo directly and trigger a sprite move; sprite changes call Actor::set_model. Promote Actor::set_model from protected to public so the dispatcher can apply remote sprite updates; set_anim stays internal (it wires up animation callbacks the engine owns). The host's server_loop now registers a NetworkSink for the duration of the connection and flushes it each tick. The remote's join loop sends CL_MOUSE_FORCE on mouse motion (batched per frame) and feeds inbound packets to dispatch_event_from_server. The actual handshake, level transport, and end-to-end LAN play come next; this commit lays the substrate.
Two flaws in the vendored buffer library that didn't matter while it was only used for tiny stub packets, but break a real packetized protocol: * Buffer::read() returned -1 at EOF without setting FAILBIT/EOFBIT. A loop like `while (b >> tag)` on a Uint8 read past the end never fails: operator void*() reads FAILBIT and stays truthy, the byte variable keeps its last value, and the dispatcher spins forever on a no-payload tag (e.g. activate-item). * Buffer::clear() reset the byte cursor but not iostate. Once any prior read set FAILBIT, every subsequent assign() (which goes through clear()) inherited the failure, and every fresh packet was silently undispatchable -- so reliable events after the first one consumed to EOF were dropped. Fix both: read() now sets FAILBIT|EOFBIT on overflow, and clear() resets iostate to GOODBIT.
Two enigma binaries can now connect over ENet, load the same level, and play it together: host drives the black marble, joiner drives the white. Tested locally with two windows on the same machine. Wire protocol (in netgame.cc): * Tags partitioned into 0x10..0x7F for client->host inputs and 0x80+ for host->client events, plus CL_HELLO/SV_HELLO for the handshake. Two ENet channels: 0 unreliable for per-tick state, 1 reliable for everything else. * SV_HELLO ships protocol version, level pack name, level index, RNG seed, and the assigned player number; CL_HELLO acks with the protocol version. The remote then setCurrentIndex / setCurrentPosition / RandomState / Msg_LoadLevel / SetCurrentPlayer (=1) so both sides have the same world before the game starts. * NetworkSink (a client::EventSink subclass) batches outbound events into per-channel buffers and ships them once per host tick. * dispatch_input_from_client (host side) and dispatch_event_from_server (client side) parse and apply. Engine taps so the protocol has something to ship: * World::move_actors emits OnActorMoved per actor per tick, keyed by the actor's index in the live actorlist (not Object id, which isn't reproducible across peers without prior level history). * Actor::set_model emits OnActorSpriteChanged, also by index. * display::SetModel(GridLoc, string) emits OnGridSpriteChanged keyed by (layer, x, y) -- covers stones/floors/items state changes including turnstile rotations. * display::KillModel emits OnGridSpriteCleared so cells emptied by stone removal (e.g. the cell a turnstile arm vacates) are cleared on the remote too. Loops (netgame.cc): * host_main_loop: registers NetworkSink, runs client::Tick + server::Tick, polls the remote's inputs each frame (dispatch_input_from_client with player=1), flushes the sink. * remote_main_loop: client::Tick only (no physics), polls inbound events, flushes batched local input. Local input routing (client.cc): * netgame::IsClient() / IsActive() let the client know it's a remote session; mouse motion, item activation, inventory rotate, and pickup-inhibit fall through netgame::SendInput* helpers to the host instead of the local server::Msg_*. * In an active LAN session, focus loss no longer pops the in-game menu (the other peer keeps simulating, so popping a local pause looks like a freeze). UX: * MainMenu unhides the Network Game button (was gated on ENABLE_EXPERIMENTAL). * The visible mouse cursor's saved-background snapshot is refreshed at the cls_preparing_game -> cls_game transition via the new MouseCursor::recapture_background, so the first cursor move on the joining client doesn't paint stale menu pixels into the loaded level. * Mouse stays ungrabbed and visible during a network session so the user can move between two windows for testing. Object::getNextIdSnapshot / setNextId are added but no longer load-bearing; switching from object id to actorlist index made them unnecessary. Left in place for now; can be removed later. Known gaps for follow-up: sounds emitted directly via sound::EmitSoundEvent (rather than client::Msg_PlaySound) don't sync; keyboard inputs (suicide, restart, etc.) aren't forwarded; the host's "current level" picker is just whatever was last selected in the level menu.
Sounds, inventory, move counter, and keyboard commands now travel
between host and remote, and per-player inventory operations target
the right player.
Outbound (host -> client):
* Tap sound::EmitSoundEvent (the single chokepoint for all 34
in-engine sound sites). New SV_SOUND tag carries name, position,
volume, and the global flag. Drops the now-redundant SV_PLAY_SOUND
and SV_PLAY_SOUND_REL tags and the matching OnPlaySound /
OnPlaySoundRelative hooks; client::Msg_PlaySound just calls
sound::EmitSoundEvent and the lower-level tap fans out.
* Tap player::RedrawInventory(Inventory*) keyed by the inventory's
owner, so each peer can render its own player's inventory bar
even when the host's CurrentPlayer is the other one. Adds
Inventory::getOwner().
* Tap StatusBarImpl::set_counter, so the remote's stone-move
counter no longer freezes at zero.
Inbound (remote -> host):
* F3 / Shift+F3 / Ctrl+A / Ctrl+Shift+F3 forward as
send_command("suicide" | "restart"). F4 / F5 forward as
"advance_strict" / "advance_unsolved". F6 forwards as
"jumpback". server::Msg_Command grows handlers for the three
new tokens.
Per-player input routing fix:
* player::ActivateFirstItem and player::RotateInventory now take
an explicit player index instead of using the host-side
icurrent_player. server::Msg_ActivateItem(int iplayer) takes a
player too.
* The local mouse handlers pass player::CurrentPlayer(); the
network dispatcher passes the connection's player index. Without
this, when the remote left-clicked, the host activated *its*
first item (e.g. the yin-yang), causing both marbles to swap and
end up sharing inventory.
Also pin and snapshot the level position once at the start of
netgame::Start, so a stray index advance (or hover-induced change)
between proxy capture and SV_HELLO can't make the host load one
level locally and tell the remote to load another. Printfs now
display 1-indexed level numbers to match the level menu.
server::Msg_Command grows an iplayer parameter (default -1 means "all"). The "suicide" command honours it: if iplayer >= 0, only that player's actors die; otherwise the legacy global behaviour applies for any internal caller that still wants it. Local F3 / Ctrl+A pass player::CurrentPlayer(); the network dispatcher passes the connection's player index. So in a LAN game the remote pressing F3 only kills the white marble, not both. player::LevelLoaded now skips AddYinYang when netgame::IsActive(), so neither peer can swap to the other player's marble in a LAN session. Single-player and same-machine 2P keep their existing behaviour. The session flag is set before Msg_LoadLevel in both host and remote so this gate fires at the right time. player::Suicide(int) is the per-player primitive; the no-arg overload that iterates all players stays for legacy callers.
Previously both peers ran the level's Lua at load time and tried to allocate Objects in lock-step. The id alignment was fragile: any allocation difference between the two peers (cached vs cold proxy parse, prior Lua state, residual inventory contents) silently desynced every actor id by the count of the mismatch, and the host's SV_ACTOR_MOVED packets started driving the wrong sprites on the remote. Rework so only the host loads levels: * World::Resize fires NotifyResize -> SV_RESIZE. The remote responds by calling Resize(w, h) locally, which rebuilds an empty world structure and resizes the display grid. Two SV_RESIZE events per load (initial 20x13 from PrepareLevel, then the actual level size from Lua's createWorld). * World::add_actor fires NotifyActorAdded unconditionally (was gated on !preparing_level). The remote's SV_ACTOR_ADDED dispatcher pins Object::setNextId(host_id) around MakeActor + AddActor so the new actor lands on the host's id. * The existing display::SetModel / KillModel taps populate grid sprites on the remote as they fire during the host's load. * The remote no longer calls server::Msg_LoadLevel for SV_RELOAD or for SV_ADVANCE_LEVEL / SV_JUMP_BACK; client::Msg_AdvanceLevel and Msg_JumpBack skip the local load when netgame::IsClient(). * netgame::Join calls client::Stop() before remote_main_loop so m_state left at cls_abort from a prior game doesn't trip AbortGameP() on the first iteration. * Drop the Object::next_id snapshot from the handshake, drop Proxy::release() exposure, drop the per-peer Lua execution on the client side. PROTO_VERSION bumped to 3. Also fixes the issues from the post-LAN code review: * Initialize Inventory::ownerId(-1) so RedrawInventory can't read uninitialized memory before NewGame runs. * World::move_actors re-reads actorlist size at the bottom rather than reusing a pre-physics snapshot, so mid-tick yield/add doesn't read past the new end. * World::yield_actor fires NotifyActorKilled BEFORE erasing, so the dispatcher can still resolve the id. * World::add_actor sends the actor's kind / pos / vel / owner so the remote can recreate via MakeActor. * server::Msg_Pause emits SV_PAUSE; Menu::manage calls netgame::Service() in its event-pump loop so the host's menu doesn't strand the ENet connection or the pause event. * CL_COMMAND whitelisted to suicide/restart/advance_*/jumpback. * ~Peer_Enet only disconnects when m_connected; SinkGuard + SessionGuard so a throw inside the loop doesn't leak a freed sink pointer or strand the session flags. * Wire format switched to double for actor positions; the type-pun union in ecl_buffer.cc replaced with memcpy. * Wider sound tap (sound::EmitSoundEvent) and inventory tap (player::RedrawInventory keyed by Inventory owner) so the remote sees the right player's bar. Known gaps documented in PLAN.md: per-player document text scoping, score submission semantics, occasional stale turnstile arm, lobby UI (still a Start/Join/Back stub).
Network menu now opens HostLobbyMenu / JoinLobbyMenu. The host sees a random 6-digit access code and the listening port, can browse packs and levels (with a network-only filter on by default), and the Start Game button lights up once a client has authenticated. Failed auth attempts are counted and the last source address is shown. Protocol bumped to 4: the client sends CL_AUTH (proto + code) right after connect, and the host replies SV_AUTH_OK or SV_AUTH_FAIL before the existing SV_HELLO/CL_HELLO exchange. netgame::Start was split into OpenHostLobby/ServiceHostLobby/StartHostedGame so the menu can keep the listener alive while the host is in the lobby UI.
TextField::setMaxChars limited input to maxChars-1 because the overflow check ran on the post-insert length with >= instead of >. The join dialog's 6-digit code field only accepted 5 digits because of this. The other caller (username, maxChars=20) gets one more character. The host lobby now embeds the full LevelWidget instead of the prev/next-level buttons + the small preview I had inlined. Same code the regular level menu uses, so it brings score icons, mouse hover, arrow / page navigation, and the grid of preview thumbnails for free. Clicking a level fires our on_action with the widget as sender; we treat that as "this level is now picked" and refresh the label — unlike LevelMenu, we do not launch the game. The Start Game button is the only path to actually start.
The host lobby no longer has a Start Game button. Clicking (or pressing Enter on) a level in the LevelWidget arms that level; the game starts as soon as a client has authenticated. If a client is already waiting, the click launches immediately. Status text shows which level is armed while waiting. Re-clicking before the client arrives replaces the armed level. The join dialog now defaults the host field to the last value that produced a successful game (persisted as NetGameJoinHost in app state), falling back to "localhost" the first time. The host status line now prints "Listening on 0.0.0.0:N" to make it explicit that the server already accepts connections on every interface.
Two LAN gameplay polish fixes: Advance-after-win used Index::advanceLevel which steps to the very next slot in the pack regardless of mode. Hitting a single-player-only level inside a network session would silently turn off two-player mode for everyone. Msg_AdvanceLevel now loops past non-network levels when netgame::IsActive(), falling back to abort if the pack has no more network-mode levels. player::NewGame put two extralifes into each player's inventory but never notified the EventSink for the non-current player, so the remote saw an empty inventory on level start and ended up with zero extra lives. NewGame now calls RedrawInventory for both inventories; NotifyInventoryChanged is then fired for each, and the remote's status bar matches the host's setup.
Two defensive cleanups in the netgame code: The seed broadcast in SV_HELLO was Uint32(SDL_GetTicks()) ^ Uint32(uintptr_t(peer)). XORing with a heap pointer leaked the lower 32 bits of the host's heap address layout to the authenticated peer (useful only as an ASLR-weakening primitive if combined with another bug, but easy to avoid). Use std::random_device directly. failed_attempts was a plain int++ in record_failed_attempt. Cap it at INT_MAX so a long-running session can't get to undefined-behaviour overflow.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This patch series implements multi-player games in the LAN. You can start it from the main menu by selecting network play. The host selects a starting level and provides a security code that the second player has to enter to join. This is meant purely for LAN play, there is no NAT traversal or anything like that.
Architecture wise the series first organizes the input per user, then sets up a network pipe, and then propagates all state across the network. Originally I tried to load levels by running the same code on client and server, but that is unreliable because the Lua code is sometimes non-deterministic. A later commit changes that to simply run everything on the server and to only propagate the result state to the client.
I should mention that this patch series was created with the help of claude code. I kept it on a short leash, and I reviewed the design and the code, but I am not familiar with the code base. Thus it is probably a good idea if an experienced developer looks at it again. The series was play-tested with two Linux machine in a LAN.