fix(parser): drop unsafe borderId read inside the RoomUnitParser per-user loop

The Infostand Borders merge (origin/Dev 4b7d04d, upstream commit) added

    user.borderId = (wrapper.bytesAvailable ? wrapper.readInt() : 0);

inside the per-user loop in RoomUnitParser (the parser for the
RoomUsersComposer packet — header 3920 — which ships the full roster
on room enter). The guard is unsafe inside a loop: `bytesAvailable`
is a boolean meaning "any bytes left in the WHOLE packet?", not
"any bytes left in THIS user record". For every user except the
last one, `bytesAvailable === true` because the NEXT user's bytes
still follow, so the parser reads an int and steals 4 bytes from
the next user — cascade corruption of the entire roster.

Symptom in production: users don't see each other on first room
sight. The roster arrives, the parser sfasa, RoomEngine drops the
malformed records.

Fix: stop reading borderId inside the loop. The per-user border id
is shipped separately via RoomUnitInfoParser (single-user packet,
no loop), where the bytesAvailable guard is safe. The roster
packet's last-tail extension story stays clean for any future
trailing block the same way other parsers do — but only when the
guard is the LAST read in the packet, not a per-record one.

This also makes the renderer wire-compatible with both old
emulators (no borderId at all) and the new Arcturus version that
ships borderId in RoomUsersComposer — the latter just has 4 extra
trailing bytes per user that the parser ignores. A follow-up change
on Arcturus' RoomUsersComposer can drop the borderId append, or
keep it and the client simply doesn't read it from the roster
(which is fine — the infostand re-fetch via RoomUnitInfoParser
gives the authoritative border).

mvn-equivalent: yarn compile:fast clean, vitest 138/138.
This commit is contained in:
simoleo89
2026-05-19 21:05:36 +02:00
parent cc1e8fe9c7
commit 05ea0db806
@@ -146,7 +146,16 @@ export class RoomUnitParser implements IMessageParser
user.roomEntryMethod = wrapper.readString(); user.roomEntryMethod = wrapper.readString();
user.roomEntryTeleportId = wrapper.readInt(); user.roomEntryTeleportId = wrapper.readInt();
user.borderId = (wrapper.bytesAvailable ? wrapper.readInt() : 0); // NOTE: do NOT read borderId here. `wrapper.bytesAvailable`
// is a boolean meaning "any bytes left in the whole packet?",
// not "any bytes left in THIS user". For users that aren't
// the last in the roster, bytesAvailable === true because
// the NEXT user's bytes follow — reading an int would steal
// 4 bytes from the next user and cascade-corrupt the entire
// roster (users not seeing each other on first sight). The
// border id for an individual user arrives via
// RoomUnitInfoParser (single-user packet), where the
// bytesAvailable guard is safe because there is no loop.
i++; i++;
} }