fix: stop showing nearby voice chat intro tip - #9698
Conversation
🚦 CI StatusBuild failed! Check the logs to see what went wrong. Warnings not reduced: 13292 => 13292 — remove at least 1 warning to merge. Warnings/errors in files changed by this PR (3)
|
|
Slack notification sent to #explorer-ext-contributions for external review. |
decentraland-bot
left a comment
There was a problem hiding this comment.
Review — fix: stop showing nearby voice chat intro tip
STEP 2 — Root-cause check: PASS ✅
The PR removes an outdated Nearby Voice Chat intro/tip banner (NearbyVoiceTipFlow). The change directly addresses the intent — eliminating the feature entirely rather than working around it. This is a deliberate removal, not a symptom patch.
STEP 3 — Design & integration: PASS ✅
No new units are introduced. This is a pure deletion of:
NearbyVoiceTipViewfield + constructor parameter fromVoiceChatPluginnearbyTipCts(CancellationTokenSource) field + itsSafeCancelAndDispose()inDispose()RunNearbyVoiceTipAsyncstatic methodNearbyVoiceTipFlownested static classSidebarView.NearbyVoiceTipargument fromDynamicWorldContainer
Teardown trace: The removed nearbyTipCts.SafeCancelAndDispose() was the only teardown for nearbyTipCts, and both are removed together — no leak introduced. All remaining voice chat initialization (button controller, widget controller, state model, audio systems, mute service, suppressor) is untouched and correctly scoped via pluginScope.
loadingStatus field: Still consumed by NearbyVoiceChatSuppressor — not dead after this removal.
STEP 4 — Member audit: PASS ✅
No new members added. All removed members (nearbyVoiceTipView, nearbyTipCts, constructor parameter, methods, nested class) are consistently removed as a unit.
STEP 5 — Line-level review: No issues found
The deletion is clean and consistent across both files. No orphaned references, no broken constructor signatures, no missing dispose paths.
[P2] Leftover dead assets (acknowledged): After this PR, the following become unreferenced from runtime code:
NearbyVoiceTipViewclass (Explorer/Assets/DCL/VoiceChat/NearbyVoiceChat/UI/NearbyVoiceTipView.cs)SidebarView.NearbyVoiceTipserialized property (Explorer/Assets/DCL/UI/Sidebar/SidebarView.cs)DCLPrefKeys.NEARBY_VOICE_TIP_DISMISSEDconstant
The PR description explicitly states these are left in place to avoid Unity prefab serialization risk without editor validation — this is a reasonable scope decision. Consider filing a follow-up issue to clean up the dead view class, serialized field, and pref key constant.
Git conventions (ADR-6): PASS ✅
- PR title:
fix: stop showing nearby voice chat intro tip— valid semantic commit format - Branch:
fix/9696-remove-nearby-voice-tip— matches<type>/<summary>convention
Security review: No security issues found
Pure UI code removal with no auth, input handling, or data exposure implications.
REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Deletion-only change in 2 files removing an unused UI tip banner from VoiceChatPlugin — no ECS, async, or architectural changes.
QA_REQUIRED: YES
Reviewed by Jarvis 🤖 · Requested via GitHub
|
Superseded by: #9698 |
What does this PR change?
Stops the outdated Nearby Voice Chat intro/tip banner from being started by
VoiceChatPlugin.The change removes the tip-specific runtime flow (
RunNearbyVoiceTipAsync,NearbyVoiceTipFlow, and its cancellation token/constructor wiring) while keeping the Nearby Voice Chat feature initialization intact: button controller, widget controller, state model, audio systems, mute service, suppressor, and feature flag behavior are unchanged.Closes #9696
Plan
The implementation was based on
docs/plans/2026-08-12-fix-remove-nearby-voice-tip-plan.md:NearbyVoiceTipView/tip cancellation state fromVoiceChatPluginconstructor and fields.RunNearbyVoiceTipAsynccall and all tip flow methods fromVoiceChatPlugin.DynamicWorldContainerto stop passingSidebarView.NearbyVoiceTipintoVoiceChatPlugin.Test Instructions
Steps (standard run):
metaforge explorer run XXXX # ← replace with this PR numberExpected result:
FeatureId.NearbyVoiceChatis enabled.Steps (fresh account):
metaforge account create --clear metaforge explorer run XXXX # ← replace with this PR numberExpected result:
Automation (if applicable):
metaforge explorer test XXXXPrerequisites
Test Steps
Additional Testing Notes
bash scripts/lint/lint-changed.shwas attempted; it skipped because ReSharper CLI is not installed in this environment.make test-editmode TEST_FILTER=NearbyVoiceChatStateModelShouldwas attempted; it could not run becausemakeis not available in this environment.Quality Checklist
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.
Requested by Ignacio Mazzara via Slack
🤖 Created via Slack with Claude