Avoid working with AMBI subrecord in tools unnecessarily (bug #5269)

CHANGELOG.md

@@ -195,6 +195,7 @@
     Bug #5249: Wandering NPCs start walking too soon after they hello
     Bug #5250: Creatures display shield ground mesh instead of shield body part
     Bug #5255: "GetTarget, player" doesn't return 1 during NPC hello
+    Bug #5269: Editor: Cell lighting in resaved cleaned content files is corrupted
     Feature #1774: Handle AvoidNode
     Feature #2229: Improve pathfinding AI
     Feature #3025: Analogue gamepad movement controls

apps/esmtool/record.cpp

@@ -534,10 +534,18 @@ void Record<ESM::Cell>::print()
     if (mData.mData.mFlags & ESM::Cell::Interior &&
         !(mData.mData.mFlags & ESM::Cell::QuasiEx))
     {
-        std::cout << "  Ambient Light Color: " << mData.mAmbi.mAmbient << std::endl;
+        if (mData.hasAmbient())
-        std::cout << "  Sunlight Color: " << mData.mAmbi.mSunlight << std::endl;
+        {
-        std::cout << "  Fog Color: " << mData.mAmbi.mFog << std::endl;
+            // TODO: see if we can change the integer representation to something more sensible
-        std::cout << "  Fog Density: " << mData.mAmbi.mFogDensity << std::endl;
+            std::cout << "  Ambient Light Color: " << mData.mAmbi.mAmbient << std::endl;
+            std::cout << "  Sunlight Color: " << mData.mAmbi.mSunlight << std::endl;
+            std::cout << "  Fog Color: " << mData.mAmbi.mFog << std::endl;
+            std::cout << "  Fog Density: " << mData.mAmbi.mFogDensity << std::endl;
+        }
+        else
+        {
+            std::cout << "  No Ambient Information" << std::endl;
+        }
         std::cout << "  Water Level: " << mData.mWater << std::endl;
     }
     else

apps/opencs/model/world/nestedcoladapterimp.cpp

@@ -867,6 +867,8 @@ namespace CSMWorld
         switch (subColIndex)
         {
             case 0: return isInterior;
+            // While the ambient information is not necessarily valid if the subrecord wasn't loaded,
+            // the user should still be allowed to edit it
             case 1: return (isInterior && !behaveLikeExterior) ?
                     cell.mAmbi.mAmbient : QVariant(QVariant::UserType);
             case 2: return (isInterior && !behaveLikeExterior) ?
@@ -912,7 +914,10 @@ namespace CSMWorld
             case 1:
             {
                 if (isInterior && !behaveLikeExterior)
+                {
                     cell.mAmbi.mAmbient = static_cast<int32_t>(value.toInt());
+                    cell.setHasAmbient(true);
+                }
                 else
                     return; // return without saving
                 break;
@@ -920,7 +925,10 @@ namespace CSMWorld
             case 2:
             {
                 if (isInterior && !behaveLikeExterior)
+                {
                     cell.mAmbi.mSunlight = static_cast<int32_t>(value.toInt());
+                    cell.setHasAmbient(true);
+                }
                 else
                     return; // return without saving
                 break;
@@ -928,7 +936,10 @@ namespace CSMWorld
             case 3:
             {
                 if (isInterior && !behaveLikeExterior)
+                {
                     cell.mAmbi.mFog = static_cast<int32_t>(value.toInt());
+                    cell.setHasAmbient(true);
+                }
                 else
                     return; // return without saving
                 break;
@@ -936,7 +947,10 @@ namespace CSMWorld
             case 4:
             {
                 if (isInterior && !behaveLikeExterior)
+                {
                     cell.mAmbi.mFogDensity = value.toFloat();
+                    cell.setHasAmbient(true);
+                }
                 else
                     return; // return without saving
                 break;

components/esm/loadcell.cpp

@@ -110,6 +110,7 @@ namespace ESM
     void Cell::loadCell(ESMReader &esm, bool saveContext)
     {
         bool isLoaded = false;
+        mHasAmbi = false;
         while (!isLoaded && esm.hasMoreSubs())
         {
             esm.getSubName();
@@ -127,6 +128,7 @@ namespace ESM
                     break;
                 case ESM::FourCC<'A','M','B','I'>::value:
                     esm.getHT(mAmbi);
+                    mHasAmbi = true;
                     break;
                 case ESM::FourCC<'R','G','N','N'>::value:
                     mRegion = esm.getHString();
@@ -182,7 +184,12 @@ namespace ESM
             if (mData.mFlags & QuasiEx)
                 esm.writeHNOCString("RGNN", mRegion);
             else
-                esm.writeHNT("AMBI", mAmbi, 16);
+            {
+                // Try to avoid saving ambient lighting information when it's unnecessary.
+                // This is to fix black lighting in resaved cell records that lack this information.
+                if (mHasAmbi)
+                    esm.writeHNT("AMBI", mAmbi, 16);
+            }
         }
         else
         {
@@ -272,6 +279,7 @@ namespace ESM
         mData.mX = 0;
         mData.mY = 0;
 
+        mHasAmbi = true;
         mAmbi.mAmbient = 0;
         mAmbi.mSunlight = 0;
         mAmbi.mFog = 0;

components/esm/loadcell.hpp

@@ -90,6 +90,7 @@ struct Cell
 
   Cell() : mName(""),
            mRegion(""),
+           mHasAmbi(true),
            mWater(0),
            mWaterInt(false),
            mMapColor(0),
@@ -108,6 +109,7 @@ struct Cell
   CellId mCellId;
 
   AMBIstruct mAmbi;
+  bool mHasAmbi;
 
   float mWater; // Water level
   bool mWaterInt;
@@ -152,6 +154,16 @@ struct Cell
       return ((mData.mFlags&HasWater) != 0) || isExterior();
   }
 
+  bool hasAmbient() const
+  {
+      return mHasAmbi;
+  }
+
+  void setHasAmbient(bool hasAmbi)
+  {
+      mHasAmbi = hasAmbi;
+  }
+
   // Restore the given reader to the stored position. Will try to open
   // the file matching the stored file name. If you want to read from
   // somewhere other than the file system, you need to pre-open the