fixes errors and warnings

Applies review comments

getWorldspaceTerrain => returns a reference because never null
crashfix in navigator
updateLandPositions fixes naming of it
const ESM4::Land* MWWorld::Store<ESM4::Land>::search(ESM::ExteriorCellLocation cellLocation) const removes useless else
ExteriorCellLocation uses default initializers
get terrain height returns -MAX_FLOAT when there is no esm4 terrain.

applied review comments

use default initlializer when possible
factorise code
uses pattern matching in for loop.

apps/opencs/view/render/terrainstorage.cpp

@@ -82,7 +82,7 @@ namespace CSVRender
         return &mAlteredHeight[inCellY * ESM::Land::LAND_SIZE + inCellX];
     }
 
-    void TerrainStorage::getBounds(float& minX, float& maxX, float& minY, float& maxY)
+    void TerrainStorage::getBounds(float& minX, float& maxX, float& minY, float& maxY, ESM::RefId worldspace)
     {
         // not needed at the moment - this returns the bounds of the whole world, but we only edit individual cells
         throw std::runtime_error("getBounds not implemented");

apps/opencs/view/render/terrainstorage.hpp

@@ -40,7 +40,7 @@ namespace CSVRender
         osg::ref_ptr<const ESMTerrain::LandObject> getLand(ESM::ExteriorCellLocation cellLocation) override;
         const ESM::LandTexture* getLandTexture(int index, short plugin) override;
 
-        void getBounds(float& minX, float& maxX, float& minY, float& maxY) override;
+        void getBounds(float& minX, float& maxX, float& minY, float& maxY, ESM::RefId worldspace) override;
 
         int getThisHeight(int col, int row, const ESM::Land::LandData* heightData) const;
         int getLeftHeight(int col, int row, const ESM::Land::LandData* heightData) const;

apps/openmw/mwrender/renderingmanager.cpp

@@ -458,7 +458,7 @@ namespace MWRender
         mTerrainStorage = std::make_unique<TerrainStorage>(mResourceSystem, normalMapPattern, heightMapPattern,
             useTerrainNormalMaps, specularMapPattern, useTerrainSpecularMaps);
 
-        mTerrain = getWorldspaceTerrain(ESM::Cell::sDefaultWorldspaceId);
+        mTerrain = &getWorldspaceTerrain(ESM::Cell::sDefaultWorldspaceId);
         bool groundcover = Settings::Manager::getBool("enabled", "Groundcover");
 
         if (groundcover)
@@ -758,7 +758,7 @@ namespace MWRender
         if (store->getCell()->isExterior())
         {
             getWorldspaceTerrain(store->getCell()->getWorldSpace())
-                ->unloadCell(store->getCell()->getGridX(), store->getCell()->getGridY());
+                .unloadCell(store->getCell()->getGridX(), store->getCell()->getGridY());
         }
 
         mWater->removeCell(store);
@@ -770,7 +770,7 @@ namespace MWRender
             mWater->setCullCallback(nullptr);
         else
         {
-            Terrain::World* newTerrain = getWorldspaceTerrain(worldspace);
+            Terrain::World* newTerrain = &getWorldspaceTerrain(worldspace);
             if (newTerrain != mTerrain)
             {
                 mTerrain->enable(false);
@@ -1328,11 +1328,11 @@ namespace MWRender
         mStateUpdater->setFogColor(color);
     }
 
-    Terrain::World* RenderingManager::getWorldspaceTerrain(ESM::RefId worldspace)
+    Terrain::World& RenderingManager::getWorldspaceTerrain(ESM::RefId worldspace)
     {
         auto existingTerrain = mWorldspaceTerrains.find(worldspace);
         if (existingTerrain != mWorldspaceTerrains.end())
-            return existingTerrain->second.get();
+            return *existingTerrain->second.get();
         std::unique_ptr<Terrain::World> newTerrain;
 
         const float lodFactor = Settings::Manager::getFloat("lod factor", "Terrain");
@@ -1366,8 +1366,9 @@ namespace MWRender
         float distanceMult = std::cos(osg::DegreesToRadians(std::min(mFieldOfView, 140.f)) / 2.f);
         newTerrain->setViewDistance(mViewDistance * (distanceMult ? 1.f / distanceMult : 1.f));
 
-        mWorldspaceTerrains[worldspace] = std::move(newTerrain);
+        Terrain::World* result = newTerrain.get();
-        return mWorldspaceTerrains[worldspace].get();
+        mWorldspaceTerrains.emplace(worldspace, std::move(newTerrain));
+        return *result;
     }
 
     void RenderingManager::reportStats() const
@@ -1473,7 +1474,7 @@ namespace MWRender
 
     float RenderingManager::getTerrainHeightAt(const osg::Vec3f& pos, ESM::RefId worldspace)
     {
-        return getWorldspaceTerrain(worldspace)->getHeightAt(pos);
+        return getWorldspaceTerrain(worldspace).getHeightAt(pos);
     }
 
     void RenderingManager::overrideFieldOfView(float val)

apps/openmw/mwrender/renderingmanager.hpp

@@ -282,7 +282,7 @@ namespace MWRender
         void updateAmbient();
         void setFogColor(const osg::Vec4f& color);
         void updateThirdPersonViewMode();
-        Terrain::World* getWorldspaceTerrain(ESM::RefId worldspace);
+        Terrain::World& getWorldspaceTerrain(ESM::RefId worldspace);
 
         void reportStats() const;
 

apps/openmw/mwrender/terrainstorage.cpp

@@ -34,6 +34,18 @@ namespace MWRender
         return land != nullptr;
     }
 
+    static void BoundUnion(float& minX, float& maxX, float& minY, float& maxY, float x, float y)
+    {
+        if (x < minX)
+            minX = x;
+        if (x > maxX)
+            maxX = x;
+        if (y < minY)
+            minY = y;
+        if (y > maxY)
+            maxY = y;
+    }
+
     void TerrainStorage::getBounds(float& minX, float& maxX, float& minY, float& maxY, ESM::RefId worldspace)
     {
         minX = 0;
@@ -46,20 +58,11 @@ namespace MWRender
         if (ESM::isEsm4Ext(worldspace))
         {
             const auto& lands = esmStore.get<ESM4::Land>().getLands();
-            for (const auto& it : lands)
+            for (const auto& [landPos, _] : lands)
             {
-                if (it.first.mWorldspace == worldspace)
+                if (landPos.mWorldspace == worldspace)
                 {
-                    int x = it.first.mX;
+                    BoundUnion(minX, maxX, minY, maxY, static_cast<float>(landPos.mX), static_cast<float>(landPos.mY));
-                    int y = it.first.mY;
-                    if (x < minX)
-                        minX = static_cast<float>(x);
-                    if (x > maxX)
-                        maxX = static_cast<float>(x);
-                    if (y < minY)
-                        minY = static_cast<float>(y);
-                    if (y > maxY)
-                        maxY = static_cast<float>(y);
                 }
             }
         }
@@ -68,14 +71,7 @@ namespace MWRender
             MWWorld::Store<ESM::Land>::iterator it = esmStore.get<ESM::Land>().begin();
             for (; it != esmStore.get<ESM::Land>().end(); ++it)
             {
-                if (it->mX < minX)
+                BoundUnion(minX, maxX, minY, maxY, static_cast<float>(it->mX), static_cast<float>(it->mY));
-                    minX = static_cast<float>(it->mX);
-                if (it->mX > maxX)
-                    maxX = static_cast<float>(it->mX);
-                if (it->mY < minY)
-                    minY = static_cast<float>(it->mY);
-                if (it->mY > maxY)
-                    maxY = static_cast<float>(it->mY);
             }
         }
         // since grid coords are at cell origin, we need to add 1 cell

apps/openmw/mwworld/scene.cpp

@@ -433,7 +433,7 @@ namespace MWWorld
                         return heights;
                     }
                 }();
-                mNavigator.addHeightfield(cellPosition, data->getSize(), shape, navigatorUpdateGuard);
+                mNavigator.addHeightfield(cellPosition, worldsize, shape, navigatorUpdateGuard);
             }
         }
 

apps/openmw/mwworld/store.cpp

@@ -1199,15 +1199,15 @@ namespace MWWorld
 
     void Store<ESM4::Land>::updateLandPositions(const Store<ESM4::Cell>& cells)
     {
-        for (auto& it : mStatic)
+        for (const auto& [id, value] : mStatic)
         {
-            const ESM4::Cell* cell = cells.find(it.second.mCell);
+            const ESM4::Cell* cell = cells.find(value.mCell);
-            mLands[cell->getExteriorCellLocation()] = &it.second;
+            mLands[cell->getExteriorCellLocation()] = &value;
         }
-        for (auto& it : mDynamic)
+        for (const auto& [id, value] : mDynamic)
         {
-            const ESM4::Cell* cell = cells.find(it.second.mCell);
+            const ESM4::Cell* cell = cells.find(value.mCell);
-            mLands[cell->getExteriorCellLocation()] = &it.second;
+            mLands[cell->getExteriorCellLocation()] = &value;
         }
     }
 
@@ -1216,8 +1216,7 @@ namespace MWWorld
         auto foundLand = mLands.find(cellLocation);
         if (foundLand == mLands.end())
             return nullptr;
-        else
+        return foundLand->second;
-            return foundLand->second;
     }
 
     // ESM4 Reference

apps/openmw/mwworld/store.hpp

@@ -18,6 +18,7 @@
 #include <components/esm3/loadland.hpp>
 #include <components/esm3/loadpgrd.hpp>
 #include <components/esm4/loadcell.hpp>
+#include <components/esm4/loadland.hpp>
 #include <components/esm4/loadrefr.hpp>
 #include <components/misc/rng.hpp>
 #include <components/misc/strings/algorithm.hpp>

components/esm/util.hpp

@@ -51,14 +51,10 @@ namespace ESM
 
     struct ExteriorCellLocation
     {
-        int mX, mY;
+        int mX = 0;
-        ESM::RefId mWorldspace;
+        int mY = 0;
-        ExteriorCellLocation()
+        ESM::RefId mWorldspace = ESM::Cell::sDefaultWorldspaceId;
-            : mX(0)
+        ExteriorCellLocation() = default;
-            , mY(0)
-            , mWorldspace(ESM::Cell::sDefaultWorldspaceId)
-        {
-        }
 
         ExteriorCellLocation(int x, int y, ESM::RefId worldspace)
             : mX(x)

components/esm3terrain/storage.cpp

@@ -337,7 +337,7 @@ namespace ESMTerrain
 
         if (!validHeightDataExists && ESM::isEsm4Ext(worldspace))
         {
-            for (int iVert = 0; iVert < numVerts * numVerts; iVert++)
+            for (unsigned int iVert = 0; iVert < numVerts * numVerts; iVert++)
             {
                 (*positions)[static_cast<unsigned int>(iVert)] = osg::Vec3f(0.f, 0.f, 0.f);
             }
@@ -485,7 +485,7 @@ namespace ESMTerrain
 
         osg::ref_ptr<const LandObject> land = getLand(ESM::ExteriorCellLocation(cellX, cellY, worldspace));
         if (!land)
-            return defaultHeight;
+            return ESM::isEsm4Ext(worldspace) ? std::numeric_limits<float>::lowest() : defaultHeight;
 
         const ESM::LandData* data = land->getData(ESM::Land::DATA_VHGT);
         if (!data)

components/terrain/quadtreeworld.hpp

@@ -60,12 +60,7 @@ namespace Terrain
         {
         public:
             virtual ~ChunkManager() {}
-            ChunkManager()
+            ChunkManager() = default;
-                : mWorldspace(ESM::RefId())
-                , mViewDistance(0.f)
-                , mMaxLodLevel(~0u)
-            {
-            }
             ChunkManager(ESM::RefId worldspace)
                 : ChunkManager()
             {
@@ -84,11 +79,11 @@ namespace Terrain
             void setMaxLodLevel(unsigned int level) { mMaxLodLevel = level; }
 
         protected:
-            ESM::RefId mWorldspace;
+            ESM::RefId mWorldspace = ESM::RefId();
 
         private:
-            float mViewDistance;
+            float mViewDistance = 0.f;
-            unsigned int mMaxLodLevel;
+            unsigned int mMaxLodLevel = ~0u;
         };
         void addChunkManager(ChunkManager*);