Fix unresolvable RequestedOrientation cref + sanity-check fixes (oboe teardown race, vulkan unchecked returns, ClicksPerSecond rewind correctness) - #244
Merged
Conversation
Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/8111c8d2-c4b8-44dd-8781-2101b8565806 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
…crash.log) Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/8111c8d2-c4b8-44dd-8781-2101b8565806 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
… returns, dead init flag) Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/723b0a22-1e22-46b1-8a18-b6ec5a94f645 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
…f path Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/723b0a22-1e22-46b1-8a18-b6ec5a94f645 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
winnerspiros
April 22, 2026 21:16
View session
winnerspiros
marked this pull request as ready for review
April 22, 2026 21:26
There was a problem hiding this comment.
Pull request overview
This PR addresses Android build hygiene and stability/correctness improvements across the Android native/managed stack, plus a rewind-correctness fix for the clicks-per-second HUD controller.
Changes:
- Fixes an unresolvable XML doc
crefinOsuGameAndroidby qualifyingRequestedOrientationtoAndroid.App.Activity. - Hardens Android native bridges: adds teardown guards to Oboe recovery paths and checks Vulkan probe API return codes / zero-count branches.
- Fixes ClicksPerSecond counting correctness under gameplay rewind/seek by removing assumptions about monotonic timestamps.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| osu.Game/Screens/Play/HUD/ClicksPerSecond/ClicksPerSecondController.cs | Updates timestamp trimming/counting to be correct with out-of-order timestamps caused by rewind/seek. |
| osu.Android/OsuGameAndroid.cs | Fixes XML doc cref resolution for RequestedOrientation. |
| osu.Android/Native/vulkan_bridge.cpp | Adds return-code checks and early-outs for Vulkan probing enumeration calls. |
| osu.Android/Native/oboe_bridge.h | Introduces an atomic disposing_ flag for teardown coordination. |
| osu.Android/Native/oboe_bridge.cpp | Uses disposing_ to prevent error-callback recovery from racing with teardown. |
| osu.Android/CrashDiagnostics.cs | Makes InstallNativeHandler() idempotent via an early Interlocked guard; removes trailing dead exchange. |
| native_crash.log | Removes an accidentally-committed runtime log from repo root. |
| archive.zip | Removes an accidentally-committed archive from repo root. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+39
to
+43
| // Timestamps are appended at clock.CurrentTime which is *usually* monotonic, but | ||
| // gameplay rewinds (and replay seeks) can append a smaller value after a larger | ||
| // one — so the list is not strictly sorted. We still scan from the end (where | ||
| // newly-appended entries live) to match the access pattern of the previous | ||
| // implementation, but we cannot stop early on either bound because an older |
|
|
||
| std::vector<VkPhysicalDevice> devices(deviceCount); | ||
| if (vkEnumeratePhysicalDevices(instance_, &deviceCount, devices.data()) != VK_SUCCESS) return false; | ||
| if (deviceCount == 0) return false; |
Comment on lines
+52
to
+56
| { | ||
| double t = timestamps[read]; | ||
|
|
||
| if (t > latestValidTime) | ||
| continue; |
Comment on lines
+41
to
+48
| // one — so the list is not strictly sorted. We still scan from the end (where | ||
| // newly-appended entries live) to match the access pattern of the previous | ||
| // implementation, but we cannot stop early on either bound because an older | ||
| // out-of-order entry may live anywhere in the list. | ||
|
|
||
| // First pass: drop any timestamps now in the future (caused by rewinding). | ||
| // Walk backwards and shift surviving entries down in-place; this is O(n) and | ||
| // avoids the O(n²) RemoveAt-in-loop pattern of the original code. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the
Build Android APKwarning atosu.Android/OsuGameAndroid.cs:93(XML comment has cref attribute 'RequestedOrientation' that could not be resolved) and follows up with a full sanity-check pass over the Android stack and fork-introduced changes.Primary fix
RequestedOrientationis inherited fromAndroid.App.Activity; the C# compiler will not resolve it on the derivedOsuGameActivityinside acref. Qualified to the declaring base type:/// Last value passed to <see cref="global::Android.App.Activity.RequestedOrientation"/> by ...Repo hygiene
archive.zip(5.1 MB realm/db dump) andnative_crash.log(4.2 KB device log) accidentally committed at the repo root.Latent issues found and fixed during the sanity check
Native/oboe_bridge.{h,cpp}— added adisposing_atomic set bystop()and re-checked at the entry ofonErrorAfterCloseand insidereopenAndRestart(both pre- and post-streamLock_). Closes the window where Oboe's internal error-callback thread could reopen the stream while .NET is destroying the bridge, leavingrequestStart()running inside a freed object.Native/vulkan_bridge.cpp—vkEnumeratePhysicalDevicesandvkEnumerateDeviceExtensionPropertiesreturn values are now checked; zero-count branches return early before allocating result vectors.CrashDiagnostics.InstallNativeHandler— replaced trailing deadInterlocked.Exchange(ref initialised, 1)with an early-return idempotency guard at function entry, matching the docstring.ClicksPerSecondController.Update— the existing perf rewrite assumedtimestampsis monotonically sorted, butOnPressedappends atclock.CurrentTimewhich can decrease on rewind/seek, so the suffix-only trim and the< earliestTimeValid → breakshort-circuit can both under-count the window. Replaced with an O(n) in-place compaction trim and a full-scan window count — same big-O as the intended optimization, correct under non-monotonic insertions.TestInputsDiscardedOnRewind(monotonic inputs) is unaffected.Re-verified (no change needed)
OsuGameActivityis internally consistent:ScreenOrientation.Landscapein the[Activity]attribute and no runtimeRequestedOrientationwrite inOnCreatefor phones is the deliberate fix from4c2868a46eto avoid SurfaceView recreate during Vulkan-init. The older "SensorLandscape" stored memory was stale.Audit coverage (no further changes)
Android native + managed (input handlers, AndroidImportTask, AndroidHighPerformanceSessionManager, OboeAudioBridge, VulkanProbe, AndroidNativeBridgeManager); build/packaging (
osu.Android.{csproj,props},mono.env,AndroidManifest.xml,CMakeLists.txt,build/PatchElfPageSize.targets— unused but harmless,local-packages/,NuGet.Config); fork changes inOsuGame.cs,OsuGameBase.cs,WebSocketServer.cs,OsuWebSocketProvider.cs, gameplay hot-path LINQ→loop conversions inHitObjectContainer/GameplaySampleTriggerSource/ColourHitErrorMeter; CI workflows (keystore secrets are properly::add-mask::'d before being placed into$GITHUB_OUTPUT); GH advisory DB check on bumped deps (AutoMapper 16, Humanizer 3, SharpCompress 0.47.3, Sentry 6.2.0, …) clean.