feat: pointerEvents component maxCameraDistance - #9902
Conversation
…stance Follow decentraland/protocol#470: max_distance (and its deprecated alias max_player_distance, larger wins) is the player-distance threshold, and the new max_camera_distance is the camera-distance one. Combination: only player, only camera, both (OR), neither (player <= 10). PrepareDefaultValues no longer writes MaxDistance/MaxPlayerDistance onto the live message: the generated setters flip the Has* presence bits, so every entry looked like it had both fields and only the OR branch ever ran (#9320). The fallback lives in IsQualifiedByDistance instead. Scene-entity raycast results now carry the ray length as the camera distance in every camera mode; the global-entity path is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
decentraland-bot
added
the
ext-contribution
🚦 CI StatusWindows and Mac built successfully in Unity Cloud.
Warnings count reduced: 12170 => 12166 Warnings/errors in files changed by this PR (7)Lint run · full InspectCode report · took 22m 57s All Unity tests passed ✅
Tests time sums the test cases; Job time is the job's wall clock including checkout, licensing and asset import. Slowest tests
Full report: run summary · results + editor logs: editmode · playmode 🏁 Bare-metal benchmark finished — run #33824440522. Full reportPR #9902, run #33824440522 Overall: ✅ no significant changes Builds: Windows change, Windows baseline, macOS change, macOS baseline How to read this table
Apple M1
Exception breakdown
Intel Core i5
|
|
Slack notification sent to #explorer-ext-contributions for external review. |
decentraland-bot
left a comment
decentraland-bot
left a comment
There was a problem hiding this comment.
PR Review: fix: PointerEvents max_distance is player distance; add max_camera_distance
STEP 1 — Context & Scope
Files in diff:
InteractionInputUtils.cs— core distance-qualification rewrite +PrepareDefaultValuescleanupInteractionInputUtilsShould.cs— new unit tests for the 4-way distance rulesPlayerOriginatedRaycastSystem.cs— scene-entitySetupHitnow passeshitInfo.distance(ray length)PointerEvents.gen.cs— regenerated protobuf withMaxCameraDistance(field 8)
Surrounding context read (not in diff):
PlayerOriginatedProximitySystem.cs— proximity overlap +GetMaxDistanceAndHighestPriority(line 217)ProcessPointerEventsSystem.cs— main consumer callingPrepareDefaultValues+ bothIsQualifiedByDistanceoverloads (lines 306-310)HoverFeedbackUtils.cs— callsIsQualifiedByDistancefor hover-leave events (line 30)PlayerOriginRaycastResultForSceneEntities.cs— struct withfloat? DistanceToPlayerandfloat GetDistance()ProximityResultForSceneEntities.cs— class with non-nullablefloat DistanceToPlayerPlayerInteractionEntity.cs—PlayerPositionnullable whenCharacterControllerabsent
Docs: CLAUDE.md, docs/README.md, review-instructions prompt.
STEP 2 — Root-Cause Check
Problem: The protocol (decentraland/protocol#470) redefines max_distance as player distance (matching what the explorer has effectively measured since 2024), deprecates max_player_distance as an alias, and adds a new max_camera_distance for the camera-origin check. The old code also had a bug (#9320): PrepareDefaultValues wrote MaxDistance = 10 / MaxPlayerDistance = 0, flipping Has* presence bits, so every entry appeared to have both fields and only the OR branch of IsQualifiedByDistance ever ran.
Does the diff fix the cause? Yes for the cursor/raycast path — the rewrite correctly resolves player and camera thresholds from field presence and moves the default out of PrepareDefaultValues into the (null, null) fallback. However, the proximity interaction path (PlayerOriginatedProximitySystem + proximity overload of IsQualifiedByDistance) still reads only MaxPlayerDistance, which is now the deprecated field. This is an incomplete application of the semantic change — see P1 below.
STEP 2 verdict: PARTIAL — cursor path fixed, proximity path not updated.
STEP 3 — Design & Integration
No new long-lived units are introduced. The change is a semantic rewrite within existing static helpers and an existing system. Design is sound for the cursor path.
Teardown/consumption trace: No new subscriptions, events, or resources are opened. The protobuf field MaxCameraDistance is purely a data read. ✅
STEP 4 — Member Audit
DEFAULT_MAX_DISTANCE(new public const, line 13): Used by the(null, null)fallback inIsQualifiedByDistance(1 consumer). Appropriate — documents the protocol default and avoids a magic number.
STEP 5 — Line-Level Findings
P1 — Proximity system not updated for new max_distance semantics
Locations (not in diff):
PlayerOriginatedProximitySystem.csline 233 —GetMaxDistanceAndHighestPriority()InteractionInputUtils.cslines 62-67 — proximity overload ofIsQualifiedByDistance
Problem: This PR redefines max_distance as the canonical player-distance field and deprecates max_player_distance as an alias. The cursor overload correctly resolves both fields (taking the max when both present). The proximity code path has two gaps:
-
GetMaxDistanceAndHighestPriority(line 233) readsinfo.MaxPlayerDistancedirectly (deprecated field only, noHascheck). When a scene uses onlymax_distance: 5for proximity events,MaxPlayerDistancereturns 0 (proto default),sqrMaxPlayerDistance = 0, and every entity is filtered out → proximity silently broken. -
Proximity
IsQualifiedByDistanceoverload (line 66) only checksHasMaxPlayerDistance. Three broken cases:- Scene sets only
max_distance:HasMaxPlayerDistanceis false → returnstrueunconditionally (no distance limit applied) - Scene sets both: only deprecated value used, "larger wins" reconciliation skipped
- Scene sets neither: returns
trueunconditionally — contradicts the cursor overload'sDEFAULT_MAX_DISTANCE = 10ffallback (unlimited range for proximity while cursor has 10-unit cap)
- Scene sets only
Once the SDK migrates scenes from deprecated max_player_distance to canonical max_distance, proximity interactions will silently break.
Fix for GetMaxDistanceAndHighestPriority (line 233):
// Replace:
float maxDistance = info.MaxPlayerDistance;
// With:
float maxDistance = (info.HasMaxDistance, info.HasMaxPlayerDistance) switch
{
(true, true) => Mathf.Max(info.MaxDistance, info.MaxPlayerDistance),
(true, false) => info.MaxDistance,
(false, true) => info.MaxPlayerDistance,
(false, false) => PROXIMITY_DEFAULT_MAX_DISTANCE,
};Fix for proximity IsQualifiedByDistance (lines 62-67):
public static bool IsQualifiedByDistance(
in ProximityResultForSceneEntities proximityResultForSceneEntities,
PBPointerEvents.Types.Info info
)
{
float? maxPlayerDistance = (info.HasMaxDistance, info.HasMaxPlayerDistance) switch
{
(true, true) => Mathf.Max(info.MaxDistance, info.MaxPlayerDistance),
(true, false) => info.MaxDistance,
(false, true) => info.MaxPlayerDistance,
_ => null,
};
float effectiveMax = maxPlayerDistance ?? DEFAULT_MAX_DISTANCE;
return proximityResultForSceneEntities.DistanceToPlayer <= effectiveMax;
}Consider extracting the alias-resolution switch into a shared static float? ResolveMaxPlayerDistance(PBPointerEvents.Types.Info info) helper to avoid duplicating it across three call sites.
P2 — Missing reverse-alias test case
(See inline suggestion on test file)
P2 — Branch/title convention mismatch (ADR-6)
Branch: feat/max-camera-distance, PR title: fix:. The branch prefix should match the commit type. Minor — not blocking.
Security Review
No security issues found.
- NaN distance values:
<=comparisons returnfalseper IEEE 754 (rejects interaction — safe) - Infinity: bounded by
MAX_RAYCAST_DISTANCE = 100fraycast cap - Negatives: distances are non-negative, so
<= negativeisfalse - No hardcoded secrets, no sensitive data in logs
STEP 6 — Complexity
COMPLEX — modifies pointer event distance semantics, interaction qualification logic, and raycast system input handling.
STEP 7 — QA Assessment
QA required — changes affect runtime interaction behavior (distance checks for pointer events governing hover, click, and proximity).
STEP 8 — Non-blocking warnings
None. Main.unity not modified.
REVIEW_RESULT: FAIL ❌
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Modifies pointer-event distance semantics and interaction qualification across raycast and proximity systems
QA_REQUIRED: YES
Reviewed by Jarvis 🤖 · Requested by unknown (<@unknown>) via Slack
| Assert.IsTrue(InteractionInputUtils.IsQualifiedByDistance(result, new PBPointerEvents.Types.Info { MaxPlayerDistance = 6 })); | ||
| Assert.IsFalse(InteractionInputUtils.IsQualifiedByDistance(result, new PBPointerEvents.Types.Info { MaxPlayerDistance = 4 })); | ||
| Assert.IsTrue(InteractionInputUtils.IsQualifiedByDistance(result, new PBPointerEvents.Types.Info { MaxDistance = 4, MaxPlayerDistance = 6 })); | ||
|
|
There was a problem hiding this comment.
[P2] Missing reverse-alias test case. The test verifies MaxDistance=4, MaxPlayerDistance=6 (alias wins), but not the reverse direction where the canonical field wins. Add a case to confirm Mathf.Max works both ways.
| Assert.IsTrue(InteractionInputUtils.IsQualifiedByDistance(result, new PBPointerEvents.Types.Info { MaxDistance = 4, MaxPlayerDistance = 6 })); | |
| Assert.IsTrue(InteractionInputUtils.IsQualifiedByDistance(result, new PBPointerEvents.Types.Info { MaxDistance = 6, MaxPlayerDistance = 4 })); |
Review follow-up: the proximity broad-phase and its IsQualifiedByDistance overload still read only the deprecated max_player_distance. Resolve the threshold through a shared ResolveMaxPlayerDistance (max_distance, alias, larger wins; null when neither) so cursor and proximity agree; proximity falls back to its 3 m default in the broad-phase and to the 10 m default in the qualifier. Adds the reverse-alias and proximity test cases. Also clears four RedundantArgumentDefaultValue warnings elsewhere to satisfy the lint ratchet (no warnings in the files this PR touches). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
robtfm
changed the title
|
Addressed the review:
|
This comment has been minimized.
This comment has been minimized.
pravusjif
changed the title
pravusjif
changed the title
anicalbano
left a comment
anicalbano
left a comment
There was a problem hiding this comment.
QA
✔️ Reviewed in both macOS and Windows systems
✔️ No regressions were found
03.09.2026_08.52.40_REC.mp4
4 participants
Pull Request Description
What does this PR change?
Implements decentraland/protocol#470 for
PBPointerEvents.Info:max_distanceis the player-distance threshold (this is what the explorer has effectively measured since 2024 — avatar head in every mode except first-person — and what deployed scenes rely on; the protocol comment now says so instead of "camera distance").max_player_distanceis a deprecated alias formax_distance; when both are set the larger wins.max_camera_distance: the camera-origin check (ray length), for scenes driving a virtual camera away from the avatar.Fixes #9320:
PrepareDefaultValueswroteMaxDistance = 10/MaxPlayerDistance = 0onto the live message, and the generated setters flip theHas*presence bits, so every entry looked like it had both fields and only the OR branch ofIsQualifiedByDistanceever ran (the "onlymax_player_distance" branch was dead code). The fallback now lives inIsQualifiedByDistance.Changes:
InteractionInputUtils.IsQualifiedByDistance(cursor overload): resolves the player and camera thresholds from field presence, then the 4-way rule. Proximity overload untouched.InteractionInputUtils.PrepareDefaultValues: no longer touches the distance fields.PlayerOriginatedRaycastSystem: scene-entitySetupHitgetshitInfo.distanceas the camera distance in every camera mode (previously mode-dependent). Global-entity path unchanged.PointerEvents.gen.csregenerated from the Feat: worlds compatibility chat commands #470 protocol (scripts/npm run build-protocol);scripts/package.jsonstill pins the older npm release — bump it once Feat: worlds compatibility chat commands #470 is published.InteractionInputUtilsShould.QualifyByDistancecovers the four rules and the alias.Behaviour deltas to be aware of: first-person
maxDistancenow measures from the avatar root to the hit point rather than along the ray from the near plane; scenes setting onlymaxPlayerDistanceno longer get an implicit|| rayDistance <= 10.Test scene:
pointer-camera-distance-scene(cubes withmaxDistance: 2,maxPlayerDistance: 2,maxCameraDistance: 5, keyed virtual cameras) — same scene used to verify the bevy side in decentraland/bevy-explorer#1158. SDK side: decentraland/js-sdk-toolchain#1560.🤖 Generated with Claude Code
QA TEST STEPS
Use the build from this PR to enter the ZONE/SEPOLIA world
sdk7testscenes.dcl.ethat position34,20and confirm that by getting close to the cubes you can switch them LIVE in any waySpecially confirm that you can toggle the L4 cube by standing next to it, and changing from 3rd person cam to drone cam, so that the camera distance toggles the cube LIVE state.
(the test scene can also be ran locally: https://github.com/decentraland/sdk7-test-scenes/tree/main/scenes/34%2C20-pointer-events-distances)
Screen.Recording.2026-09-02.at.4.54.20.PM.mp4