Skip to content

chore: consolidate DCL.PrivateWorlds into DCL.RealmNavigation - #8800

Merged
biotech77 merged 5 commits into
devfrom
fix/7551-private-worlds-asmdef
Jun 4, 2026
Merged

chore: consolidate DCL.PrivateWorlds into DCL.RealmNavigation#8800
biotech77 merged 5 commits into
devfrom
fix/7551-private-worlds-asmdef

Conversation

@biotech77

@biotech77 biotech77 commented May 18, 2026

Copy link
Copy Markdown
Contributor

Pull Request Description

Resolves #7551

What does this PR change?

Removes DCL.PrivateWorlds.asmdef and folds its 8 files into DCL.RealmNavigation/PrivateWorlds/. Removes CommunityMembershipCheckerAdapter, replaced by CommunitiesDataProvider implementing ICommunityMembershipChecker directly.

The adapter existed only to avoid an asmdef cycle (Social → PrivateWorlds, but PrivateWorlds needed CommunitiesDataProvider from Social).

Test Instructions

Steps (standard run):

metaforge explorer run XXXX  # ← replace with this PR number

Expected result:

Steps (fresh account):

metaforge account create --clear
metaforge explorer run XXXX  # ← replace with this PR number

Expected result:

Automation (if applicable):

metaforge explorer test XXXX

Test Steps

Retest and verify all tests work from original PR:
#6983

Quality Checklist

  • Changes have been tested locally
  • Documentation has been updated (if required)
  • Performance impact has been considered
  • For SDK features: Test scene is included

Code Review Reference

Please review our Branch & PR Standards before submitting. It explains the automated review flow, QA/DEV approval requirements, and what each label does — especially useful for first-time contributors.

@biotech77 biotech77 self-assigned this May 18, 2026
@biotech77
biotech77 requested review from a team as code owners May 18, 2026 11:28
@github-actions

github-actions Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

@claude

claude Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

PR Review

  • Gather context and read CLAUDE.md / docs
  • Read changed files
  • Analyze diff and identify issues
  • Post review

View job run

@github-actions github-actions Bot deleted a comment from claude Bot May 20, 2026
@github-actions github-actions Bot deleted a comment from claude Bot May 20, 2026
@claude

This comment has been minimized.

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

🔍 Claude reviewed this PR and found no blocking issues, but assessed it as complex — human DEV review is still required before merging.

@m3taphysics

Copy link
Copy Markdown
Contributor

PR #8800, run #26943877931

Builds: Windows change, Windows baseline, macOS change, macOS baseline

How to read this table
  • Each build is measured 3 times. The values are the median, and (min–max) is the lowest and highest of those runs — a wide range means the metric is noisy and small differences are not trustworthy.
  • Δ is Change minus Baseline (a negative Δ means Change is faster).
  • 🟢 faster / 🔴 slower — a real difference: larger than both 3% and the run-to-run range.
  • ⚪ within noise — the difference is smaller than how much the build varies between its own runs, so it cannot be told apart from random variation. Treat it as no change.
  • Exceptions logged — exceptions found in the run logs; more than the baseline is flagged 🔴 even when frame times look fine.

Framework 13 i7

Metric Baseline Change Δ Result
Samples 2701 (×3) 2701 (×3)
CPU average 33.3 ms 33.3 ms -0.0 ms ⚪ within noise
CPU 1% worst 33.8 ms (33.6–33.8) 33.7 ms (33.5–33.8) -0.0 ms ⚪ within noise
CPU 0.1% worst 36.2 ms (36.0–36.8) 37.1 ms (34.4–37.3) 0.8 ms ⚪ within noise
GPU average 8.8 ms (7.0–9.8) 8.4 ms (8.3–8.9) -0.4 ms ⚪ within noise
GPU 1% worst 18.3 ms (17.8–18.5) 16.8 ms (16.7–17.1) -1.5 ms 🟢 8% faster
GPU 0.1% worst 22.7 ms (21.2–26.7) 19.4 ms (18.8–23.3) -3.3 ms ⚪ within noise
Exceptions logged 44 48 +4 🔴 more errors

@Ludmilafantaniella Ludmilafantaniella left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on Windows and Mac. ✅

Verified all Private Worlds flows from the original PR #6983:

  • Public - entered normally, no popups
  • Password - password popup appeared, correct password granted access, wrong password showed error, owner bypassed without password
  • Invite - listed user entered successfully, non-listed user was denied
  • Kick - user inside a public world was kicked to Genesis Plaza after owner switched permissions

All flows working correctly after the consolidation. LGTM 🚀

Image
8800-evi.mp4

✅Smoke test performed:

  • ✔️ Backpack and wearables in world
  • ✔️ Emotes in world and in backpack
  • ✔️ Teleport with map/coordinates/Jump In
  • ✔️ Chat and multiplayer
  • ✔️ Profile card
  • ✔️ Camera
  • ✔️ Skybox

@claude

claude Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Claude finished @biotech77's task in 3m 18s —— View job


PR Review

  • Gather context and read CLAUDE.md / docs
  • Read changed files
  • Investigate +4 exceptions from performance test
  • Analyze diff and identify issues
  • Post review

Summary

Problem: DCL.PrivateWorlds was a standalone assembly that created a circular dependency — DCL.Social needed DCL.PrivateWorlds (for ICommunityMembershipChecker), but DCL.PrivateWorlds needed CommunitiesDataProvider from DCL.Social. The CommunityMembershipCheckerAdapter existed solely as indirection to break that cycle.

Fix: The PR dissolves DCL.PrivateWorlds into DCL.RealmNavigation, moves ICommunityMembershipChecker there, and has CommunitiesDataProvider implement it directly. This is a proper fix, not a workaround. ✅


No blocking issues found

Assembly dependency chain verified correct:

  • CommunitiesDataProvider.cs compiles into DCL.Social via DCL.Communities.asmreff56000518aba31544aa96f4739e50a64
  • DCL.Social.asmdef already references DCL.RealmNavigation (GUID:f3634757d00dab2429c6c11e69404e97, line 36) — the using DCL.PrivateWorlds; namespace in CommunitiesDataProvider.cs resolves correctly
  • The 7 new GUIDs added to DCL.RealmNavigation.asmdef exactly match the non-overlapping refs from the deleted DCL.PrivateWorlds.asmdef
  • Old DCL.PrivateWorlds GUID (f8127c6ac263abf468221dcbccbde182) correctly removed from both DCL.Social.asmdef and DCL.Plugins.asmdef

Logic unchanged:

  • CommunitiesDataProvider.IsMemberOfCommunityAsync (CommunitiesDataProvider.cs:66) is functionally identical to the deleted CommunityMembershipCheckerAdapter.IsMemberOfCommunityAsync — both call GetCommunityAsync and check role != none
  • Async exception handling in RunPermissionCheckAsync correctly ignores OperationCanceledException, logs others via ReportHub.LogException — compliant with CLAUDE.md §9
  • Cancellation checked via ct.IsCancellationRequested, not ThrowIfCancellationRequested — correct per CLAUDE.md §9

Performance test +4 exceptions (44 → 48) — non-blocking:
The flagged 🔴 in the perf run is worth acknowledging: there are no new exception-throwing code paths in this diff. The membership check path (CommunitiesDataProvider.IsMemberOfCommunityAsyncGetCommunityAsync) behaves identically to the old adapter, and all exception sites are unchanged. The 4 additional exceptions are consistent with network timing variance in the test environment and are not attributable to this change.

Non-blocking observation

PrivateWorldEvents.cs (containing IWorldAccessGate and ICommunityMembershipChecker) sits at RealmNavigation/PrivateWorldEvents.cs while all other private world files live under RealmNavigation/PrivateWorlds/. No functional impact — this was noted in the previous review as well.


REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Modifies assembly definitions, DI wiring in DynamicWorldContainer, and moves cross-assembly interface ownership.
QA_REQUIRED: YES

@m3taphysics

Copy link
Copy Markdown
Contributor

PR #8800, run #26964028283

Builds: Windows change, Windows baseline, macOS change, macOS baseline

How to read this table
  • Each build is measured 3 times. The values are the median, and (min–max) is the lowest and highest of those runs — a wide range means the metric is noisy and small differences are not trustworthy.
  • Δ is Change minus Baseline (a negative Δ means Change is faster).
  • 🟢 faster / 🔴 slower — a real difference: larger than both 3% and the run-to-run range.
  • ⚪ within noise — the difference is smaller than how much the build varies between its own runs, so it cannot be told apart from random variation. Treat it as no change.
  • Exceptions logged — exceptions found in the run logs; more than the baseline is flagged 🔴 even when frame times look fine.

Framework 13 i7

Metric Baseline Change Δ Result
Samples 2701 (×3) 2701 (×3)
CPU average 33.3 ms 33.3 ms -0.0 ms ⚪ within noise
CPU 1% worst 33.7 ms (33.4–33.9) 33.5 ms (33.5–33.6) -0.2 ms ⚪ within noise
CPU 0.1% worst 34.1 ms (33.6–36.1) 34.9 ms (34.9–35.6) 0.8 ms ⚪ within noise
GPU average 8.0 ms (7.5–8.5) 8.4 ms (8.4–8.7) 0.4 ms ⚪ within noise
GPU 1% worst 18.3 ms (17.4–21.9) 17.2 ms (17.1–17.4) -1.1 ms ⚪ within noise
GPU 0.1% worst 24.0 ms (23.8–25.5) 18.8 ms (18.7–20.0) -5.2 ms 🟢 22% faster
Exceptions logged 46 42 -4 🟢 fewer errors

@biotech77
biotech77 merged commit 97f8d45 into dev Jun 4, 2026
16 checks passed
@biotech77
biotech77 deleted the fix/7551-private-worlds-asmdef branch June 4, 2026 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TECH DEBT] Private Worlds | move asmdef to social asmdef

4 participants