feat: player movement (authoritative + ValidatePosition)#3
Open
fxckdead wants to merge 14 commits into
Open
Conversation
10 task-by-task breakdown covering Player state extensions, new MoveBackwardToLocation/ValidatePosition request packets, MoveToLocation fix, opcode wiring on GameClientConnection, and the world-tick timer on GameServer. Each task ends with build + commit. Final task is manual smoke verification per CLAUDE.md (no automated tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
character_id_ on GameClientConnection is a slot index (0..6) per the existing convention — set during handle_request_game_start_packet via set_character_id(getCharacterObjectId()), where the packet field name is misleading (it's the slot the client clicked, not a Player object id). All existing handlers (RequestGameStart, EnterWorld, etc.) consistently use getCharacterBySlot(player_name_, character_id_). Task 6/7/8 used getCharacterById(character_id_) which expects a real Player::getObjectId() value, so slot 0 (a normal first character) hit the empty-optional branch and the move packet was rejected with ActionFailed and "no character 0" in the log. Also drop the character_id_ == 0 guard in advance_player_movement — slot 0 is valid; the IN_GAME state check already covers "no character selected". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the hardcoded "safer coordinates for all races" midpoint with HumanFighter[0] from Mobius's data/stats/chars/baseStats/HumanFighter.xml: (-71338, 258271, -3104) — Talking Island Village near the warrior guildmaster. Real spawn point with known walkable terrain. Mobius picks one of four nearby nodes at random per spawn; we use the first deterministically until a per-class spawn template lands. Heading kept at the existing 48969 (~268°). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The ValidatePosition handler was setPosition'ing the player to the client-reported coords on every small-delta VP (~once per second). That wiped out the world tick's progress between VPs, so the player could never make actual forward progress under server-authoritative movement — tick advances 12u, VP arrives, server snaps back to where the client thinks we are, repeat forever. Mobius's ValidatePosition.java:86-100 does NOT update server XYZ on small delta — only triggers conditional Z snaps in narrow sub-cases that depend on geo (which we don't have). The small-delta intent is "client and server agree closely enough, leave authoritative pos alone". Also adds a temporary [Movement] diagnostic in Player::advanceMovement so we can see the tick firing — to be removed after smoke confirms the fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The killer bug was a 2-byte misalignment in UserInfo: section 19's "C6 new h's" loop wrote 13 shorts instead of Mobius's 12 (lines 150-161), shifting every subsequent field by 2 bytes. The client read garbage for the movement-speed fields (runSpd/walkSpd) and refused walk animation while still allowing rotation. Confirmed working in-game after the fix. Other changes in this commit: * Trust the client's reported origin in MoveBackwardToLocation. Without server geodata the L2 Interlude client snaps the model to its own floor and the server's stored Z drifts from the client's. Resyncing server position to the client's origin before sending MoveToLocation makes FROM match the client's known self position. Pragmatic fix until geodata lands. * Start PC object IDs at 0x10000000 (L2J convention). Real L2 character IDs are in this range; using 1 was likely fine here but matches the L2J world expectation. * Restore spawn coords to the original Gludio "safer coordinates" (-84318, 244579, -3730). Mobius's HumanFighter spawn at (-71338, 258271, -3104) put the model behind a prop in this client. * Update spawn comment to document the client-side geo snap behavior discovered during smoke testing (the visible altitude is determined by the client, not by the Z we send). * Remove the temporary per-tick [Movement] diagnostic log. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fxckdead
force-pushed
the
feat/player-movement
branch
from
eb685f8 to
b18a47b
Compare
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.
Summary
Spec:
docs/superpowers/specs/2026-05-22-player-movement-design.mdPlan:
docs/superpowers/plans/2026-05-22-player-movement.md9 commits, all reviewed task-by-task (spec compliance + code quality) before landing.
Test plan
🤖 Generated with Claude Code