Repository navigation
Run Retro Rewind's InitScreens patches under the boot-race native - #359
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Runtime call-through runtime/include/abi_bridge.h, runtime/include/guest_function_imports.h, runtime/src/abi_bridge.cpp, runtime/src/hle/boot_race.cpp, translator/tests/Translator.Tests/GuestAddressTableTests.cs |
The runtime adds wrapper registration macros and MKW_GUEST_NEXT, which resolves a translation entry through NextTranslation. The boot-race native uses this call-through, and the guest-address test recognizes the new macro. |
Overlay registration and indexing runtime/src/hle/storage/nand_isfs.cpp, translator/src/Translator.Core/GeneratedMarkers.cs, translator/src/Translator.Core/RuntimeNativeIndex.cs, translator/src/Translator.Core/RuntimeNativeIndexFile.cs, translator/src/Translator.Cli/Program.cs, translator/tests/Translator.Tests/RuntimeNativeIndexTests.cs |
The NAND native is registered as a replacement. The translator extracts and persists wrapper and replacement kinds, includes them in registration ordering, and excludes explicitly classified overlays from native-collision warnings. Tests cover registration kinds, index round-tripping, and wrapper call-through declarations. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix · Severity of issue fixed: Medium
Sequence Diagram(s)
sequenceDiagram
participant BootRace as RacedataScenario_InitScreens
participant Lookup as GuestFunctions::NextTranslation
participant Entry as TranslationEntry
BootRace->>Lookup: resolve guest address through MKW_GUEST_NEXT
Lookup->>Entry: select best non-native entry with a raw invoker
Entry-->>BootRace: return translation entry
Possibly related PRs
- patchzyy/Wiicompiled#346: It adds the boot-race native that this change updates to call through to the next translation.
Merge Risk: 
ab929
-
The wrapper call-through and overlay reporting changes have no established issue requiring a fix before merge.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 11 files. Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: running Retro Rewind's InitScreens patches through the boot-race native. Linked Issues check [ #355] Requirement: the RacedataScenario::InitScreens native at 0x8052F4E8 must not block Retro Rewind's mod overlay at that address. The issue offers two options. This PR implements the second: the n…Out of Scope Changes check Changes group as follows. Each group ties to the PR objective or to #355.
1. Pass-through path: abi_bridge.h (REGISTER_NATIVE_WRAPPER_AS, REGISTER_NATIVE_REPLACEMENT_AS), guest_function_imports.h (MK…Full details: Linked Issues check
-
Explanation
[
#355] Requirement: the RacedataScenario::InitScreens native at 0x8052F4E8 must not block Retro Rewind's mod overlay at that address. The issue offers two options. This PR implements the second: the native's pass-through calls the mod overlay.Evidence:
- runtime/src/hle/boot_race.cpp: RacedataScenario_InitScreens_8052f4e8 calls MKW_GUEST_NEXT(8052F4E8) on the non-CPU path and on the CPU-driven path. The controller-channel save and restore runs only in the CPU-driven path.
- The native is registered with REGISTER_NATIVE_WRAPPER_AS, a macro added in runtime/include/abi_bridge.h.
- runtime/src/abi_bridge.cpp, GuestFunctions::NextTranslation: it selects the best entry at the address that is not Native and has a rawCpuInvoker. IsBetterCandidate ranks priority first, then kind (ModTranslated above BaseTranslated), then moduleId. A mod overlay therefore wins over the base translation. Native entries are excluded, so the wrapper cannot call itself.
- A missing target calls the shared ReportMissingGuestFunction handler, the same handler BaseTranslation uses.
- Translator changes (RuntimeNativeIndex.cs, Program.cs) stop the wrapper native from being reported as a lost overlay, as the PR states.
Remaining uncertainty:
- The RR ghost checks at 0x8052f5c8 and 0x8052f5e4 are inside InitScreens. They run only if the overlay body for 0x8052F4E8 contains those call sites. The overlay body is not in this repository, so this link rests on the issue log and author report, not on code reviewed here.
- The added test RepositoryWrapperNativesCallThroughToTheNextTranslation checks source text only (MKW_GUEST_NEXT present, MKW_GUEST_FUNC absent). No runtime test covers NextTranslation selection. The issue does not request a runtime test.
The --race-cpu controller behavior described in the issue is kept.
Full details: Out of Scope Changes check
-
Explanation
Changes group as follows. Each group ties to the PR objective or to
#355.-
Pass-through path: abi_bridge.h (REGISTER_NATIVE_WRAPPER_AS, REGISTER_NATIVE_REPLACEMENT_AS), guest_function_imports.h (MKW_GUEST_NEXT, NextTranslation), abi_bridge.cpp (ReportMissingGuestFunction extraction and NextTranslation), and boot_race.cpp. This implements
#355's second option. -
Overlay-loss reporting: RuntimeNativeIndex.cs, RuntimeNativeIndexFile.cs (format and extractor version 1 to 2), GeneratedMarkers.cs, and Translator.Cli/Program.cs. These implement the PR statement that wrapper and deliberate replacement natives are no longer reported as lost overlays.
#355quotes the lost-overlay warning. -
nand_isfs.cpp: NAND_IOS_OpenBody_HLE_801938FC changes from REGISTER_NATIVE_FUNCTION_AS to REGISTER_NATIVE_REPLACEMENT_AS. This address is not named in
#355. The macro expands to the same registration with the same arguments, so dispatch does not change. The change applies the PR's reporting rule to this address. Whether this native is a deliberate replacement is not shown in the reviewed diff. The change is therefore related to the PR objective but its reporting effect is unverified. -
Tests: GuestAddressTableTests.cs and RuntimeNativeIndexTests.cs cover the new macros and the wrapper call-through rule.
No change was found that is unrelated to the PR objective.
-
-
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Wrapper natives now call through to the winning mod overlay instead of the base translation, so Retro Rewind's ghost patches in RacedataScenario::InitScreens run again. Natives marked as wrapper or deliberate replacement are no longer reported as lost overlays.
Fixes #355
Summary by CodeRabbit