Merge branch 'fragile' into 'master'

More closely replicate Morrowind.exe's locks

Closes #7415

See merge request OpenMW/openmw!3116

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

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)
+        if (lockLevel)
-            text += "\n#{sLockLevel}: " + MWGui::ToolTips::toString(lockLevel);
+        {
-        else if (lockLevel < 0)
+            if (ptr.getCellRef().isLocked())
-            text += "\n#{sUnlocked}";
+                text += "\n#{sLockLevel}: " + MWGui::ToolTips::toString(lockLevel);
+            else
+                text += "\n#{sUnlocked}";
+        }
         if (ptr.getCellRef().getTrap() != ESM::RefId())
             text += "\n#{sTrapped}";
 

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
+        if (ptr.getCellRef().getTeleport() && !ptr.getCellRef().isLocked() && ptr.getCellRef().getTrap().empty())
-            && 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)
+        if (lockLevel)
-            text += "\n#{sLockLevel}: " + MWGui::ToolTips::toString(ptr.getCellRef().getLockLevel());
+        {
-        else if (ptr.getCellRef().getLockLevel() < 0)
+            if (ptr.getCellRef().isLocked())
-            text += "\n#{sUnlocked}";
+                text += "\n#{sLockLevel}: " + MWGui::ToolTips::toString(lockLevel);
+            else
+                text += "\n#{sUnlocked}";
+        }
         if (!ptr.getCellRef().getTrap().empty())
             text += "\n#{sTrapped}";
 

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;

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() && !cellref.isLocked() && cellref.getTrap().empty())
-        if (target.getClass().isDoor() && (lockLevel <= 0 || lockLevel == ESM::UnbreakableLock)
-            && 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
+            // Note that attempting to unlock something that has ever been locked is a crime even if it's already
-            // even if it's already unlocked. Likewise, it's illegal to unlock something that has a trap but isn't
+            // unlocked. Likewise, it's illegal to unlock something that has a trap but isn't otherwise locked.
-            // 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());
         }
     }

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.getCellRef().getLockLevel() == ESM::UnbreakableLock
+        // If it's unlocked or can not be unlocked back out immediately. Note that we're not strictly speaking checking
-            || !lock.getClass().hasToolTip(lock)) // If it's unlocked or can not be unlocked back out immediately
+        // 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);

apps/openmw/mwmechanics/spelleffects.cpp

@@ -947,7 +947,7 @@ namespace MWMechanics
                 int magnitude = static_cast<int>(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);

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());
             }
         };
 

apps/openmw/mwworld/cellref.cpp

@@ -283,15 +283,24 @@ namespace MWWorld
 
     void CellRef::lock(int lockLevel)
     {
-        if (lockLevel != 0)
+        setLockLevel(lockLevel);
-            setLockLevel(abs(lockLevel)); // Changes lock to locklevel, if positive
+        setLocked(true);
-        else
-            setLockLevel(ESM::UnbreakableLock); // If zero, set to max lock level
     }
 
     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)

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
         {

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<int>::max();
+            record.mLockLevel = 0;
+            record.mIsLocked = true;
             record.mKey = generateRandomRefId();
             record.mTrap = generateRandomRefId();
             record.mReferenceBlocked = std::numeric_limits<signed char>::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);

components/esm3/cellref.cpp

@@ -1,12 +1,18 @@
 #include "cellref.hpp"
 
 #include <algorithm>
+#include <limits>
 
 #include <components/debug/debuglog.hpp>
 
 #include "esmreader.hpp"
 #include "esmwriter.hpp"
 
+namespace
+{
+    constexpr int ZeroLock = std::numeric_limits<int>::max();
+}
+
 namespace ESM
 {
     namespace
@@ -145,11 +151,12 @@ namespace ESM
 
             if constexpr (load)
             {
-                if (cellRef.mLockLevel == 0 && !cellRef.mKey.empty())
+                if (esm.getFormatVersion() == DefaultFormatVersion) // loading a content file
-                {
+                    cellRef.mIsLocked = !cellRef.mKey.empty() || cellRef.mLockLevel > 0;
-                    cellRef.mLockLevel = UnbreakableLock;
+                else
-                    cellRef.mTrap = ESM::RefId();
+                    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;

components/esm3/cellref.hpp

@@ -1,7 +1,6 @@
 #ifndef OPENMW_ESM_CELLREF_H
 #define OPENMW_ESM_CELLREF_H
 
-#include <limits>
 #include <string>
 
 #include "components/esm/common.hpp"
@@ -14,8 +13,6 @@ namespace ESM
     class ESMWriter;
     class ESMReader;
 
-    const int UnbreakableLock = std::numeric_limits<int>::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,