Merge pull request #405 from RetroPortingToolKit/fix/aot-feature-presentation-keys

aot: presentation keys and exclude_writes; mods: hide_hidden_mod_features

docs/AOT_SHARDING.md

@@ -60,7 +60,7 @@ Profiles declare these reusable methods:
 | `images[].method = fixed_address_files` | Whole original files placed at a verified `load_addr`. Optional duplicate-leaf checks require identical bytes. |
 | `images[].method = fixed_address_extents` | Declared `{file, file_offset, base, address, size, sha256}` byte extents placed at a verified `load_addr`. Offset rules match `words` checks; size and SHA-256 are required. |
 | `mod_packages[]` / `mod_package` | A named, verified selection of a trusted mod package. Images and checks carrying `mod_package` read the original disc with that selection applied; see below. |
-| `images[].transfer_entries` | `{from: "mod_package_writes", count}`: J/JAL targets inside the image, taken only from words the selected package wrote. The count pins the inventory. |
+| `images[].transfer_entries` | `{from: "mod_package_writes", count}`: J/JAL targets inside the image, taken only from words the selected package wrote. The count pins the inventory. Optional `exclude_writes: [{start, end, reason}]` names intervals of package-written DATA (for example a pack linked into the EXE that the package replaced) whose words are not transfers. |
 | `images[].method = tagged_relocated_files` | Apply a trailing two-bit ABS32/HI16/LO16/J26 relocation stream to a verified `load_addr`, retaining only the original loader's image interval. Destinations are explicit evidence, never inferred from a historical RAM capture. |
 | `images[].method = psx_exe` | Read the load address and image from a PS-X EXE header; the generic extractor may split resident and overlay floors. |
 | `images[].method = packed_sector_members` | Decode u32 count/offset sector descriptors across ordered payload files. Verify every member classification and requested full-payload coverage; use loader-established addresses for executable members. |

docs/MOD_PACKAGES.md

@@ -231,6 +231,15 @@ feature-local: `when = { option = "value", ... }` requires every listed option
 to match. The legacy `when_option`/`when_value` pair remains accepted for a
 single condition.
 
+A feature with `hidden = true` is left out of the launcher's lists while it is
+disabled; while enabled it is listed so the player can turn it off. A title that
+sets `[runtime] hide_hidden_mod_features = true` in `game.toml` never shows it:
+the launcher does not list it (enabled or not), does not name it in the lobby
+summary, "Enable all" / "Disable all" leave it alone, and a package whose every
+feature is hidden is not listed under "Installed packages". Either way it runs
+exactly as `default_enabled` and `mods/state.toml` say, so a hidden default-on
+feature is simply active. (recomp-ui `launcher_mod_visibility.h`.)
+
 ## Bounded integer patches
 
 Package format 2 can encode a bounded integer option directly into a guarded

recompiler/src/config_loader.cpp

@@ -458,6 +458,9 @@ static RuntimeConfig parse_runtime_block(const toml::value& cfg, const fs::path&
     if (runtime.contains("bios_hle_keep_intro")) {
         rt.bios_hle_keep_intro = toml::find<bool>(runtime, "bios_hle_keep_intro");
     }
+    if (runtime.contains("hide_hidden_mod_features")) {
+        rt.hide_hidden_mod_features = toml::find<bool>(runtime, "hide_hidden_mod_features");
+    }
     if (runtime.contains("hle_scheduler")) {
         rt.hle_scheduler = toml::find<bool>(runtime, "hle_scheduler");
     }

recompiler/src/config_loader.h

@@ -253,6 +253,14 @@ struct RuntimeConfig {
     bool                  bios_hle = true;
     bool                  bios_hle_keep_intro = false;
 
+    // hide_hidden_mod_features: the launcher never presents a mod feature its
+    // package marks `hidden` -- not even while enabled -- nor a package whose
+    // every feature is hidden, and Enable/Disable all leave such features alone
+    // (recomp-ui RecompLauncherCModProvider::hide_hidden_features). The feature
+    // still runs as its package default and mods/state.toml say. Default false:
+    // a hidden feature is listed while enabled so it can be turned off.
+    bool                  hide_hidden_mod_features = false;
+
     // hle_scheduler: the HLE tier's standing SUBSYSTEM REPLACEMENT for guest
     // thread switching (deterministic TCB scheduler vs the legacy
     // non-deterministic host-fiber bridge). Default ON under BOTH BIOS

runtime/include/mod_runtime.h

@@ -27,6 +27,9 @@ const std::filesystem::path& mod_runtime_effective_disc_path();
 
 #if defined(RECOMP_LAUNCHER)
 const ::RecompLauncherCModProvider* mod_runtime_launcher_provider();
+/* [runtime] hide_hidden_mod_features: the launcher never presents a hidden
+ * feature (recomp-ui RecompLauncherCModProvider::hide_hidden_features). */
+void mod_runtime_set_hide_hidden_features(bool hide);
 #endif
 
 } // namespace PSXRecompV4

runtime/src/main.cpp

@@ -1,4 +1,4 @@
-/* main.cpp — Phase 3 runtime entry point.
+/* main.cpp — Phase 3 runtime entry point.
  *
  * Loads BIOS ROM, initializes CPU state + SDL display, calls into
  * the recompiled reset vector. BIOS drives execution; SDL presents
@@ -13066,6 +13066,10 @@ int main(int argc, char** argv) {
             fast_boot     = gc.runtime.fast_boot;
             bios_hle      = gc.runtime.bios_hle;
             bios_hle_keep_intro = gc.runtime.bios_hle_keep_intro;
+#if defined(RECOMP_LAUNCHER)
+            PSXRecompV4::mod_runtime_set_hide_hidden_features(
+                gc.runtime.hide_hidden_mod_features);
+#endif
             /* Developer compatibility finding, applied before BIOS selection.
              * Not exposed to settings.toml on purpose — see BIOS_SELECTION.md. */
             s_openbios_allowed  = gc.runtime.openbios;

runtime/src/mod_runtime.cpp

@@ -1298,6 +1298,10 @@ const std::filesystem::path& mod_runtime_effective_disc_path() {
 const RecompLauncherCModProvider* mod_runtime_launcher_provider() {
     return &provider;
 }
+
+void mod_runtime_set_hide_hidden_features(bool hide) {
+    provider.hide_hidden_features = hide ? 1 : 0;
+}
 #endif
 
 } // namespace PSXRecompV4

tools/aot_overlay_pipeline.py

@@ -133,13 +133,25 @@ def transfer_entries(source, view):
     A detour farm is entered only through the package's patched transfers, so
     those instructions establish its entry points statically. Unpatched text is
     excluded: stock data words that happen to decode as a jump are not evidence.
+    Neither are package-written DATA words (for example a pack linked into the
+    EXE that the package replaced): `exclude_writes` names such intervals of
+    the loaded image, each with a reason, and their words are not transfers.
     """
     spec = source['spec']['transfer_entries']
     require(spec.get('from') == 'mod_package_writes' and hasattr(view, 'written_words'),
             'transfer_entries requires a mod package view')
+    excluded = []
+    for item in spec.get('exclude_writes', []):
+        start, end = number(item['start']), number(item['end'])
+        require(start < end and start % 4 == end % 4 == 0,
+                'Invalid excluded write interval')
+        require(item.get('reason', '').strip(), 'Excluded write interval needs a reason')
+        excluded.append((start, end))
     lo, hi = source['base'], source['base'] + len(source['body'])
     found = set()
     for address, word in view.written_words():
+        if any(start <= address < end for start, end in excluded):
+            continue
         if word >> 26 in (2, 3):
             target = ((address + 4) & 0xF0000000) | ((word & 0x3FFFFFF) << 2)
             if lo <= target < hi:

tools/mod_package_images.py

@@ -46,14 +46,20 @@ import tomllib
 SECTOR = 2048
 MANIFEST_KEYS = {'format_version', 'id', 'version', 'name', 'author', 'description', 'license',
                  'source_name', 'source_url', 'resolver', 'save_compatibility', 'author_link',
-                 'target', 'feature', 'option', 'plugin', 'patch', 'overlay', 'requirement'}
+                 'target', 'feature', 'option', 'plugin', 'patch', 'overlay', 'requirement',
+                 # Presentation / release metadata the runtime parses
+                 # (mod_packages.cpp); it never changes the image.
+                 'channel'}
 CONDITION_KEYS = {'when', 'when_option', 'when_value'}
 SECTION_KEYS = {
     'patch': {'feature', 'target', 'address', 'offset', 'expected', 'replace'} | CONDITION_KEYS,
     'overlay': {'feature', 'target', 'offset', 'file', 'sha256', 'expected_sha256'} | CONDITION_KEYS,
     'plugin': {'feature', 'id', 'order'} | CONDITION_KEYS,
     'requirement': {'feature', 'package', 'version', 'requires_feature'} | CONDITION_KEYS,
-    'feature': {'id', 'name', 'description', 'group', 'default_enabled'},
+    # author / hidden / channel are launcher presentation and release
+    # filtering (mod_packages.cpp parse); they never change the image.
+    'feature': {'id', 'name', 'author', 'description', 'group', 'default_enabled', 'hidden',
+                'channel'},
     'option': {'feature', 'id', 'label', 'description', 'group', 'type', 'default', 'choice',
                'min', 'max', 'step'},
     'target': {'game_id', 'disc_sha256', 'exe_sha256'},

tools/tests/test_aot_overlay_pipeline.py

@@ -217,6 +217,23 @@ class ModPackageImageTest(unittest.TestCase):
         with self.assertRaisesRegex(ValueError, 'encoded twice'):
             self.view()
 
+    def test_presentation_keys_do_not_change_the_image(self):
+        # hidden / author / channel are launcher and release metadata the
+        # runtime accepts; a hidden default-on feature must still build.
+        base = self.view()
+        text = self.manifest.replace(
+            'format_version = 5\n', 'format_version = 6\nchannel = "stable"\n', 1)
+        text = text.replace(
+            '[[feature]]\nid = "engine"\nname = "Engine"\n',
+            '[[feature]]\nid = "engine"\nname = "Engine"\nauthor = "someone"\n'
+            'hidden = true\ndefault_enabled = true\nchannel = "stable"\n', 1)
+        self.assertNotEqual(text, self.manifest)
+        self.write_manifest(text)
+        view = self.view()
+        self.assertEqual(view.read('GAME.EXE'), base.read('GAME.EXE'))
+        self.assertEqual(view.read('DATA.BIN'), base.read('DATA.BIN'))
+        self.assertEqual(view.plugins, base.plugins)
+
     def test_extent_in_ram_mirror_uses_patched_transfers_as_entries(self):
         inventory = self.prepare(self.profile())
         job, = inventory['jobs']
@@ -228,6 +245,22 @@ class ModPackageImageTest(unittest.TestCase):
         self.assertTrue(record['strict_producer_ranges'])
         self.assertEqual(record['producer_ranges'], [dict(start='0x80780000', end='0x80780020')])
 
+    def test_excluded_package_writes_are_not_transfer_entries(self):
+        # The detour at 0x80010040 lies in an interval the profile declares as
+        # package-written data; only the call at 0x80010048 still counts.
+        exclude = [dict(start='0x80010040', end='0x80010048', reason='written data')]
+        inventory = self.prepare(self.profile(transfer_entries={
+            'from': 'mod_package_writes', 'count': 1, 'exclude_writes': exclude}))
+        self.assertEqual(inventory['jobs'][0]['required_entries'], [self.ENGINE])
+        for item, error in [(dict(start='0x80010040', end='0x80010048'), 'needs a reason'),
+                            (dict(start='0x80010042', end='0x80010048', reason='x'),
+                             'Invalid excluded write'),
+                            (dict(start='0x80010048', end='0x80010040', reason='x'),
+                             'Invalid excluded write')]:
+            with self.subTest(error=error), self.assertRaisesRegex(ValueError, error):
+                self.prepare(self.profile(transfer_entries={
+                    'from': 'mod_package_writes', 'count': 1, 'exclude_writes': [item]}))
+
     def test_extent_evidence_and_inventory_drift_fail_closed(self):
         for changes, error in [(dict(sha256='0' * 64), 'Extent bytes changed'),
                                (dict(transfer_entries={'from': 'mod_package_writes', 'count': 3}),