Skip to content

chore: await or forget abandoned async calls#9044

Open
NickKhalow wants to merge 1 commit into
devfrom
chore/fix-abandoned-async-calls
Open

chore: await or forget abandoned async calls#9044
NickKhalow wants to merge 1 commit into
devfrom
chore/fix-abandoned-async-calls

Conversation

@NickKhalow

@NickKhalow NickKhalow commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Pull Request Description

What does this PR change?

Fixes abandoned / mislabeled async calls surfaced by the compiler warnings CS4014 (call not awaited) and CS1998 (async method without await). These are correctness/readability hazards: un-awaited tasks run detached with unobserved exceptions, and async methods with no await allocate a state machine while running synchronously and misleading the reader.

CS4014 — un-awaited calls (now explicitly .Forget()):

  • PluginSystem/Global/FriendsContainer.csLaunchSubscriptionsAsync(...) was fire-and-forget without .Forget(); now matches the sibling PrewarmAsync(...).Forget() in the same method.
  • RestrictedActions/Tests/GlobalWorldActionsShould.cs (×4) — TriggerSceneEmoteAsync(...) calls; the tests assert the synchronous side effects, so .Forget() makes the fire-and-forget intent explicit.

CS1998 — async methods with no await (converted to non-async UniTask-returning):

  • Empty / synchronous InitializeAsync / EnableAsync in the MapRenderer layer controllers (CategoryMarkersController, LiveEventsMarkersController, ScenesOfInterestMarkersController, SearchResultMarkersController) and AudioAnalysisPluginUniTask.CompletedTask.
  • LightSourcePlugin.CreateLightSourcePoolAsync → returns UniTask.CompletedTask (body is fully synchronous).
  • SceneAdminsChatCommand.ExecuteCommandAsync and IRealmController null-object IsReachableAsyncUniTask.FromResult(...).
  • UnityLocalizationSceneTipsProvider.GetAsyncUniTask.FromResult(fallbackTips) (the real async body is currently commented out).
  • VoiceChat/Microphone/MicrophoneTrackPublisher.CreateMicrophoneSourceAsync — the only await is behind #if UNITY_STANDALONE_OSX, so non-macOS builds legitimately have no await; wrapped in a scoped #pragma warning disable/restore CS1998 rather than adding a no-op await.
  • StreamableLoading/Tests/LoadSystemBaseOngoingRequestRaceShould.cs — the race test drives the system synchronously and never awaits; converted async Taskvoid and dropped the now-unused System.Threading.Tasks import.

No behavioral change is intended — every call site preserves its prior runtime semantics; the fixes only make intent explicit and remove the spurious async state machines.

Test Instructions

This is a pure correctness/warnings cleanup with no expected behavioral change — primarily a sanity check that nothing regressed.

Steps (standard run):

metaforge explorer run XXXX  # ← replace with this PR number

Expected result:

  • App boots normally. Friends pre-warm/subscriptions still work, the map layers (categories, live events, points of interest, search results) still populate, light-source SDK components still load, voice chat microphone publishing still works, and the /scene-admins chat command still returns the admin list.

Test Steps

  1. Happy path, no behaviour should change against the dev branch

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

@NickKhalow
NickKhalow requested review from a team as code owners June 22, 2026 14:15
@github-actions
github-actions Bot requested a review from DafGreco June 22, 2026 14:15
@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

@NickKhalow NickKhalow changed the title fix: await or forget abandoned async calls chore: await or forget abandoned async calls Jun 22, 2026
@claude

claude Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @NickKhalow's task in 3m 2s —— View job


PR Review

  • Load project docs and context
  • Read diff and changed files
  • Analyze for correctness, async patterns, and code standards
  • Post review findings

Summary

What this PR solves: CS4014 (un-awaited UniTask calls) and CS1998 (async methods with no await) compiler warnings. The diff fixes the root cause in each case — it is not suppressing symptoms. Every changed call site either makes fire-and-forget intent explicit with .Forget(), or removes unnecessary async state machine allocation by returning UniTask.CompletedTask / UniTask.FromResult(...) directly. No behavioral change is intended and none is introduced.


No Blocking Issues Found

All 13 files pass review:

File Change Verdict
FriendsContainer.cs LaunchSubscriptionsAsync(...).Forget() ✅ Consistent with sibling PrewarmAsync(...).Forget() in the same method
GlobalWorldActionsShould.cs (×4) .Forget() on sync-test emote calls ✅ Tests are public void, sync side effects observed before any yield point
LoadSystemBaseOngoingRequestRaceShould.cs async Taskvoid, drops unused System.Threading.Tasks import ✅ Body contains no await; void test is correct
IRealmController.cs async UniTask<bool>UniTask.FromResult(false) in null-object
CategoryMarkersController.cs Empty InitializeAsyncUniTask.CompletedTask; EnableAsync → non-async with explicit return
LiveEventsMarkersController.cs Same empty InitializeAsync pattern
ScenesOfInterestMarkersController.cs Same
SearchResultMarkersController.cs Same for both InitializeAsync and EnableAsync
AudioAnalysisPlugin.cs Same empty InitializeAsync pattern
LightSourcePlugin.cs CreateLightSourcePoolAsync → non-async
UnityLocalizationSceneTipsProvider.cs GetAsyncUniTask.FromResult(fallbackTips) ✅ Value semantics preserved; commented-out real implementation unaffected
SceneAdminsChatCommand.cs Non-async ExecuteCommandAsync with UniTask.FromResult ✅ Both early-return paths covered
MicrophoneTrackPublisher.cs #pragma warning disable CS1998 inside #if !UNITY_STANDALONE_OSX ✅ Pragma applies only on non-macOS where no await exists; macOS path unchanged

Non-Blocking Note

LightSourcePlugin.CreateLightSourcePoolAsync (LightSourcePlugin.cs:83) retains the CancellationToken ct parameter, which is now unused after removing async. This was a pre-existing dead parameter — not introduced by this PR, not a blocker.


REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Modifies async/UniTask patterns across plugin initialization, map renderer controllers, voice chat, and test infrastructure spanning 13 files.
QA_REQUIRED: YES

@github-actions

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.

@balducciv

Copy link
Copy Markdown

✅ PR reviewed and approved by QA on Mac following the PR test instructions.
✅ Smoke test performed on macOS to verify no regressions were introduced.

Build: v0.152.0-alpha-chore/fix-abandoned-async-calls-e0658ae
OS: macOS 15.7.4 — Apple M3 Pro

Test results:

  • App boots normally — Loading stage: Completed confirmed in Player.log (multiple realm changes also completed successfully)
  • FriendsContainer loaded without exceptions — GlobalPlugin FriendsContainer initialized in 3ms with no subscription errors
  • Map layers initialized — all map layer plugins loaded (no InitializeAsync/EnableAsync exceptions)
  • LightSourcePlugin loaded cleanly — ECSWorldPlugin LightSourcePlugin initialized in 0ms, no exceptions
  • AudioAnalysisPlugin loaded cleanly — ECSWorldPlugin AudioAnalysisPlugin initialized in 0ms, no exceptions
  • No unhandled exceptions from any PR-touched systems (FriendsContainer, map controllers, LightSourcePlugin, AudioAnalysisPlugin, SceneAdminsChatCommand, IRealmController, MicrophoneTrackPublisher)
  • Clean shutdown — OnDestroy successfully finished in 367ms, no disposal-time exceptions from modified code

Unrelated errors noted (do not affect verdict):

  • TimeoutException: MultiThreadSync timeout (×21) — ECS lock contention in scene threads at coords (-3,-2) and (-4,-4) around 15:52–15:53; pre-existing issue unrelated to this PR's async cleanup
  • Exception: Failed to load image from data (×1) — metamorph-api/convert web request failure for a wearable thumbnail; unrelated network issue
  • DOTWEEN safe mode tween failure at shutdown — known disposal noise

Verdict: PASS ✅

Note: Tested on Mac only. No behavioral changes were expected per PR description — validation confirms all touched systems (Friends, map layers, light sources, audio analysis, microphone, realm controller, scene admins) initialize and run without regression.

Player.log

22.06.2026_17.46.04_REC.mp4

@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. ✅

Smoke test passed with no regressions. App boots normally, friends subscriptions work, map layers populate correctly, voice chat microphone publishing works, and /scene-admins command returns the admin list as expected. LGTM 🚀

@NickKhalow
NickKhalow enabled auto-merge (squash) June 22, 2026 20:50
Comment on lines +199 to +202
// suppress CS1998 on other platforms.
#if !UNITY_STANDALONE_OSX
#pragma warning disable CS1998
#endif

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.

Not very happy with this one but I guess there's no other way given the OSX specifics inside the method.

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.

4 participants