From 88b433841613b2d5b9eca52df19e89ea5c55d9f0 Mon Sep 17 00:00:00 2001 From: hidwood <78058766+hidwood@users.noreply.github.com> Date: Fri, 30 Jan 2026 15:42:53 -0500 Subject: [PATCH] fix archway door position in auto-walk path detection Use object->position instead of the archway tile position when reporting door locations in FindFirstClosedDoorOnWalkPath and FindFirstTrackerPathBlock. Archway tiles (negative dObject refs) resolve to the parent door object, but the returned position was the archway tile rather than the door itself, causing the player to stop one tile short. Also: move DOOR_CLOSED/DOOR_OPEN/DOOR_BLOCKED enum from objects.cpp to objects.h so diablo.cpp can use the canonical constants directly instead of maintaining duplicate constexpr values. Add static_assert for MAXOBJECTS fitting in int8_t, defensive assertions in AddDoor(), and clarify comments around door state checks. Co-Authored-By: Claude Opus 4.5 --- Source/diablo.cpp | 96 ++++++++++++++--------------------- Source/objects.cpp | 17 +++---- Source/objects.h | 11 ++++ test/tile_properties_test.cpp | 2 +- 4 files changed, 58 insertions(+), 68 deletions(-) diff --git a/Source/diablo.cpp b/Source/diablo.cpp index f8d0fece9..9dbece3d7 100644 --- a/Source/diablo.cpp +++ b/Source/diablo.cpp @@ -5,8 +5,8 @@ */ #include #include -#include #include +#include #include #include #include @@ -74,9 +74,9 @@ #include "levels/gendung.h" #include "levels/setmaps.h" #include "levels/themes.h" +#include "levels/tile_properties.hpp" #include "levels/town.h" #include "levels/trigs.h" -#include "levels/tile_properties.hpp" #include "lighting.h" #include "loadsave.h" #include "lua/lua_global.hpp" @@ -88,15 +88,15 @@ #include "nthread.h" #include "objects.h" #include "options.h" +#include "panels/charpanel.hpp" #include "panels/console.hpp" #include "panels/info_box.hpp" -#include "panels/charpanel.hpp" #include "panels/partypanel.hpp" #include "panels/spell_book.hpp" #include "panels/spell_list.hpp" #include "pfile.h" -#include "portal.h" #include "plrmsg.h" +#include "portal.h" #include "qol/chatlog.h" #include "qol/floatingnumbers.h" #include "qol/itemlabels.h" @@ -175,13 +175,13 @@ namespace { char gszVersionNumber[64] = "internal version unknown"; - void SelectNextTownNpcKeyPressed(); - void SelectPreviousTownNpcKeyPressed(); - void UpdateAutoWalkTownNpc(); - void UpdateAutoWalkTracker(); - void AutoWalkToTrackerTargetKeyPressed(); - void SpeakSelectedSpeedbookSpell(); - void SpellBookKeyPressed(); +void SelectNextTownNpcKeyPressed(); +void SelectPreviousTownNpcKeyPressed(); +void UpdateAutoWalkTownNpc(); +void UpdateAutoWalkTracker(); +void AutoWalkToTrackerTargetKeyPressed(); +void SpeakSelectedSpeedbookSpell(); +void SpellBookKeyPressed(); std::optional> FindKeyboardWalkPathForSpeech(const Player &player, Point startPosition, Point destinationPosition, bool allowDestinationNonWalkable = false); std::optional> FindKeyboardWalkPathForSpeechRespectingDoors(const Player &player, Point startPosition, Point destinationPosition, bool allowDestinationNonWalkable = false); std::optional> FindKeyboardWalkPathForSpeechIgnoringMonsters(const Player &player, Point startPosition, Point destinationPosition, bool allowDestinationNonWalkable = false); @@ -189,7 +189,7 @@ std::optional> FindKeyboardWalkPathForSpeechRespectingDoorsI std::optional> FindKeyboardWalkPathForSpeechLenient(const Player &player, Point startPosition, Point destinationPosition, bool allowDestinationNonWalkable = false); std::optional> FindKeyboardWalkPathToClosestReachableForSpeech(const Player &player, Point startPosition, Point destinationPosition, Point &closestPosition); void AppendKeyboardWalkPathForSpeech(std::string &message, const std::vector &path); - void AppendDirectionalFallback(std::string &message, const Displacement &delta); +void AppendDirectionalFallback(std::string &message, const Displacement &delta); bool gbGameLoopStartup; bool forceSpawn; @@ -666,8 +666,8 @@ void PressKey(SDL_Keycode vkey, uint16_t modState) if (next) { MyPlayer->_pRSpell = *next; MyPlayer->_pRSplType = (MyPlayer->_pAblSpells & GetSpellBitmask(*next)) != 0 ? SpellType::Skill - : (MyPlayer->_pISpells & GetSpellBitmask(*next)) != 0 ? SpellType::Charges - : SpellType::Spell; + : (MyPlayer->_pISpells & GetSpellBitmask(*next)) != 0 ? SpellType::Charges + : SpellType::Spell; UpdateSpellTarget(*next); RedrawEverything(); SpeakText(pgettext("spell", GetSpellData(*next).sNameText), /*force=*/true); @@ -699,8 +699,8 @@ void PressKey(SDL_Keycode vkey, uint16_t modState) if (next) { MyPlayer->_pRSpell = *next; MyPlayer->_pRSplType = (MyPlayer->_pAblSpells & GetSpellBitmask(*next)) != 0 ? SpellType::Skill - : (MyPlayer->_pISpells & GetSpellBitmask(*next)) != 0 ? SpellType::Charges - : SpellType::Spell; + : (MyPlayer->_pISpells & GetSpellBitmask(*next)) != 0 ? SpellType::Charges + : SpellType::Spell; UpdateSpellTarget(*next); RedrawEverything(); SpeakText(pgettext("spell", GetSpellData(*next).sNameText), /*force=*/true); @@ -748,8 +748,8 @@ void PressKey(SDL_Keycode vkey, uint16_t modState) if (first) { MyPlayer->_pRSpell = *first; MyPlayer->_pRSplType = (MyPlayer->_pAblSpells & GetSpellBitmask(*first)) != 0 ? SpellType::Skill - : (MyPlayer->_pISpells & GetSpellBitmask(*first)) != 0 ? SpellType::Charges - : SpellType::Spell; + : (MyPlayer->_pISpells & GetSpellBitmask(*first)) != 0 ? SpellType::Charges + : SpellType::Spell; UpdateSpellTarget(*first); RedrawEverything(); SpeakText(pgettext("spell", GetSpellData(*first).sNameText), /*force=*/true); @@ -775,8 +775,8 @@ void PressKey(SDL_Keycode vkey, uint16_t modState) if (first) { MyPlayer->_pRSpell = *first; MyPlayer->_pRSplType = (MyPlayer->_pAblSpells & GetSpellBitmask(*first)) != 0 ? SpellType::Skill - : (MyPlayer->_pISpells & GetSpellBitmask(*first)) != 0 ? SpellType::Charges - : SpellType::Spell; + : (MyPlayer->_pISpells & GetSpellBitmask(*first)) != 0 ? SpellType::Charges + : SpellType::Spell; UpdateSpellTarget(*first); RedrawEverything(); SpeakText(pgettext("spell", GetSpellData(*first).sNameText), /*force=*/true); @@ -1976,11 +1976,6 @@ void UpdateAttackableMonsterAnnouncements() SpeakText(name, /*force=*/true); } -// Door state values are defined in `Source/objects.cpp` (DOOR_CLOSED=0, DOOR_OPEN=1, DOOR_BLOCKED=2). -constexpr int DoorClosed = 0; -constexpr int DoorOpen = 1; -constexpr int DoorBlocked = 2; - [[nodiscard]] StringOrView DoorLabelForSpeech(const Object &door) { if (!door.isDoor()) @@ -1988,11 +1983,11 @@ constexpr int DoorBlocked = 2; // Catacombs doors are grates, so differentiate them for the screen reader / tracker. if (IsAnyOf(door._otype, _object_id::OBJ_L2LDOOR, _object_id::OBJ_L2RDOOR)) { - if (door._oVar4 == DoorOpen) + if (door._oVar4 == DOOR_OPEN) return _("Open Grate Door"); - if (door._oVar4 == DoorClosed) + if (door._oVar4 == DOOR_CLOSED) return _("Closed Grate Door"); - if (door._oVar4 == DoorBlocked) + if (door._oVar4 == DOOR_BLOCKED) return _("Blocked Grate Door"); return _("Grate Door"); } @@ -2240,7 +2235,7 @@ enum class TrackerTargetCategory : uint8_t { TrackerTargetCategory SelectedTrackerTargetCategory = TrackerTargetCategory::Items; TrackerTargetCategory AutoWalkTrackerTargetCategory = TrackerTargetCategory::Items; ///< Category of the active auto-walk target. -int AutoWalkTrackerTargetId = -1; ///< ID of the target being auto-walked to, or -1 if inactive. +int AutoWalkTrackerTargetId = -1; ///< ID of the target being auto-walked to, or -1 if inactive. Point NextPositionForWalkDirection(Point position, int8_t walkDir) { @@ -3353,8 +3348,9 @@ std::optional FindFirstClosedDoorOnWalkPath(Point startPosition, for (int i = 0; i < steps; ++i) { const Point next = NextPositionForWalkDirection(position, path[i]); Object *object = FindObjectAtPosition(next); - if (object != nullptr && object->isDoor() && object->_oVar4 == DoorClosed) { - return DoorBlockInfo { .beforeDoor = position, .doorPosition = next }; + // Only closed doors block the path; blocked doors (DOOR_BLOCKED) are physically passable. + if (object != nullptr && object->isDoor() && object->_oVar4 == DOOR_CLOSED) { + return DoorBlockInfo { .beforeDoor = position, .doorPosition = object->position }; } position = next; } @@ -3385,12 +3381,13 @@ struct TrackerPathBlockInfo { } Object *object = FindObjectAtPosition(next); - if (considerDoors && object != nullptr && object->isDoor() && object->_oVar4 == DoorClosed) { + // Only closed doors block the path; blocked doors (DOOR_BLOCKED) are physically passable. + if (considerDoors && object != nullptr && object->isDoor() && object->_oVar4 == DOOR_CLOSED) { return TrackerPathBlockInfo { .type = TrackerPathBlockType::Door, .stepIndex = i, .beforeBlock = position, - .blockPosition = next, + .blockPosition = object->position, }; } if (considerBreakables && object != nullptr && object->_oSolidFlag && object->IsBreakable()) { @@ -4246,42 +4243,27 @@ void AutoWalkToTrackerTargetKeyPressed() break; } case TrackerTargetCategory::Chests: - targetId = ResolveObjectTrackerTarget(lockedTargetId, playerPosition, - IsTrackedChestObject, FindNearestUnopenedChestObjectId, - [](int id) -> StringOrView { return Objects[id].name(); }, - N_("No chests found."), targetName); + targetId = ResolveObjectTrackerTarget(lockedTargetId, playerPosition, IsTrackedChestObject, FindNearestUnopenedChestObjectId, [](int id) -> StringOrView { return Objects[id].name(); }, N_("No chests found."), targetName); if (!targetId) return; break; case TrackerTargetCategory::Doors: - targetId = ResolveObjectTrackerTarget(lockedTargetId, playerPosition, - IsTrackedDoorObject, FindNearestDoorObjectId, - [](int id) -> StringOrView { return DoorLabelForSpeech(Objects[id]); }, - N_("No doors found."), targetName); + targetId = ResolveObjectTrackerTarget(lockedTargetId, playerPosition, IsTrackedDoorObject, FindNearestDoorObjectId, [](int id) -> StringOrView { return DoorLabelForSpeech(Objects[id]); }, N_("No doors found."), targetName); if (!targetId) return; break; case TrackerTargetCategory::Shrines: - targetId = ResolveObjectTrackerTarget(lockedTargetId, playerPosition, - IsShrineLikeObject, FindNearestShrineObjectId, - [](int id) -> StringOrView { return Objects[id].name(); }, - N_("No shrines found."), targetName); + targetId = ResolveObjectTrackerTarget(lockedTargetId, playerPosition, IsShrineLikeObject, FindNearestShrineObjectId, [](int id) -> StringOrView { return Objects[id].name(); }, N_("No shrines found."), targetName); if (!targetId) return; break; case TrackerTargetCategory::Objects: - targetId = ResolveObjectTrackerTarget(lockedTargetId, playerPosition, - IsTrackedMiscInteractableObject, FindNearestMiscInteractableObjectId, - [](int id) -> StringOrView { return Objects[id].name(); }, - N_("No objects found."), targetName); + targetId = ResolveObjectTrackerTarget(lockedTargetId, playerPosition, IsTrackedMiscInteractableObject, FindNearestMiscInteractableObjectId, [](int id) -> StringOrView { return Objects[id].name(); }, N_("No objects found."), targetName); if (!targetId) return; break; case TrackerTargetCategory::Breakables: - targetId = ResolveObjectTrackerTarget(lockedTargetId, playerPosition, - IsTrackedBreakableObject, FindNearestBreakableObjectId, - [](int id) -> StringOrView { return Objects[id].name(); }, - N_("No breakables found."), targetName); + targetId = ResolveObjectTrackerTarget(lockedTargetId, playerPosition, IsTrackedBreakableObject, FindNearestBreakableObjectId, [](int id) -> StringOrView { return Objects[id].name(); }, N_("No breakables found."), targetName); if (!targetId) return; break; @@ -4483,8 +4465,7 @@ std::optional> FindKeyboardWalkPathForSpeechBfs(const Player const int8_t yDir = delta.deltaY > 0 ? WALK_SW : (delta.deltaY < 0 ? WALK_NE : WALK_NONE); if (allowDiagonalSteps && delta.deltaX != 0 && delta.deltaY != 0) { - const int8_t diagDir = - delta.deltaX > 0 ? (delta.deltaY > 0 ? WALK_S : WALK_E) : (delta.deltaY > 0 ? WALK_W : WALK_N); + const int8_t diagDir = delta.deltaX > 0 ? (delta.deltaY > 0 ? WALK_S : WALK_E) : (delta.deltaY > 0 ? WALK_W : WALK_N); addUniqueDir(diagDir); } @@ -4667,8 +4648,7 @@ std::optional> FindKeyboardWalkPathToClosestReachableForSpee const int8_t yDir = delta.deltaY > 0 ? WALK_SW : (delta.deltaY < 0 ? WALK_NE : WALK_NONE); if (allowDiagonalSteps && delta.deltaX != 0 && delta.deltaY != 0) { - const int8_t diagDir = - delta.deltaX > 0 ? (delta.deltaY > 0 ? WALK_S : WALK_E) : (delta.deltaY > 0 ? WALK_W : WALK_N); + const int8_t diagDir = delta.deltaX > 0 ? (delta.deltaY > 0 ? WALK_S : WALK_E) : (delta.deltaY > 0 ? WALK_W : WALK_N); addUniqueDir(diagDir); } @@ -5303,7 +5283,7 @@ void SpeakNearestExitKeyPressed() } const int triggerIndex = FindLockedTownDungeonTriggerIndex(dungeonCandidates) - .value_or(FindDefaultTownDungeonTriggerIndex(dungeonCandidates).value_or(dungeonCandidates.front())); + .value_or(FindDefaultTownDungeonTriggerIndex(dungeonCandidates).value_or(dungeonCandidates.front())); LockedTownDungeonTriggerIndex = triggerIndex; const TriggerStruct &trigger = trigs[triggerIndex]; diff --git a/Source/objects.cpp b/Source/objects.cpp index 908d0783f..9f0706bee 100644 --- a/Source/objects.cpp +++ b/Source/objects.cpp @@ -3,6 +3,7 @@ * * Implementation of object functionality, interaction, spawning, loading, etc. */ +#include #include #include #include @@ -100,14 +101,6 @@ enum shrine_type : uint8_t { NumberOfShrineTypes }; -enum { - // clang-format off - DOOR_CLOSED = 0, - DOOR_OPEN = 1, - DOOR_BLOCKED = 2, - // clang-format on -}; - int trapid; int trapdir; OptionalOwnedClxSpriteList pObjCels[40]; @@ -1182,12 +1175,18 @@ void AddDoor(Object &door) case OBJ_L5LDOOR: door._oVar1 = dPiece[door.position.x][door.position.y] + 1; door._oVar2 = dPiece[door.position.x][door.position.y - 1] + 1; + // Register the archway tile so FindObjectAtPosition resolves it to this door, + // enabling auto-walk door detection and IsTileWalkable with ignoreDoors. + assert(door.position.y > 0); dObject[door.position.x][door.position.y - 1] = -(static_cast(door.GetId()) + 1); break; case OBJ_L1RDOOR: case OBJ_L5RDOOR: door._oVar1 = dPiece[door.position.x][door.position.y] + 1; door._oVar2 = dPiece[door.position.x - 1][door.position.y] + 1; + // Register the archway tile so FindObjectAtPosition resolves it to this door, + // enabling auto-walk door detection and IsTileWalkable with ignoreDoors. + assert(door.position.x > 0); dObject[door.position.x - 1][door.position.y] = -(static_cast(door.GetId()) + 1); break; default: @@ -4305,7 +4304,7 @@ void MonstCheckDoors(const Monster &monster) continue; Object &door = *object; - // Doors use _oVar4 to track open/closed state, non-zero values indicate an open door + // Doors use _oVar4 to track state (DOOR_CLOSED, DOOR_OPEN, or DOOR_BLOCKED); skip non-closed doors if (!door.isDoor() || door._oVar4 != DOOR_CLOSED) continue; diff --git a/Source/objects.h b/Source/objects.h index c9032454d..7712fdc67 100644 --- a/Source/objects.h +++ b/Source/objects.h @@ -29,6 +29,17 @@ namespace devilution { #define MAXOBJECTS 127 +static_assert(MAXOBJECTS <= 127, "MAXOBJECTS must fit in int8_t for the dObject encoding scheme"); + +/** Door state values stored in Object::_oVar4 for door-type objects. */ +enum { + // clang-format off + DOOR_CLOSED = 0, + DOOR_OPEN = 1, + DOOR_BLOCKED = 2, + // clang-format on +}; + struct Object { _object_id _otype = OBJ_NULL; bool applyLighting = false; diff --git a/test/tile_properties_test.cpp b/test/tile_properties_test.cpp index 45653be2f..9dbb40dac 100644 --- a/test/tile_properties_test.cpp +++ b/test/tile_properties_test.cpp @@ -67,7 +67,7 @@ TEST(TilePropertiesTest, DoorArchwaySolidTileBecomesWalkableWhenIgnoringDoors) Objects[0]._otype = _object_id::OBJ_L1LDOOR; Objects[0]._oSolidFlag = false; dObject[5][5] = 1; - dObject[5][4] = -(static_cast(0) + 1); // archway tile (large object convention) + dObject[5][4] = -1; // Negative dObject value: extended area of Objects[0] (the door) EXPECT_TRUE(IsTileWalkable({ 5, 4 }, true)) << "Solid archway tile referencing a door becomes walkable when ignoring doors"; EXPECT_FALSE(IsTileWalkable({ 5, 4 }))