Do not rely on exceptions to handle absent cell

apps/openmw/mwscript/transformationextensions.cpp

@@ -396,33 +396,31 @@ namespace MWScript
                 if (isPlayer)
                     world->getPlayer().setTeleported(true);
 
-                MWWorld::CellStore* store = nullptr;
+                MWWorld::CellStore* store = worldModel->findCell(cellID);
-                try
+
-                {
+                if (store != nullptr && store->isExterior())
-                    store = &worldModel->getCell(cellID);
+                    store = &worldModel->getExterior(
-                    if (store->isExterior())
+                        ESM::positionToExteriorCellLocation(x, y, store->getCell()->getWorldSpace()));
-                    {
+
-                        const ESM::ExteriorCellLocation cellIndex
+                if (store == nullptr)
-                            = ESM::positionToExteriorCellLocation(x, y, store->getCell()->getWorldSpace());
-                        store = &worldModel->getExterior(cellIndex);
-                    }
-                }
-                catch (std::exception&)
                 {
                     // cell not found, move to exterior instead if moving the player (vanilla PositionCell
                     // compatibility)
-                    std::string error = "Warning: PositionCell: unknown interior cell (" + std::string(cellID) + ")";
+                    std::string error = "PositionCell: unknown interior cell (" + std::string(cellID) + ")";
                     if (isPlayer)
                         error += ", moving to exterior instead";
                     runtime.getContext().report(error);
-                    Log(Debug::Warning) << error;
                     if (!isPlayer)
-                        return;
-                    store = &worldModel->getExterior(
-                        ESM::positionToExteriorCellLocation(x, y, ESM::Cell::sDefaultWorldspaceId));
-                }
-                if (store)
                     {
+                        Log(Debug::Error) << error;
+                        return;
+                    }
+                    Log(Debug::Warning) << error;
+                    const ESM::ExteriorCellLocation cellIndex
+                        = ESM::positionToExteriorCellLocation(x, y, ESM::Cell::sDefaultWorldspaceId);
+                    store = &worldModel->getExterior(cellIndex);
+                }
+
                 MWWorld::Ptr base = ptr;
                 ptr = world->moveObject(ptr, store, osg::Vec3f(x, y, z));
                 dynamic_cast<MWScript::InterpreterContext&>(runtime.getContext()).updatePtr(base, ptr);
@@ -439,7 +437,6 @@ namespace MWScript
                 bool cellActive = MWBase::Environment::get().getWorldScene()->isCellActive(*ptr.getCell());
                 ptr.getClass().adjustPosition(ptr, isPlayer || !cellActive);
             }
-            }
         };
 
         template <class R>
@@ -468,7 +465,7 @@ namespace MWScript
                     ptr.getClass().getCreatureStats(ptr).setTeleported(true);
                 if (isPlayer)
                     world->getPlayer().setTeleported(true);
-                const ESM::ExteriorCellLocation cellIndex
+                const ESM::ExteriorCellLocation location
                     = ESM::positionToExteriorCellLocation(x, y, ESM::Cell::sDefaultWorldspaceId);
 
                 // another morrowind oddity: player will be moved to the exterior cell at this location,
@@ -476,7 +473,7 @@ namespace MWScript
                 MWWorld::Ptr base = ptr;
                 if (isPlayer)
                 {
-                    MWWorld::CellStore* cell = &MWBase::Environment::get().getWorldModel()->getExterior(cellIndex);
+                    MWWorld::CellStore* cell = &MWBase::Environment::get().getWorldModel()->getExterior(location);
                     ptr = world->moveObject(ptr, cell, osg::Vec3(x, y, z));
                 }
                 else
@@ -517,18 +514,15 @@ namespace MWScript
                 Interpreter::Type_Float zRotDegrees = runtime[0].mFloat;
                 runtime.pop();
 
-                MWWorld::CellStore* store = nullptr;
+                MWWorld::CellStore* const store = MWBase::Environment::get().getWorldModel()->findCell(cellName);
-                try
+                if (store == nullptr)
                 {
-                    store = &MWBase::Environment::get().getWorldModel()->getCell(cellName);
+                    const std::string message = "unknown cell (" + std::string(cellName) + ")";
+                    runtime.getContext().report(message);
+                    Log(Debug::Error) << message;
+                    return;
                 }
-                catch (std::exception&)
+
-                {
-                    runtime.getContext().report("unknown cell (" + std::string(cellName) + ")");
-                    Log(Debug::Error) << "Error: unknown cell (" << cellName << ")";
-                }
-                if (store)
-                {
                 ESM::Position pos;
                 pos.pos[0] = x;
                 pos.pos[1] = y;
@@ -541,7 +535,6 @@ namespace MWScript
                 MWWorld::Ptr placed = MWBase::Environment::get().getWorld()->placeObject(ref.getPtr(), store, pos);
                 placed.getClass().adjustPosition(placed, true);
             }
-            }
         };
 
         class OpPlaceItem : public Interpreter::Opcode0

apps/openmw/mwstate/statemanagerimp.cpp

@@ -564,7 +564,7 @@ void MWState::StateManager::loadGame(const Character* character, const std::file
         else
         {
             // Cell no longer exists (i.e. changed game files), choose a default cell
-            Log(Debug::Warning) << "Warning: Player character's cell no longer exists, changing to the default cell";
+            Log(Debug::Warning) << "Player character's cell no longer exists, changing to the default cell";
             ESM::ExteriorCellLocation cellIndex(0, 0, ESM::Cell::sDefaultWorldspaceId);
             MWWorld::CellStore& cell = MWBase::Environment::get().getWorldModel()->getExterior(cellIndex);
             osg::Vec2 posFromIndex = ESM::indexToPosition(cellIndex, false);

apps/openmw/mwworld/player.cpp

@@ -360,16 +360,9 @@ namespace MWWorld
 
             MWBase::World& world = *MWBase::Environment::get().getWorld();
 
-            try
+            mCellStore = MWBase::Environment::get().getWorldModel()->findCell(player.mCellId);
-            {
+            if (mCellStore == nullptr)
-                mCellStore = &MWBase::Environment::get().getWorldModel()->getCell(player.mCellId);
+                Log(Debug::Warning) << "Player cell " << player.mCellId << " no longer exists";
-            }
-            catch (...)
-            {
-                Log(Debug::Warning) << "Warning: Player cell '" << player.mCellId << "' no longer exists";
-                // Cell no longer exists. The loader will have to choose a default cell.
-                mCellStore = nullptr;
-            }
 
             if (!player.mBirthsign.empty())
             {

apps/openmw/mwworld/worldimp.cpp

@@ -2744,16 +2744,11 @@ namespace MWWorld
     {
         pos.rot[0] = pos.rot[1] = pos.rot[2] = 0;
 
-        const MWWorld::CellStore* cellStore = nullptr;
+        const MWWorld::CellStore* cellStore = mWorldModel.findCell(nameId);
-        try
+
-        {
+        if (cellStore != nullptr && !cellStore->isExterior())
-            cellStore = &mWorldModel.getCell(nameId);
-            if (!cellStore->isExterior())
             return ESM::RefId();
-        }
+
-        catch (std::exception&)
-        {
-        }
         if (!cellStore)
         {
             size_t comma = nameId.find(',');

apps/openmw/mwworld/worldmodel.cpp

@@ -1,6 +1,8 @@
 #include "worldmodel.hpp"
 
 #include <algorithm>
+#include <cassert>
+#include <optional>
 
 #include <components/debug/debuglog.hpp>
 #include <components/esm/defs.hpp>
@@ -30,6 +32,58 @@ namespace MWWorld
                     std::forward_as_tuple(Cell(cell), store, readers))
                 .first->second;
         }
+
+        const ESM::Cell* createEsmCell(ESM::ExteriorCellLocation location, ESMStore& store)
+        {
+            ESM::Cell record;
+            record.mData.mFlags = ESM::Cell::HasWater;
+            record.mData.mX = location.mX;
+            record.mData.mY = location.mY;
+            record.mWater = 0;
+            record.mMapColor = 0;
+            record.updateId();
+            return store.insert(record);
+        }
+
+        const ESM4::Cell* createEsm4Cell(ESM::ExteriorCellLocation location, ESMStore& store)
+        {
+            ESM4::Cell record;
+            record.mParent = location.mWorldspace;
+            record.mX = location.mX;
+            record.mY = location.mY;
+            record.mCellFlags = 0;
+            return store.insert(record);
+        }
+
+        Cell createExteriorCell(ESM::ExteriorCellLocation location, ESMStore& store)
+        {
+            if (ESM::isEsm4Ext(location.mWorldspace))
+            {
+                if (store.get<ESM4::World>().search(location.mWorldspace) == nullptr)
+                    throw std::runtime_error(
+                        "Exterior ESM4 world is not found: " + location.mWorldspace.toDebugString());
+                const ESM4::Cell* cell = store.get<ESM4::Cell>().searchExterior(location);
+                if (cell == nullptr)
+                    cell = createEsm4Cell(location, store);
+                assert(cell != nullptr);
+                return MWWorld::Cell(*cell);
+            }
+
+            const ESM::Cell* cell = store.get<ESM::Cell>().search(location.mX, location.mY);
+            if (cell == nullptr)
+                cell = createEsmCell(location, store);
+            assert(cell != nullptr);
+            return Cell(*cell);
+        }
+
+        std::optional<Cell> createCell(ESM::RefId id, const ESMStore& store)
+        {
+            if (const ESM4::Cell* cell = store.get<ESM4::Cell>().search(id))
+                return Cell(*cell);
+            if (const ESM::Cell* cell = store.get<ESM::Cell>().search(id))
+                return Cell(*cell);
+            return std::nullopt;
+        }
     }
 }
 
@@ -102,143 +156,88 @@ MWWorld::WorldModel::WorldModel(MWWorld::ESMStore& store, ESM::ReadersCache& rea
 {
 }
 
-MWWorld::CellStore& MWWorld::WorldModel::getExterior(ESM::ExteriorCellLocation cellIndex, bool forceLoad)
+namespace MWWorld
 {
-    std::map<ESM::ExteriorCellLocation, CellStore*>::iterator result;
+    CellStore& WorldModel::getExterior(ESM::ExteriorCellLocation location, bool forceLoad) const
-
-    result = mExteriors.find(cellIndex);
-
-    if (result == mExteriors.end())
     {
-        if (!ESM::isEsm4Ext(cellIndex.mWorldspace))
+        auto it = mExteriors.find(location);
-        {
+        if (it != mExteriors.end())
-            const ESM::Cell* cell = mStore.get<ESM::Cell>().search(cellIndex.mX, cellIndex.mY);
+            return *it->second;
-
+        Cell cell = createExteriorCell(location, mStore);
-            if (cell == nullptr)
+        const ESM::RefId id = cell.getId();
-            {
+        CellStore& cellStore = emplaceCellStore(id, std::move(cell), mStore, mReaders, mCells);
-                // Cell isn't predefined. Make one on the fly.
+        mExteriors.emplace(location, &cellStore);
-                ESM::Cell record;
+        if (forceLoad && cellStore.getState() != CellStore::State_Loaded)
-                record.mData.mFlags = ESM::Cell::HasWater;
+            cellStore.load();
-                record.mData.mX = cellIndex.mX;
+        return cellStore;
-                record.mData.mY = cellIndex.mY;
-                record.mWater = 0;
-                record.mMapColor = 0;
-                record.updateId();
-
-                cell = mStore.insert(record);
     }
 
-            CellStore* cellStore = &emplaceCellStore(cell->mId, *cell, mStore, mReaders, mCells);
+    CellStore* WorldModel::findInterior(std::string_view name, bool forceLoad) const
-            result = mExteriors.emplace(cellIndex, cellStore).first;
-        }
-        else
     {
-            const Store<ESM4::Cell>& cell4Store = mStore.get<ESM4::Cell>();
+        const auto it = mInteriors.find(name);
-            bool exteriorExists = mStore.get<ESM4::World>().search(cellIndex.mWorldspace) != nullptr;
+        if (it == mInteriors.end())
-            const ESM4::Cell* cell = cell4Store.searchExterior(cellIndex);
+            return nullptr;
-            if (!exteriorExists)
+        assert(it->second != nullptr);
-                throw std::runtime_error("Exterior ESM4 world is not found: " + cellIndex.mWorldspace.toDebugString());
+        if (forceLoad && it->second->getState() != CellStore::State_Loaded)
-            if (cell == nullptr)
+            it->second->load();
-            {
+        return it->second;
-                ESM4::Cell record;
-                record.mParent = cellIndex.mWorldspace;
-                record.mX = cellIndex.mX;
-                record.mY = cellIndex.mY;
-                // Other ESM4::Cell members use default values from class definition.
-                cell = mStore.insert(record);
-            }
-            CellStore* cellStore = &emplaceCellStore(cell->mId, *cell, mStore, mReaders, mCells);
-            result = mExteriors.emplace(cellIndex, cellStore).first;
-        }
-    }
-    if (forceLoad && result->second->getState() != CellStore::State_Loaded)
-    {
-        result->second->load();
     }
 
-    return *result->second;
+    CellStore& WorldModel::getInterior(std::string_view name, bool forceLoad) const
+    {
+        CellStore* const cellStore = findInterior(name, forceLoad);
+        if (cellStore == nullptr)
+            throw std::runtime_error("Interior cell is not found: '" + std::string(name) + "'");
+        return *cellStore;
     }
 
-MWWorld::CellStore* MWWorld::WorldModel::getInteriorOrNull(std::string_view name)
+    CellStore* WorldModel::findCell(ESM::RefId id, bool forceLoad) const
     {
-    auto result = mInteriors.find(name);
+        auto it = mCells.find(id);
-    if (result == mInteriors.end())
+        if (it != mCells.end())
-    {
+            return &it->second;
-        CellStore* newCellStore = nullptr;
-        if (const ESM::Cell* cell = mStore.get<ESM::Cell>().search(name))
-            newCellStore = &emplaceCellStore(cell->mId, *cell, mStore, mReaders, mCells);
-        else if (const ESM4::Cell* cell4 = mStore.get<ESM4::Cell>().searchCellName(name))
-            newCellStore = &emplaceCellStore(cell4->mId, *cell4, mStore, mReaders, mCells);
-        if (!newCellStore)
-            return nullptr; // Cell not found
-        result = mInteriors.emplace(name, newCellStore).first;
-    }
-    return result->second;
-}
-
-MWWorld::CellStore& MWWorld::WorldModel::getInterior(std::string_view name, bool forceLoad)
-{
-    CellStore* res = getInteriorOrNull(name);
-    if (res == nullptr)
-        throw std::runtime_error("Interior not found: '" + std::string(name) + "'");
-    if (forceLoad && res->getState() != CellStore::State_Loaded)
-        res->load();
-    return *res;
-}
-
-MWWorld::CellStore& MWWorld::WorldModel::getCell(const ESM::RefId& id, bool forceLoad)
-{
-    auto result = mCells.find(id);
-    if (result != mCells.end())
-        return result->second;
 
         if (const auto* exteriorId = id.getIf<ESM::ESM3ExteriorCellRefId>())
-        return getExterior(
+            return &getExterior(
                 ESM::ExteriorCellLocation(exteriorId->getX(), exteriorId->getY(), ESM::Cell::sDefaultWorldspaceId),
                 forceLoad);
 
-    const ESM4::Cell* cell4 = mStore.get<ESM4::Cell>().search(id);
+        std::optional<Cell> cell = createCell(id, mStore);
-    CellStore* newCellStore = nullptr;
+        if (!cell.has_value())
-    if (!cell4)
+            return nullptr;
-    {
+
-        const ESM::Cell* cell = mStore.get<ESM::Cell>().find(id);
+        CellStore& cellStore = emplaceCellStore(id, std::move(*cell), mStore, mReaders, mCells);
-        newCellStore = &emplaceCellStore(cell->mId, *cell, mStore, mReaders, mCells);
+
-    }
+        if (cellStore.isExterior())
+            mExteriors.emplace(ESM::ExteriorCellLocation(cellStore.getCell()->getGridX(),
+                                   cellStore.getCell()->getGridY(), cellStore.getCell()->getWorldSpace()),
+                &cellStore);
         else
-    {
+            mInteriors.emplace(cellStore.getCell()->getNameId(), &cellStore);
-        newCellStore = &emplaceCellStore(cell4->mId, *cell4, mStore, mReaders, mCells);
+
-    }
+        if (forceLoad && cellStore.getState() != CellStore::State_Loaded)
-    if (newCellStore->getCell()->isExterior())
+            cellStore.load();
-    {
+
-        std::pair<int, int> coord
+        return &cellStore;
-            = std::make_pair(newCellStore->getCell()->getGridX(), newCellStore->getCell()->getGridY());
-        ESM::ExteriorCellLocation extIndex = { coord.first, coord.second, newCellStore->getCell()->getWorldSpace() };
-        mExteriors.emplace(extIndex, newCellStore);
-    }
-    else
-    {
-        mInteriors.emplace(newCellStore->getCell()->getNameId(), newCellStore);
-    }
-    if (forceLoad && newCellStore->getState() != CellStore::State_Loaded)
-    {
-        newCellStore->load();
-    }
-    return *newCellStore;
     }
 
-MWWorld::CellStore& MWWorld::WorldModel::getCell(std::string_view name, bool forceLoad)
+    CellStore& WorldModel::getCell(ESM::RefId id, bool forceLoad) const
     {
-    if (CellStore* res = getInteriorOrNull(name)) // first try interiors
+        CellStore* const result = findCell(id, forceLoad);
-    {
+        if (result == nullptr)
-        if (forceLoad && res->getState() != CellStore::State_Loaded)
+            throw std::runtime_error("Cell does not exist: " + id.toDebugString());
-            res->load();
+        return *result;
-        return *res;
     }
 
+    CellStore* WorldModel::findCell(std::string_view name, bool forceLoad) const
+    {
+        if (CellStore* const cellStore = findInterior(name, forceLoad))
+            return cellStore;
+
         // try named exteriors
         const ESM::Cell* cell = mStore.get<ESM::Cell>().searchExtByName(name);
 
-    if (!cell)
+        if (cell == nullptr)
         {
             // treat "Wilderness" like an empty string
             static const std::string& defaultName
@@ -246,25 +245,33 @@ MWWorld::CellStore& MWWorld::WorldModel::getCell(std::string_view name, bool for
             if (Misc::StringUtils::ciEqual(name, defaultName))
                 cell = mStore.get<ESM::Cell>().searchExtByName({});
         }
-    if (!cell)
+
+        if (cell == nullptr)
         {
             // now check for regions
-        for (const ESM::Region& region : mStore.get<ESM::Region>())
+            const Store<ESM::Region>& regions = mStore.get<ESM::Region>();
-        {
+            const auto region = std::find_if(regions.begin(), regions.end(),
-            if (Misc::StringUtils::ciEqual(name, region.mName))
+                [&](const ESM::Region& v) { return Misc::StringUtils::ciEqual(name, v.mName); });
-            {
+            if (region != regions.end())
-                cell = mStore.get<ESM::Cell>().searchExtByRegion(region.mId);
+                cell = mStore.get<ESM::Cell>().searchExtByRegion(region->mId);
-                break;
         }
-        }
-    }
-    if (!cell)
-        throw std::runtime_error(std::string("Can't find cell with name ") + std::string(name));
 
-    return getExterior(
+        if (cell == nullptr)
+            return nullptr;
+
+        return &getExterior(
             ESM::ExteriorCellLocation(cell->getGridX(), cell->getGridY(), ESM::Cell::sDefaultWorldspaceId), forceLoad);
     }
 
+    CellStore& WorldModel::getCell(std::string_view name, bool forceLoad) const
+    {
+        CellStore* const result = findCell(name, forceLoad);
+        if (result == nullptr)
+            throw std::runtime_error(std::string("Can't find cell with name ") + std::string(name));
+        return *result;
+    }
+}
+
 MWWorld::Ptr MWWorld::WorldModel::getPtr(const ESM::RefId& name)
 {
     for (const auto& [cachedId, cellStore] : mIdCache)
@@ -383,17 +390,7 @@ public:
 
     MWWorld::WorldModel& mWorldModel;
 
-    MWWorld::CellStore* getCellStore(const ESM::RefId& cellId) override
+    MWWorld::CellStore* getCellStore(const ESM::RefId& cellId) override { return mWorldModel.findCell(cellId); }
-    {
-        try
-        {
-            return &mWorldModel.getCell(cellId);
-        }
-        catch (...)
-        {
-            return nullptr;
-        }
-    }
 };
 
 bool MWWorld::WorldModel::readRecord(ESM::ESMReader& reader, uint32_t type, const std::map<int, int>& contentFileMap)
@@ -403,16 +400,10 @@ bool MWWorld::WorldModel::readRecord(ESM::ESMReader& reader, uint32_t type, cons
         ESM::CellState state;
         state.mId = reader.getCellId();
 
-        CellStore* cellStore = nullptr;
+        CellStore* const cellStore = findCell(state.mId);
-
+        if (cellStore == nullptr)
-        try
         {
-            cellStore = &getCell(state.mId);
+            Log(Debug::Warning) << "Dropping state for cell " << state.mId << " (cell no longer exists)";
-        }
-        catch (...)
-        {
-            // silently drop cells that don't exist anymore
-            Log(Debug::Warning) << "Warning: Dropping state for cell " << state.mId << " (cell no longer exists)";
             reader.skipRecord();
             return true;
         }

apps/openmw/mwworld/worldmodel.hpp

@@ -4,8 +4,8 @@
 #include <list>
 #include <map>
 #include <string>
+#include <string_view>
 #include <unordered_map>
-#include <variant>
 
 #include <components/esm/util.hpp>
 #include <components/misc/algorithm.hpp>
@@ -47,10 +47,19 @@ namespace MWWorld
 
         void clear();
 
-        CellStore& getExterior(ESM::ExteriorCellLocation cellIndex, bool forceLoad = true);
+        CellStore& getExterior(ESM::ExteriorCellLocation location, bool forceLoad = true) const;
-        CellStore& getInterior(std::string_view name, bool forceLoad = true);
+
-        CellStore& getCell(std::string_view name, bool forceLoad = true); // interior or named exterior
+        CellStore* findCell(ESM::RefId Id, bool forceLoad = true) const;
-        CellStore& getCell(const ESM::RefId& Id, bool forceLoad = true);
+
+        CellStore& getCell(ESM::RefId Id, bool forceLoad = true) const;
+
+        CellStore* findInterior(std::string_view name, bool forceLoad = true) const;
+
+        CellStore& getInterior(std::string_view name, bool forceLoad = true) const;
+
+        CellStore* findCell(std::string_view name, bool forceLoad = true) const;
+
+        CellStore& getCell(std::string_view name, bool forceLoad = true) const;
 
         Ptr getPtr(const ESM::RefId& name);
 
@@ -102,8 +111,6 @@ namespace MWWorld
 
         CellStore& insertCellStore(const ESM::Cell& cell);
 
-        CellStore* getInteriorOrNull(std::string_view name);
-
         Ptr getPtrAndCache(const ESM::RefId& name, CellStore& cellStore);
 
         void writeCell(ESM::ESMWriter& writer, CellStore& cell) const;