From 11eb5fe7049fb96214f0318d3ae8c679ea1a083f Mon Sep 17 00:00:00 2001 From: carrdylan52 <17089587+carrdylan52@users.noreply.github.com> Date: Sun, 18 Oct 2026 09:43:17 -0400 Subject: [PATCH] Port: keep dungeon monster slots below the sub-game slots Merging PR #59 raised MONSTER_ACTIVE_MAX to 60 in the header mirror, so the active monsters became scene characters 24 to 83. Scene characters 64 to 103 (SUBGAME_CHARA_BASE, SUBGAME_CHARA_NUM) belong to the sub-games, and InitDungeonMain calls InitSubGame after binding the monsters, which clears them. The floor script's LoadDungeonMapFile then runs CMonsterMan::Initialize, which reads every one of the 60 slots back and calls Initialize on monster 40's now-empty slot: a null dereference on every dungeon entry, with game.more_monsters on or off, since the loop runs to MONSTER_ACTIVE_MAX either way. MONSTER_ACTIVE_MAX is now 40, so the slots end at scene character 63. A floor's placement list holds at most 32 monsters, so the setting still raises the opening count as far as before. A static assert in port/src/monster.cpp holds the range below SUBGAME_CHARA_BASE; a unit test runs InitSubGame over a full 128-slot scene and a header fixup test checks the mirrored constants. --- docs/PC.md | 12 ++++++++---- port/src/monster.cpp | 6 ++++++ port/src/options/rows.cpp | 2 +- port/src/platform/config.hpp | 2 +- port/src/tests/monster_memory_test.cpp | 24 ++++++++++++++++++++++++ scripts/port/header_fixups.py | 5 +++-- scripts/port/test_header_fixups.py | 14 ++++++++++++++ 7 files changed, 57 insertions(+), 8 deletions(-) diff --git a/docs/PC.md b/docs/PC.md index 2d4569e17..5d60866e1 100644 --- a/docs/PC.md +++ b/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 diff --git a/port/src/monster.cpp b/port/src/monster.cpp index 5ed0fe12c..0a8212fec 100644 --- a/port/src/monster.cpp +++ b/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(DNG_CHARA_SLOT_MONSTER_END) <= static_cast(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; diff --git a/port/src/options/rows.cpp b/port/src/options/rows.cpp index a8b29ef1f..6d5141ff8 100644 --- a/port/src/options/rows.cpp +++ b/port/src/options/rows.cpp @@ -187,7 +187,7 @@ std::span 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() }, diff --git a/port/src/platform/config.hpp b/port/src/platform/config.hpp index 6247a1d4c..09f934fff 100644 --- a/port/src/platform/config.hpp +++ b/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; diff --git a/port/src/tests/monster_memory_test.cpp b/port/src/tests/monster_memory_test.cpp index b5011b393..7c192f2c7 100644 --- a/port/src/tests/monster_memory_test.cpp +++ b/port/src/tests/monster_memory_test.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #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(); + auto monsters = std::make_unique(MONSTER_ACTIVE_MAX); + std::unique_ptr scene( + static_cast(std::calloc(1, sizeof(CScene))), &std::free); + ASSERT_NE(scene, nullptr); + scene->chara_num = std::extent_v; + 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 &info) { return info.param ? "On" : "Off"; }); diff --git a/scripts/port/header_fixups.py b/scripts/port/header_fixups.py index e512ef753..be0038e79 100644 --- a/scripts/port/header_fixups.py +++ b/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);", diff --git a/scripts/port/test_header_fixups.py b/scripts/port/test_header_fixups.py index 936ee40b0..3424c29f9 100644 --- a/scripts/port/test_header_fixups.py +++ b/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"(?