Skip to content

fix: make world.input and world.ack match their documented contract - #479

Merged
Taure merged 3 commits into
mainfrom
fix/world-input-ack-contract
Aug 14, 2026
Merged

Taure merged 3 commits into
mainfrom
fix/world-input-ack-contract

Conversation

@Taure

@Taure Taure commented Aug 14, 2026 •

Copy link
Copy Markdown
Contributor

Closes #477. Closes #478.

Both found while documenting client-side prediction across the seven SDKs, and both are the same
shape: the handler does something guides/websocket-protocol.md does not describe.

Reworked after review. The first commit's approach to #477 did not work; architecture, security
and code review all independently reproduced it. Second commit fixes it properly. Both commits are
kept so the review thread still reads.

#477 - world.ack was not monotonic

player_ack is per-zone state (asobi_zone.erl:220) and a player is subscribed to their whole
interest ring, so at view_radius 1 a one-step crossing leaves them subscribed to the zone behind
them. That zone kept emitting its own frozen mark while the destination advanced, so payload.seq
went backwards and the prune-and-replay recipe the guide teaches re-applied consumed input.

What did not work. Dropping the entry in remove_entity. record_ack/3 runs unconditionally in
apply_inputs/4, so any input still queued for the departed zone re-creates it, and the tick prune
keeps it because the player is still subscribed. Reviewers reproduced the original symptom on the
branch in both orderings. The window is the common case: remove_entity is cast at
asobi_world_server.erl:1209 but world_zone_changed only at :1243, and the session rebind is
async, so input routed to the old zone mid-crossing is expected - and only prediction clients receive
acks at all, which means they are sending every frame.

What also would not have worked. Guarding broadcast_acks on is_map_key(PlayerId, Entities).
A player whose entity was removed by game logic - dead, awaiting respawn - is still connected and
still sending input, and would receive no acks at all while their prediction buffer grew.

The fix. asobi_player_session holds last_ack_seq and drops any world.ack that does not
advance it. The session is per WebSocket and dies with it, so the mark resets exactly when the
client's counter does. That holds whichever zone emits the frame and regardless of cast ordering.
The zone keeps recording, because only the zone knows the tick-to-state binding; only the
monotonicity claim moved.

world.ack is therefore monotonic per connection, the guide now says so, and the seven SDKs can
drop the running-maximum workaround
they currently document. That follow-up is not in this PR.

#478 - data was a reserved key on world.input

payload before after
{"kind":"move","x":600} verbatim verbatim
{"data":{"x":1}} #{x} #{x}, deprecated + counted
{"kind":"fire","data":{"x":1}} #{x}, kind dropped verbatim
{"kind":"fire","data":"blob"} #{}, all dropped verbatim
"not an object" invalid_payload (via badmap) invalid_payload (explicit)

That fourth row is why asobi-unity sent world input into a void. Note the fix for Unity is
widgrensit/asobi-unity#46, not this - here that shape now arrives as #{~"data" => ~"..."}, which is
merely honest rather than useful.

Review turned up two more:

  • A silent drop I introduced. The first commit turned a non-map payload into #{}, which a game
    cannot distinguish from legitimate empty input, and told the client nothing. world_input_data/1
    now returns {ok, _} | {legacy_unwrap, _} | invalid and the caller answers invalid_payload.
  • A Medium in the sibling path. match.input's json:decode of a binary data was unguarded.
    json:decode/1 raises, landing in safe_handle_message/2's catch-all, which logs a stacktrace. At
    60 frames/sec/connection and 60 connects/sec/IP, roughly 9 sockets exhaust the logger's burst
    budget and blind every other log line on the node, auth failures included. Same class asobi#465
    closed for badmap. match_input_data/1 makes it total, and gives match.input the same contract.

The sole-key unwrap is reported as [asobi, ws, legacy_input_unwrap] and documented as deprecated
with a removal condition, so it can be retired against real traffic rather than frozen into the spec
by being written down.

Checks

fmt, xref, dialyzer (exit 0), eunit 1959 tests, 0 failures. elp eqwalize on all four
changed modules is identical to main (asobi_ws_handler 2, asobi_zone 2, asobi_player_session 0,
asobi_telemetry 1). The telemetry coverage test caught the new event missing from the attach list,
which is the check working.

Mutation-verified. Removing the session filter fails 4 of the 5 new session tests; the survivor
is the pass-through test, correctly. world_ack_rearms_in_the_zone_left_behind pins the zone-side
behaviour the filter exists to correct, so if a future guard lands in the zone that test says so.

Two defects found while documenting client-side prediction across the
seven client SDKs. Both are cases where the handler does something the
guide does not describe.

world.ack was per zone, not per connection (#477). player_ack is zone
state, and a player is subscribed to their whole interest ring, so at the
default view_radius of 1 a one-step crossing leaves them subscribed to
the zone behind them - which kept acking a frozen seq while the new zone
advanced. The client saw world.ack go backwards, and the prune-and-replay
recipe the guide teaches re-applied inputs the server had consumed.
remove_entity now drops the player's ack, so a zone stops acking a player
the moment their entity leaves it.

data was a reserved key on world.input (#478). Any top-level data was
unwrapped, silently dropping its siblings, and a data whose value was not
a map discarded the whole input as an empty map. That second branch is
why asobi-unity sent world input into a void for every release it shipped.
data is now unwrapped only when it is a map AND the sole key, the shape
asobi-unreal sends; everything else goes through verbatim, as the guide
has always claimed.

The payload decision moves into world_input_data/1 so it is testable
without a session. Both fixes are mutation-verified: reverting either one
fails the new tests.
@github-actions

github-actions Bot commented Aug 14, 2026 •

Copy link
Copy Markdown

🟡 Code Coverage — 76.7%

7597 of 9905 lines covered.


🟡 ELP Lint — 180 warnings

199 diagnostics found. See job logs for details.

Taure added 2 commits August 14, 2026 07:25
Rework after architecture, security and code review. All three found the
same blocker: the zone-side ack drop did not work.

record_ack/3 runs unconditionally in apply_inputs/4, so any input still
queued for the zone a player just left re-created the entry, and the tick
prune kept it because the crossing player is still subscribed. Both
reviewers reproduced the original symptom on the branch. The window is
the common case, not a corner: remove_entity is cast at world_server:1209
but world_zone_changed only at :1243 and the session rebind is async, so
input routed to the departed zone during the crossing is expected, and
only prediction clients get acks at all - they send every frame.

Guarding broadcast_acks on entity presence was the other candidate and is
also wrong: a player whose entity was removed by game logic, dead and
awaiting respawn, is still connected and still sending input, and would
get no acks at all.

So the guarantee moves to asobi_player_session, which is per WebSocket
and dies with it, so last_ack_seq resets exactly when the client's
counter does. It drops any ack that does not advance, which holds
whichever zone emits the frame and regardless of cast ordering. The zone
keeps recording - only the zone knows the tick to state binding. The
world.ack frame is now monotonic per connection, so the guide can promise
it and the SDKs can drop their running-maximum workaround.

Also from review:

- world_input_data/1 returns {ok, _} | {legacy_unwrap, _} | invalid. A
  non-map payload was being turned into an empty map, indistinguishable
  from legitimate empty input - the same silent failure this PR is about,
  reintroduced one clause over. It now answers invalid_payload.
- match.input gets the same contract via match_input_data/1, and its
  json:decode is now total. It could raise into safe_handle_message/2's
  catch-all, which logs a stacktrace per frame; at 60 frames a second per
  connection that is a node-wide logger burst amplifier that blinds every
  other log line. Same class asobi#465 closed for badmap.
- The sole-key data unwrap is reported to telemetry and documented as
  deprecated with a removal condition, rather than frozen into the spec.
- world_input_data/1 no longer sits inside origin_allowed/1's CSWSH
  comment block.
- The straggler case both reviewers reproduced is now a zone test, kept
  as documentation of why the session filter exists.
- migrate-from-hathora.md caught up with the per-connection rewording.
The drift guard caught it: a new event has to appear in
guides/observability.md, not just the attach list.
@Taure
Taure merged commit f18a627 into main Aug 14, 2026
16 checks passed
@Taure
Taure deleted the fix/world-input-ack-contract branch August 14, 2026 05:44
Taure added a commit to widgrensit/asobi-js that referenced this pull request Aug 14, 2026
world.ack is monotonic per connection from asobi v0.84.1
(widgrensit/asobi#479): asobi_player_session holds a last_ack_seq and drops
any ack that does not advance it, so the seq a client receives can no longer
go backwards.

Remove the client-side running maximum from the prediction sample and the
prose, along with the per-zone ack explanation and the reference to
widgrensit/asobi#477. Bump the version floor for the feature to v0.84.1.

The delta accumulation, snapshot timing, opt-in and high-water-mark material
is unchanged.
Taure added a commit to widgrensit/asobi-unity that referenced this pull request Aug 14, 2026
* docs: drop the world.ack running-maximum workaround

asobi v0.84.1 (widgrensit/asobi#479) makes world.ack monotonic per
connection: asobi_player_session holds a last_ack_seq and drops any ack
that does not advance it. The session is per WebSocket and dies with it,
so the mark resets exactly when the client's counter does.

Remove the client-side high-water filter the docs prescribed against the
old per-zone mark, the claim that seq can go backwards, the per-zone ack
explanation, and the widgrensit/asobi#477 reference. Reconciliation is
now: drop every buffered input at or below ack.seq, then replay the rest.

State the floor as asobi >= v0.84.1 rather than v0.84.0, which was
superseded the same day.

Also narrow the reserved `data` key to its v0.84.1 rule: only a payload
whose sole key is `data` holding an object is unwrapped (deprecated);
siblings are no longer dropped, and a non-object payload draws an
invalid_payload error frame instead of being read as empty input.

* docs: correct the world.ack version floor and invalid-payload story

Keep the bare v0.84.1 floor for ack monotonicity and drop the consequence
clause: v0.84.0 servers do send acks, so "an older one sends nothing" was
false.

Revert the dispatcher comment to v0.84.0. It records when world.ack first
appeared on the wire, which is provenance, not the monotonicity floor.

Align WorldInputAsync with WsFrame: a non-object world.input payload draws
an error frame with reason invalid_payload, uncorrelated because the frame
carries no cid.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant