Port: keep dungeon monster slots below the sub-game slots

docs/PC.md

@@ -616,7 +616,7 @@ Writes are atomic: the file is written as `config.json.tmp` and renamed over `co
 | `game.auto_use_event_item` | bool | false | | an event that asks for one item the bag holds is handed it without the item picker; see "Auto-use event items" |
 | `game.skip_intro` | bool | false | | boot past the Sony Computer Entertainment text, the copyright notice and the Level-5 logo to the title screen |
 | `game.floor_info_monsters` | bool | false | | Triangle on a dungeon floor's information lists its monsters (spoilers); see "Floor monster list" |
-| `game.more_monsters` | bool | false | | a dungeon floor holds up to 60 monsters instead of 24, and every monster in range shows and acts; see "Dungeon monster limit" |
+| `game.more_monsters` | bool | false | | a dungeon floor holds up to 40 monsters instead of 24, and every monster in range shows and acts; see "Dungeon monster limit" |
 | `video.present_mode` | string | fifo | fifo, mailbox, immediate | Vulkan present mode |
 | `video.interpolation` | bool | true | | interpolate display frames between ticks |
 | `video.max_fps` | number | 0 | 0 or more | display frame cap; 0 is unlimited |
@@ -1169,9 +1169,13 @@ on in the save root's `config.json`.
 
 Retail keeps 24 monster slots, scene characters `DNG_CHARA_SLOT_MONSTER` (24) to 47, and only the five nearest
 monsters (`CMonsterMan::priority_limit`) show and act; `CActiveMonster::CheckView` fades the rest out and `ThinkHost`
-skips them. The port's mirror of `monster.hpp` raises `MONSTER_ACTIVE_MAX` to 60, so the slots run to scene character
-83 of 128. `CMonsterMan::Initialize` reads `game.more_monsters` once per floor through `PortMonsterLatchLimits`
-(`port/src/monster.cpp`): off, the floor uses 24 slots and a priority limit of 5, exactly as retail; on, it uses all 60
+skips them. The port's mirror of `monster.hpp` raises `MONSTER_ACTIVE_MAX` to 40, so the slots run to scene character
+63. They must stay below the sub-game slots (`SUBGAME_CHARA_BASE`, 64): `InitDungeonMain` clears those through
+`InitSubGame` after binding the monsters, so a monster slot there reads back empty and the floor's
+`CMonsterMan::Initialize` crashes; a static assert in `port/src/monster.cpp` holds the range below them.
+`CMonsterMan::Initialize` reads `game.more_monsters` once per floor through `PortMonsterLatchLimits`
+(`port/src/monster.cpp`): off, the floor spawns into retail's 24 slots with its priority limit of 5, though all 40 monster objects are still
+initialised and scanned; on, it uses all 40
 and every monster in range shows and acts. Unit fixups hold `AutoSetMonster`'s spawn count,
 `CMonsterMan::SearchActiveMonsterBlock` and the event scene's slot memory to that limit, and `PrepareMonsterMemory`
 (`port/src/dng_event.cpp`) carves script memory only for the slots in use. A floor's placement list holds at most 32

port/src/monster.cpp

@@ -29,6 +29,7 @@
 #include "platform/model_assets.hpp"
 #include "snd_mngr.hpp"
 #include "sound_port.hpp"
+#include "subgame.hpp"
 #include "swordeffect.hpp"
 
 namespace {
@@ -185,6 +186,11 @@ int active_monsters = kRetailActiveMonsters;
 
 } // namespace
 
+// InitDungeonMain's InitSubGame clears the sub-game slots after the monsters are bound, and LoadDungeonMapFile's
+// CMonsterMan::Initialize then reads every monster slot back.
+static_assert(static_cast<int>(DNG_CHARA_SLOT_MONSTER_END) <= static_cast<int>(SUBGAME_CHARA_BASE),
+              "monster scene slots overlap the sub-game slots");
+
 int PortMonsterLatchLimits() {
     const bool more = ConfigGet().more_monsters;
     active_monsters = more ? MONSTER_ACTIVE_MAX : kRetailActiveMonsters;

port/src/options/rows.cpp

@@ -187,7 +187,7 @@ std::span<const Page> Pages() {
                              Choice<&Config::skip_intro>("game.skip_intro", "Skip intro", "Boots past the Sony text, the copyright notice\nand the Level-5 logo to the title screen.", {"Off", "On"}),
                              Choice<&Config::auto_use_event_item>("game.auto_use_event_item", "Auto-use event items", "An event that asks for one item you carry\nis handed it without the item picker.", {"Off", "On"}),
                              Choice<&Config::floor_info_monsters>("game.floor_info_monsters", "Floor monster list", "Triangle on a floor's info lists its monsters, drops and\ngate key carrier (spoilers). Off is the original game.", {"Off", "On"}),
-                             Choice<&Config::more_monsters>("game.more_monsters", "More monsters", "Floors hold up to 60 monsters instead of 24, and all of\nthem in range show and act. Applies from the next floor.", {"Off", "On"}),
+                             Choice<&Config::more_monsters>("game.more_monsters", "More monsters", "Floors hold up to 40 monsters instead of 24, and all of\nthem in range show and act. Applies from the next floor.", {"Off", "On"}),
                              Choice<&Config::debug_mode>("game.debug_mode", "Debug mode", "Unlocks the developer menu and debug controls.\nThe boot unlock applies from the next start.", {"Off", "On"}),
                          }        },
         {.name = "Bindings", .rows = Bindings()           },

port/src/platform/config.hpp

@@ -33,7 +33,7 @@ struct Config : HostSettings {
     bool auto_use_event_item = false;
     // game.floor_info_monsters: Triangle on a dungeon floor's information lists its monsters and their drops.
     bool floor_info_monsters = false;
-    // game.more_monsters: a dungeon floor holds up to 60 monsters instead of 24, and all of them in range show and act.
+    // game.more_monsters: a dungeon floor holds up to 40 monsters instead of 24, and all of them in range show and act.
     bool         more_monsters = false;
     ConfigAspect aspect = ConfigAspect::Auto;
     float        ui_scale = 1.0f;

port/src/tests/monster_memory_test.cpp

@@ -5,6 +5,7 @@
 #include <filesystem>
 #include <format>
 #include <memory>
+#include <type_traits>
 #include <vector>
 
 #include "dng_event.hpp"
@@ -20,6 +21,7 @@
 #include "platform_fixture.hpp"
 #include "runscript.hpp"
 #include "scene.hpp"
+#include "subgame.hpp"
 
 namespace {
 
@@ -166,6 +168,28 @@ TEST_P(MonsterMemory, ModelCopySizesItsSlotBeforeAllocatingScriptStacks) {
     EXPECT_GT(second_capacity, first_capacity);
 }
 
+// Every dungeon entry clears the sub-game character slots (InitDungeonMain -> InitSubGame) after the monsters are
+// bound and before the floor script's LoadDungeonMapFile binds them again, so no monster slot may lie among them.
+TEST(MonsterSlots, SurviveTheSubGameClear) {
+    auto                                          player = std::make_unique<CCharacter2>();
+    auto                                          monsters = std::make_unique<CActiveMonster[]>(MONSTER_ACTIVE_MAX);
+    std::unique_ptr<CScene, decltype(&std::free)> scene(
+        static_cast<CScene *>(std::calloc(1, sizeof(CScene))), &std::free);
+    ASSERT_NE(scene, nullptr);
+    scene->chara_num = std::extent_v<decltype(CScene::chara)>;
+    ASSERT_NE(scene->AssignChara(DNG_CHARA_SLOT_PARTY, player.get(), nullptr), -1);
+    for (int i = 0; i < MONSTER_ACTIVE_MAX; ++i) {
+        ASSERT_NE(scene->AssignChara(DNG_CHARA_SLOT_MONSTER + i, &monsters[i], nullptr), -1) << i;
+    }
+
+    InitSubGame(scene.get());
+
+    EXPECT_LE(DNG_CHARA_SLOT_MONSTER_END, SUBGAME_CHARA_BASE);
+    for (int i = 0; i < MONSTER_ACTIVE_MAX; ++i) {
+        EXPECT_EQ(scene->GetCharacter(DNG_CHARA_SLOT_MONSTER + i), &monsters[i]) << i;
+    }
+}
+
 INSTANTIATE_TEST_SUITE_P(MoreMonsters, MonsterMemory, testing::Bool(),
                          [](const testing::TestParamInfo<bool> &info) { return info.param ? "On" : "Off"; });
 

scripts/port/header_fixups.py

@@ -33,8 +33,9 @@
     ("dng_main.hpp", "    DNG_CHARA_SLOT_MONSTER = 24, /**< First slot of the MONSTER_ACTIVE_MAX active monsters. */",
      "    DNG_CHARA_SLOT_MONSTER = 24, /**< First slot of the MONSTER_ACTIVE_MAX active monsters. */"
      " DNG_CHARA_SLOT_MONSTER_END = DNG_CHARA_SLOT_MONSTER + MONSTER_ACTIVE_MAX,"),
-    # A dungeon floor holds up to 60 monsters with game.more_monsters; the floors use retail's 24 slots otherwise.
-    ("monster.hpp", "    MONSTER_ACTIVE_MAX = 24,", "    MONSTER_ACTIVE_MAX = 60,"),
+    # A dungeon floor holds up to 40 monsters with game.more_monsters; the floors use retail's 24 slots otherwise.
+    # Their scene slots must end at SUBGAME_CHARA_BASE (64): every dungeon entry clears the sub-game slots from there.
+    ("monster.hpp", "    MONSTER_ACTIVE_MAX = 24,", "    MONSTER_ACTIVE_MAX = 40,"),
     ("dng_hud.hpp", "    virtual void Draw();", "    virtual int Draw();"),
     # The quick loadout's "Equip Slot" command wraps the item command list; retail's routine stays under this name.
     ("menusys.hpp", "    int MenuItemCommandSelect(int select_key, int push_button);",

scripts/port/test_header_fixups.py

@@ -1,3 +1,4 @@
+import re
 import unittest
 from pathlib import Path
 import header_fixups
@@ -111,6 +112,19 @@ def test_monster_scene_slots_start_at_the_monster_slot(self):
             self.assertNotIn("MONSTER_ACTIVE_MAX * 2", result)
             self.assertNotRegex(result, r"(?<!DNG_CHARA_SLOT_MONSTER) \+ MONSTER_ACTIVE_MAX\b|[=-] MONSTER_ACTIVE_MAX;")
 
+    def test_monster_scene_slots_end_before_the_subgame_slots(self):
+        mirror = header_fixups.build_mirror(Path(__file__).resolve().parents[2] / "ps2/include")
+
+        def value(header, name):
+            match = re.search(rf"\b{name} = (0x[0-9A-Fa-f]+|\d+),", mirror[header].decode("latin-1"))
+            self.assertIsNotNone(match, name)
+            return int(match.group(1), 0)
+
+        active = value("monster.hpp", "MONSTER_ACTIVE_MAX")
+        self.assertGreater(active, 24)
+        self.assertLessEqual(value("dng_main.hpp", "DNG_CHARA_SLOT_MONSTER") + active,
+                             value("subgame.hpp", "SUBGAME_CHARA_BASE"))
+
     def test_georama_tables_grow_with_the_town(self):
         source = Path(__file__).resolve().parents[2] / "ps2/include"
         mirror = header_fixups.build_mirror(source)