fix/reach-and-movement-integrity #28

Merged

sickday merged 7 commits from fix/reach-and-movement-integrity into main

2026-07-28 00:32:21 +00:00

sickday commented

2026-07-28 00:31:30 +00:00

Owner

Closes #19
Closes #20
Closes #21
Closes #22
Closes #23
Closes #24

Closes #19 Closes #20 Closes #21 Closes #22 Closes #23 Closes #24

sickday added 7 commits

2026-07-28 00:31:30 +00:00

refactor(cache): decode locs in one direction, and pin the order d219c9565d

decode_locs/1 reversed the loc list twice and the two cancelled. locs/3
appended each id group in file order and then reversed the accumulator on
the way out, so the decoded list came back in reverse file order; Collision
.index_locs/3 then prepended each entry while folding from the left, which
reversed it again. locs_at/1 returned file order by accident.

That order is not cosmetic. Several locs stand on one tile routinely and an
object click carries an id and a tile but not which of them was meant, so
object_shape/2 and toggle_door/6 resolve it with Enum.find/2 over exactly
this list. Correcting either reversal alone -- and both read as plain bugs
in isolation -- would have silently changed interaction targeting across
the whole world with nothing failing.

Build it forwards once and fold from the right when indexing. Verified
against the decoded world: every multi-loc tile in the Lumbridge block
comes back byte-identical to before, and two of them are now pinned by a
test so the next person to tidy a reversal finds out.

The append was also quadratic in the number of loc ids on a square, and a
square carries a few thousand.

Closes #24

refactor(interaction): move reach into a contract, and rename the range gate fbc6ab8f70

World.reachable?/1 was three clauses of interaction policy -- what "close
enough to use this" means for an NPC, for a loc footprint and for a wall --
living inside the module that owns the tick, reachable only through
resolve_action/4. The only way to exercise any of it was to stand a player
up in a running world and drive them through World.request/2.

That is the split the whole family of reach bugs came out of. With the
cheap Chebyshev filter in a contract and the real answer buried in the
tick, the filter kept being read as the answer: reachable?/1 returned true
outright for NPCs and for walls, and follow/2 decided a walk to a moving
target had finished with the filter while arrival used the strict test.
Three sites, all live, none visible in isolation.

Contract.Reach now holds the verdict -- pure, taking a flags snapshot, one
clause per kind of target and no catch-all, so a new kind cannot compile
without saying what reaching it means. The plane comparison is a guard
rather than a per-clause check for the same reason. World keeps the two
lines that resolve the snapshot and hands it over as the thunk it always
was.

Contract.Interaction.adjacent?/2 is renamed worth_checking?/2 and its doc
says outright that a true answer means "run the real check". One of its
tests now asserts the gap directly: the four diagonals it accepts are ones
Contract.Reach refuses.

Collision.interactable?/2 goes with it -- it had no callers left.

Closes #20
Closes #21

fix(world): re-check collision while a route is walked, not only when found b5cbf9a05f

A route was computed once, when the click was handled, and then walked over
as many ticks as it was long without being looked at again. step_player/2
took the tiles from Movement.stride/3 and consulted only MovementAuthority
.claim/4, which is entity occupancy and never terrain.

So anything that changed the world between the click and the last step went
unnoticed: a door closed behind a player mid-walk, a loc added by a script,
a region whose diffs were replayed. The player walked through it, and
audit_walk/3 logged "stepped through collision" while it happened.

Each tile of the stride is now checked against Collision.step?/2 before it
is claimed, and the remainder of the path is dropped at the first refusal.
Same predicate the route was built with, so it costs two map reads per step
and can only reject a step that has genuinely become illegal. A pending
action being walked to then ends as :unreachable on its own, which is the
right answer -- the door being opened is shut.

This replaces the audit on the player path rather than sitting behind it.
Auditing the same tiles with the same predicate immediately after enforcing
them asks whether a function disagrees with itself. audit_walk/3 stays on
the NPC path, where the check that chose the step is in another module.

Only doors mutate collision at runtime today, so the window is one tile
wide. It widens with every feature that adds or removes a loc, which is why
the regression test raises a wall across a route already in flight rather
than testing a door.

Closes #22

fix(movement): say so when a route is capped at its length bound ba9f9354b3

trace/2 ended in Enum.take(@max_path), which keeps the first 128 steps of a
route. A longer one was silently shortened: the player walked 128 tiles,
stopped, and was told nothing -- no message, no log, no marker cleared.

The bound was documented as unreachable because the client cannot click
outside its 104x104 scene, but that reasons about distance and the cap is
on length. A route that doubles back through a building or around water is
far longer than the straight line to its destination while landing well
inside the scene, so ordinary play reaches this and not only a crafted
packet.

Walking as far as the search got is a defensible answer and is kept. Being
quiet about it is not: a bound that drops work without saying so reads as
"we walked you as far as you asked" when it did not.

Closes #23

fix(world): validate where a jump lands before committing to it 8933dbf539

World.jump/2 set player.position outright. Nothing checked that the tile it
landed on was standable -- fine while every destination came from the
transport dump, and not fine once Climbing.Geometry started answering for
the 380 {placement, action} pairs the dump never saw, at 53.6% exact.

Contract.Landing now stands in front of it, for scripted teleports as well
as transports: a dialogue that drops somebody inside a wall is the same bug
as a staircase that does.

Refusing outright was the obvious fix and the data says it is the wrong
one. Measured against the 3,297 extracted transports, 105 destinations read
as solid -- refusing them all breaks 3.2% of a table built from
observations of the real game. 71 of the 105 have an open tile cardinally
beside them and a one-tile nudge lands the player where they were plainly
meant to be; the remaining 34 are enclosed with no open tile in the whole
ring, and those are refused. Checking all eight neighbours instead of the
four finds exactly the same 71, so the diagonals are left out.

Both outcomes log. A nudge means our data or our geometry put somebody
where they cannot stand, and correcting that silently is how it stays true.

What this cannot catch is the void: no plane above 0 carries a single
blocked-floor flag, so a climb landing outside a building upstairs arrives
on open walkable ground and passes every check here. That needs
connectivity rather than a tile lookup, which a placement cannot afford.

Closes #19

docs(player): name the packet that resets move speed behind our back fa7def3573

report_speed/2 sends the mask only when the value changes, which is correct
exactly as long as the client's copy changes only when we say so. One
packet changes it without us: s2c opcode 70 empties the player list, nulls
every cached appearance, sets every slot's speed back to 1, and re-seeds
the local player from the block behind it.

Nothing sends it today, which is the whole reason change-only sending
works. The moment something does -- instancing, a forced world reset, a
relogin that reuses the connection -- every player reverts to walking pace
on the client while move_speed still says 2, and since nothing changed,
nothing is re-sent. That is not an error anywhere: it is the avatar
drifting behind its true tile, the same symptom as never sending the mask
at all.

Recorded next to the function rather than left to be rediscovered by
whoever adds the first caller.

Closes #26

docs(npcinfo): an NPC's render speed is the movement type, not a mask a89485d4b4

Opened as "NPCs have no move-speed mask, which blocks NPC running", on the
premise that the NPC info packet carries distance in its movement bits and
speed in a separate mask the way the player one does. Read against the
client, that is not how rev 149 works and there is nothing to reserve.

method708 hands the movement type straight to NPC.method1663(direction,
speed), which stores it on the queued tile: 1 for the single 3-bit step of
type 1, and 2 for each of the two steps of type 2. The shared Actor render
loop reads pathTraversed[pathLength - 1] and doubles its per-cycle advance
when it is 2 -- the same doubling the player path reaches through its
per-slot byte, off a different source.

So type 2 is the run mask. {:run, first, second} is already the whole of
what an NPC needs to run correctly. All seven bits of the NPC mask block
are spoken for by other fields and the eighth, 0x80, is never tested.

Combat pursuit needs a server that gives an NPC two steps in a tick, and
nothing in this encoder.

Closes #27

sickday merged commit a89485d4b4 into main

2026-07-28 00:32:21 +00:00

No reviewers

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

Revenant/Server!28

No description provided.

Rows
Columns