From da4e04522b5debe7d0b86fb5e5d23e3bab716e87 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Thu, 8 Jun 2023 20:10:32 +0200 Subject: [PATCH 1/2] More closely replicate Morrowind.exe's locks --- CHANGELOG.md | 1 + apps/openmw/mwclass/container.cpp | 13 +++++---- apps/openmw/mwclass/door.cpp | 16 ++++++----- apps/openmw/mwmechanics/aipackage.cpp | 2 +- .../mwmechanics/mechanicsmanagerimp.cpp | 11 +++----- apps/openmw/mwmechanics/security.cpp | 2 +- apps/openmw/mwmechanics/spelleffects.cpp | 2 +- apps/openmw/mwscript/miscextensions.cpp | 2 +- apps/openmw/mwworld/cellref.cpp | 19 +++++++++---- apps/openmw/mwworld/cellref.hpp | 2 ++ apps/openmw_test_suite/esm3/testsaveload.cpp | 4 ++- components/esm3/cellref.cpp | 28 ++++++++++++------- components/esm3/cellref.hpp | 4 +-- 13 files changed, 64 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df087c3979..77315c7ba4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ Bug #7243: Get Skyrim.esm loading Bug #7298: Water ripples from projectiles sometimes are not spawned Bug #7307: Alchemy "Magic Effect" search string does not match on tool tip for effects related to attributes + Bug #7415: Unbreakable lock discrepancies Feature #3537: Shader-based water ripples Feature #5492: Let rain and snow collide with statics Feature #6447: Add LOD support to Object Paging diff --git a/apps/openmw/mwclass/container.cpp b/apps/openmw/mwclass/container.cpp index bc55f091d4..e949e87803 100644 --- a/apps/openmw/mwclass/container.cpp +++ b/apps/openmw/mwclass/container.cpp @@ -155,7 +155,7 @@ namespace MWClass MWWorld::Ptr player = MWBase::Environment::get().getWorld()->getPlayerPtr(); MWWorld::InventoryStore& invStore = player.getClass().getInventoryStore(player); - bool isLocked = ptr.getCellRef().getLockLevel() > 0; + bool isLocked = ptr.getCellRef().isLocked(); bool isTrapped = !ptr.getCellRef().getTrap().empty(); bool hasKey = false; std::string_view keyName; @@ -253,10 +253,13 @@ namespace MWClass std::string text; int lockLevel = ptr.getCellRef().getLockLevel(); - if (lockLevel > 0 && lockLevel != ESM::UnbreakableLock) - text += "\n#{sLockLevel}: " + MWGui::ToolTips::toString(lockLevel); - else if (lockLevel < 0) - text += "\n#{sUnlocked}"; + if (lockLevel) + { + if (ptr.getCellRef().isLocked()) + text += "\n#{sLockLevel}: " + MWGui::ToolTips::toString(lockLevel); + else + text += "\n#{sUnlocked}"; + } if (ptr.getCellRef().getTrap() != ESM::RefId()) text += "\n#{sTrapped}"; diff --git a/apps/openmw/mwclass/door.cpp b/apps/openmw/mwclass/door.cpp index 24866fe365..ecd6cb59aa 100644 --- a/apps/openmw/mwclass/door.cpp +++ b/apps/openmw/mwclass/door.cpp @@ -142,7 +142,7 @@ namespace MWClass MWWorld::ContainerStore& invStore = actor.getClass().getContainerStore(actor); - bool isLocked = ptr.getCellRef().getLockLevel() > 0; + bool isLocked = ptr.getCellRef().isLocked(); bool isTrapped = !ptr.getCellRef().getTrap().empty(); bool hasKey = false; std::string_view keyName; @@ -248,8 +248,7 @@ namespace MWClass bool Door::allowTelekinesis(const MWWorld::ConstPtr& ptr) const { - if (ptr.getCellRef().getTeleport() && ptr.getCellRef().getLockLevel() <= 0 - && ptr.getCellRef().getTrap().empty()) + if (ptr.getCellRef().getTeleport() && !ptr.getCellRef().isLocked() && ptr.getCellRef().getTrap().empty()) return false; else return true; @@ -279,10 +278,13 @@ namespace MWClass } int lockLevel = ptr.getCellRef().getLockLevel(); - if (lockLevel > 0 && lockLevel != ESM::UnbreakableLock) - text += "\n#{sLockLevel}: " + MWGui::ToolTips::toString(ptr.getCellRef().getLockLevel()); - else if (ptr.getCellRef().getLockLevel() < 0) - text += "\n#{sUnlocked}"; + if (lockLevel) + { + if (ptr.getCellRef().isLocked()) + text += "\n#{sLockLevel}: " + MWGui::ToolTips::toString(lockLevel); + else + text += "\n#{sUnlocked}"; + } if (!ptr.getCellRef().getTrap().empty()) text += "\n#{sTrapped}"; diff --git a/apps/openmw/mwmechanics/aipackage.cpp b/apps/openmw/mwmechanics/aipackage.cpp index 288ba195f2..c3ed1fe253 100644 --- a/apps/openmw/mwmechanics/aipackage.cpp +++ b/apps/openmw/mwmechanics/aipackage.cpp @@ -313,7 +313,7 @@ void MWMechanics::AiPackage::openDoors(const MWWorld::Ptr& actor) if (!isDoorOnTheWay(actor, door, mPathFinder.getPath().front())) return; - if ((door.getCellRef().getTrap().empty() && door.getCellRef().getLockLevel() <= 0)) + if (door.getCellRef().getTrap().empty() && !door.getCellRef().isLocked()) { world->activate(door, actor); return; diff --git a/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp b/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp index 62d7f47873..a2c2e85f3b 100644 --- a/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp +++ b/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp @@ -883,9 +883,7 @@ namespace MWMechanics const MWWorld::CellRef& cellref = target.getCellRef(); // there is no harm to use unlocked doors - int lockLevel = cellref.getLockLevel(); - if (target.getClass().isDoor() && (lockLevel <= 0 || lockLevel == ESM::UnbreakableLock) - && cellref.getTrap().empty()) + if (target.getClass().isDoor() && !cellref.isLocked() && cellref.getTrap().empty()) { return true; } @@ -951,11 +949,10 @@ namespace MWMechanics MWWorld::Ptr victim; if (isOwned(ptr, item, victim)) { - // Note that attempting to unlock something that has ever been locked (even ESM::UnbreakableLock) is a crime - // even if it's already unlocked. Likewise, it's illegal to unlock something that has a trap but isn't - // otherwise locked. + // Note that attempting to unlock something that has ever been locked is a crime even if it's already + // unlocked. Likewise, it's illegal to unlock something that has a trap but isn't otherwise locked. const auto& cellref = item.getCellRef(); - if (cellref.getLockLevel() || !cellref.getTrap().empty()) + if (cellref.getLockLevel() || cellref.isLocked() || !cellref.getTrap().empty()) commitCrime(ptr, victim, OT_Trespassing, item.getCellRef().getFaction()); } } diff --git a/apps/openmw/mwmechanics/security.cpp b/apps/openmw/mwmechanics/security.cpp index 8c72fbd38f..41e999d4bc 100644 --- a/apps/openmw/mwmechanics/security.cpp +++ b/apps/openmw/mwmechanics/security.cpp @@ -28,7 +28,7 @@ namespace MWMechanics void Security::pickLock(const MWWorld::Ptr& lock, const MWWorld::Ptr& lockpick, std::string_view& resultMessage, std::string_view& resultSound) { - if (lock.getCellRef().getLockLevel() <= 0 || lock.getCellRef().getLockLevel() == ESM::UnbreakableLock + if (lock.getCellRef().getLockLevel() <= 0 || !lock.getClass().hasToolTip(lock)) // If it's unlocked or can not be unlocked back out immediately return; diff --git a/apps/openmw/mwmechanics/spelleffects.cpp b/apps/openmw/mwmechanics/spelleffects.cpp index 3afe220a24..6293242709 100644 --- a/apps/openmw/mwmechanics/spelleffects.cpp +++ b/apps/openmw/mwmechanics/spelleffects.cpp @@ -947,7 +947,7 @@ namespace MWMechanics int magnitude = static_cast(roll(effect)); if (target.getCellRef().getLockLevel() <= magnitude) { - if (target.getCellRef().getLockLevel() > 0) + if (target.getCellRef().isLocked()) { MWBase::Environment::get().getSoundManager()->playSound3D( target, ESM::RefId::stringRefId("Open Lock"), 1.f, 1.f); diff --git a/apps/openmw/mwscript/miscextensions.cpp b/apps/openmw/mwscript/miscextensions.cpp index 7787c15ad3..cd07a34575 100644 --- a/apps/openmw/mwscript/miscextensions.cpp +++ b/apps/openmw/mwscript/miscextensions.cpp @@ -555,7 +555,7 @@ namespace MWScript { MWWorld::Ptr ptr = R()(runtime); - runtime.push(ptr.getCellRef().getLockLevel() > 0); + runtime.push(ptr.getCellRef().isLocked()); } }; diff --git a/apps/openmw/mwworld/cellref.cpp b/apps/openmw/mwworld/cellref.cpp index 6b9494b24f..76f979b693 100644 --- a/apps/openmw/mwworld/cellref.cpp +++ b/apps/openmw/mwworld/cellref.cpp @@ -283,15 +283,24 @@ namespace MWWorld void CellRef::lock(int lockLevel) { - if (lockLevel != 0) - setLockLevel(abs(lockLevel)); // Changes lock to locklevel, if positive - else - setLockLevel(ESM::UnbreakableLock); // If zero, set to max lock level + setLockLevel(lockLevel); + setLocked(true); } void CellRef::unlock() { - setLockLevel(-abs(getLockLevel())); // Makes lockLevel negative + setLockLevel(-getLockLevel()); + setLocked(false); + } + + bool CellRef::isLocked() const + { + return std::visit([](auto&& ref) { return ref.mIsLocked; }, mCellRef.mVariant); + } + + void CellRef::setLocked(bool locked) + { + std::visit([=](auto&& ref) { ref.mIsLocked = locked; }, mCellRef.mVariant); } void CellRef::setTrap(const ESM::RefId& trap) diff --git a/apps/openmw/mwworld/cellref.hpp b/apps/openmw/mwworld/cellref.hpp index f32312b4c0..24f2bd51f8 100644 --- a/apps/openmw/mwworld/cellref.hpp +++ b/apps/openmw/mwworld/cellref.hpp @@ -177,6 +177,8 @@ namespace MWWorld void setLockLevel(int lockLevel); void lock(int lockLevel); void unlock(); + bool isLocked() const; + void setLocked(bool locked); // Key and trap ID names, if any ESM::RefId getKey() const { diff --git a/apps/openmw_test_suite/esm3/testsaveload.cpp b/apps/openmw_test_suite/esm3/testsaveload.cpp index a67bf305cd..96796defd4 100644 --- a/apps/openmw_test_suite/esm3/testsaveload.cpp +++ b/apps/openmw_test_suite/esm3/testsaveload.cpp @@ -297,7 +297,8 @@ namespace ESM generateArray(record.mDoorDest.pos); generateArray(record.mDoorDest.rot); record.mDestCell = generateRandomString(100); - record.mLockLevel = std::numeric_limits::max(); + record.mLockLevel = 0; + record.mIsLocked = true; record.mKey = generateRandomRefId(); record.mTrap = generateRandomRefId(); record.mReferenceBlocked = std::numeric_limits::max(); @@ -321,6 +322,7 @@ namespace ESM EXPECT_EQ(record.mDoorDest, result.mDoorDest); EXPECT_EQ(record.mDestCell, result.mDestCell); EXPECT_EQ(record.mLockLevel, result.mLockLevel); + EXPECT_EQ(record.mIsLocked, result.mIsLocked); EXPECT_EQ(record.mKey, result.mKey); EXPECT_EQ(record.mTrap, result.mTrap); EXPECT_EQ(record.mReferenceBlocked, result.mReferenceBlocked); diff --git a/components/esm3/cellref.cpp b/components/esm3/cellref.cpp index fafab6bcac..4c19abf527 100644 --- a/components/esm3/cellref.cpp +++ b/components/esm3/cellref.cpp @@ -1,12 +1,18 @@ #include "cellref.hpp" #include +#include #include #include "esmreader.hpp" #include "esmwriter.hpp" +namespace +{ + constexpr int ZeroLock = std::numeric_limits::max(); +} + namespace ESM { namespace @@ -145,11 +151,12 @@ namespace ESM if constexpr (load) { - if (cellRef.mLockLevel == 0 && !cellRef.mKey.empty()) - { - cellRef.mLockLevel = UnbreakableLock; - cellRef.mTrap = ESM::RefId(); - } + if (esm.getFormatVersion() == DefaultFormatVersion) // loading a content file + cellRef.mIsLocked = !cellRef.mKey.empty() || cellRef.mLockLevel > 0; + else + cellRef.mIsLocked = cellRef.mLockLevel > 0; + if (cellRef.mLockLevel == ZeroLock) + cellRef.mLockLevel = 0; } } } @@ -217,13 +224,13 @@ namespace ESM esm.writeHNOCString("DNAM", mDestCell); } - if (!inInventory && mLockLevel != 0) - { - esm.writeHNT("FLTV", mLockLevel); - } - if (!inInventory) { + int lockLevel = mLockLevel; + if (lockLevel == 0 && mIsLocked) + lockLevel = ZeroLock; + if (lockLevel != 0) + esm.writeHNT("FLTV", lockLevel); esm.writeHNOCRefId("KNAM", mKey); esm.writeHNOCRefId("TNAM", mTrap); } @@ -251,6 +258,7 @@ namespace ESM mGoldValue = 1; mDestCell.clear(); mLockLevel = 0; + mIsLocked = false; mKey = ESM::RefId(); mTrap = ESM::RefId(); mReferenceBlocked = -1; diff --git a/components/esm3/cellref.hpp b/components/esm3/cellref.hpp index fe17c66ee1..4ec9a26402 100644 --- a/components/esm3/cellref.hpp +++ b/components/esm3/cellref.hpp @@ -1,7 +1,6 @@ #ifndef OPENMW_ESM_CELLREF_H #define OPENMW_ESM_CELLREF_H -#include #include #include "components/esm/common.hpp" @@ -14,8 +13,6 @@ namespace ESM class ESMWriter; class ESMReader; - const int UnbreakableLock = std::numeric_limits::max(); - using RefNum = ESM::FormId; /* Cell reference. This represents ONE object (of many) inside the @@ -84,6 +81,7 @@ namespace ESM // Lock level for doors and containers int mLockLevel; + bool mIsLocked{}; ESM::RefId mKey, mTrap; // Key and trap ID names, if any // This corresponds to the "Reference Blocked" checkbox in the construction set, From a5b147d44d43f3344030ca843478ea2507150dd0 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Thu, 8 Jun 2023 21:56:54 +0200 Subject: [PATCH 2/2] Add a clarifying comment --- apps/openmw/mwmechanics/security.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/openmw/mwmechanics/security.cpp b/apps/openmw/mwmechanics/security.cpp index 41e999d4bc..a13131cae6 100644 --- a/apps/openmw/mwmechanics/security.cpp +++ b/apps/openmw/mwmechanics/security.cpp @@ -28,8 +28,9 @@ namespace MWMechanics void Security::pickLock(const MWWorld::Ptr& lock, const MWWorld::Ptr& lockpick, std::string_view& resultMessage, std::string_view& resultSound) { - if (lock.getCellRef().getLockLevel() <= 0 - || !lock.getClass().hasToolTip(lock)) // If it's unlocked or can not be unlocked back out immediately + // If it's unlocked or can not be unlocked back out immediately. Note that we're not strictly speaking checking + // if the ref is locked, lock levels <= 0 can exist but they cannot be picked + if (lock.getCellRef().getLockLevel() <= 0 || !lock.getClass().hasToolTip(lock)) return; int uses = lockpick.getClass().getItemHealth(lockpick);