Skip to content

Commit 0ad9744

Browse files
feat: consolidate player tracking in @dcl/sdk/players
The players helper gated arrivals on PlayerIdentityData AND AvatarBase, so a peer whose avatar profile replicates late — or never — was never reported at all. Every scene running an authoritative server therefore hand-rolled its own tracker: diff PlayerIdentityData each frame, keep a Set of lowercased addresses, and poll AvatarBase on a timer for the display name. Rather than add a parallel API for that, this reworks the existing one. onEnterScene/onLeaveScene keep their exact semantics and payloads by default, and take an options object: onEnterScene(cb) // unchanged: identity + profile onEnterScene(cb, { requireProfile: false }) // as soon as the identity exists Both thresholds come from one diff pass, so the sets cannot drift. Also new: onPlayerNameChanged (replaces polling AvatarBase), getPlayers, getPlayerCount, a never-empty displayName on the player payload, and nameResolved / joinedAtMs. onLeaveScene gained a second argument carrying the last known state, since getPlayer returns null once the entity is gone; extra callback parameters are source-compatible, so existing one-argument handlers are unaffected. Fixes found while reworking it, all of which affected client scenes too: - `players.length === playerEntities.size` compared a count of entities against a map cleaned elsewhere; the two could coincide while the sets differed, silently dropping a join. - Leaves were detected through AvatarBase.onChange, so a peer that lost identity without a profile change never fired onLeaveScene — and the per-entity onChange callbacks had no removal path, accumulating for the life of the process. - Duplicate entities for one address resolved the name from whichever came first in insertion order, pinning the player to the stale entity forever. Now the one carrying a profile wins. - A name starting with `0x` was treated as an address echo; `0xSomeName` is a legal claimed name. Compared against the actual address instead. - isGuest was captured once and never refreshed. - getPlayer compared addresses case-sensitively, so a lowercased address silently returned null. - Callbacks had no unsubscribe and no error isolation: one throwing handler killed the tracker for the rest of the run. - The component rows and the callback lists are iterated over snapshots, and all state settles before any callback runs, so handlers observe a consistent set even when someone joins and someone else leaves on the same tick. definePlayerHelper is now memoized per engine. It was being called twice with the global engine — once here, once inside addSyncTransport — so two systems were scanning the same component every frame; now there is one. Snapshot goldens regenerated: the rewrite moves SCENE_COMPILED_JS_SIZE_PROD by roughly +2.6KB on every scene bundle, since observables.ts pulls this module into all of them.
1 parent 5ffe873 commit 0ad9744

16 files changed

Lines changed: 1515 additions & 117 deletions

packages/@dcl/sdk/src/network/message-bus-sync.ts

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export function addSyncTransport(
3939
) {
4040
// Profile Info
4141
const myProfile: IProfile = {} as IProfile
42-
fetchProfile(myProfile!, getUserData)
42+
const profileReady = fetchProfile(myProfile!, getUserData)
4343

4444
const isServerAtom = Atom<boolean>()
4545
const isRoomReadyAtom = Atom<boolean>(false)
@@ -183,12 +183,29 @@ export function addSyncTransport(
183183
}
184184
})
185185

186-
players.onEnterScene((player) => {
187-
DEBUG_NETWORK_MESSAGES() && console.log('[onEnterScene]', player.userId)
188-
if (!isServerAtom.getOrNull() && myProfile.userId === player.userId) {
189-
requestState()
190-
}
191-
})
186+
// Deferred until the local identity is known: this compares against `myProfile.userId`,
187+
// which `fetchProfile` populates asynchronously, so subscribing at module load would miss
188+
// the local player's arrival whenever the profile lost that race.
189+
//
190+
// `requireProfile: false` — bootstrapping state needs the local identity, not the avatar
191+
// profile, which can replicate seconds later or never.
192+
// `replayPresent: true` — the players helper is shared per engine and this subscription is
193+
// late by construction, so the local player has usually arrived already. Without the replay
194+
// the bootstrap would never fire. `requestState` is guarded by `requestingState`, so an
195+
// extra call is a no-op.
196+
void profileReady
197+
.catch(() => undefined)
198+
.then(() => {
199+
players.onEnterScene(
200+
(player) => {
201+
DEBUG_NETWORK_MESSAGES() && console.log('[onEnterScene]', player.userId)
202+
if (!isServerAtom.getOrNull() && myProfile.userId?.toLowerCase() === player.userId.toLowerCase()) {
203+
requestState()
204+
}
205+
},
206+
{ requireProfile: false, replayPresent: true }
207+
)
208+
})
192209

193210
// Asks for the REQ_CRDT_STATE when its connected to comms
194211
RealmInfo.onChange(engine.RootEntity, (value) => {

packages/@dcl/sdk/src/network/utils.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import { IProfile } from './message-bus-sync'
88
export function fetchProfile(
99
myProfile: IProfile,
1010
getUserData: (value: GetUserDataRequest) => Promise<GetUserDataResponse>
11-
) {
12-
void getUserData({}).then(({ data }) => {
11+
): Promise<void> {
12+
return getUserData({}).then(({ data }) => {
1313
if (data?.userId) {
1414
const userId = data.userId
1515
const networkId = componentNumberFromName(data.userId)

0 commit comments

Comments
 (0)