Revert recent changes to CRenderWareSA.TextureReplacing.cpp (and related) once again after following 895be40 i tried to made less intrusive 'safe' changes again, but touching this stuff in any way just causes too many issues.

Dutchman101 · Dutchman101 · commit 0d4944982d28 · 2025-11-22T04:08:14.000+08:00
Crash fixes also only cause the crash to relocate. It all started with a desire to avert an old crash reported by Xenius (who was using txd's with invalid/null textures), but it only relocated it and caused new ones.
diff --git a/Client/game_sa/CRenderWareSA.TextureReplacing.cpp b/Client/game_sa/CRenderWareSA.TextureReplacing.cpp
@@ -12,7 +12,6 @@
 #include "gamesa_renderware.h"
 
 #include <map>
-#include <sstream>
 #include <unordered_map>
 #include <unordered_set>
 #include <utility>
@@ -113,49 +112,50 @@ namespace
 ////////////////////////////////////////////////////////////////
 CModelTexturesInfo* CRenderWareSA::GetModelTexturesInfo(ushort usModelId)
 {
-    auto* pModelInfo = dynamic_cast<CModelInfoSA*>(pGame->GetModelInfo(usModelId));
+    CModelInfoSA* pModelInfo = dynamic_cast<CModelInfoSA*>(pGame->GetModelInfo(usModelId));
     if (!pModelInfo)
-        return nullptr;
+        return NULL;
 
-    const ushort usTxdId = pModelInfo->GetTextureDictionaryID();
+    ushort usTxdId = pModelInfo->GetTextureDictionaryID();
 
-    if (auto it = ms_ModelTexturesInfoMap.find(usTxdId); it != ms_ModelTexturesInfoMap.end())
-        return &it->second;
-
-    // Get txd
-    RwTexDictionary* pTxd = CTxdStore_GetTxd(usTxdId);
-
-    if (!pTxd)
-    {
-        pModelInfo->Request(BLOCKING, "CRenderWareSA::GetModelTexturesInfo");
-        CTxdStore_AddRef(usTxdId);
-        ((void(__cdecl*)(unsigned short))FUNC_RemoveModel)(usModelId);
-        pTxd = CTxdStore_GetTxd(usTxdId);
-    }
-    else
+    CModelTexturesInfo* pInfo = MapFind(ms_ModelTexturesInfoMap, usTxdId);
+    if (!pInfo)
     {
-        CTxdStore_AddRef(usTxdId);
-        if (pModelInfo->GetModelType() == eModelInfoType::PED)
+        // Get txd
+        RwTexDictionary* pTxd = CTxdStore_GetTxd(usTxdId);
+
+        if (!pTxd)
         {
-            // Mystery fix for #9336: (MTA sometimes fails at loading custom textures)
-            // Possibly forces the ped model to be reloaded in some way
+            pModelInfo->Request(BLOCKING, "CRenderWareSA::GetModelTexturesInfo");
+            CTxdStore_AddRef(usTxdId);
             ((void(__cdecl*)(unsigned short))FUNC_RemoveModel)(usModelId);
+            pTxd = CTxdStore_GetTxd(usTxdId);
+        }
+        else
+        {
+            CTxdStore_AddRef(usTxdId);
+            if (pModelInfo->GetModelType() == eModelInfoType::PED)
+            {
+                // Mystery fix for #9336: (MTA sometimes fails at loading custom textures)
+                // Possibly forces the ped model to be reloaded in some way
+                ((void(__cdecl*)(unsigned short))FUNC_RemoveModel)(usModelId);
+            }
         }
-    }
 
-    if (!pTxd)
-        return nullptr;
+        if (!pTxd)
+            return NULL;
 
-    // Add new info
-    auto [it, inserted] = ms_ModelTexturesInfoMap.emplace(usTxdId, CModelTexturesInfo{});
-    auto& newInfo = it->second;
-    newInfo.usTxdId = usTxdId;
-    newInfo.pTxd = pTxd;
+        // Add new info
+        MapSet(ms_ModelTexturesInfoMap, usTxdId, CModelTexturesInfo());
+        pInfo = MapFind(ms_ModelTexturesInfoMap, usTxdId);
+        pInfo->usTxdId = usTxdId;
+        pInfo->pTxd = pTxd;
 
-    // Save original textures
-    GetTxdTextures(newInfo.originalTextures, newInfo.pTxd);
-    
-    return &newInfo;
+        // Save original textures
+        GetTxdTextures(pInfo->originalTextures, pInfo->pTxd);
+    }
+
+    return pInfo;
 }
 
 ////////////////////////////////////////////////////////////////
@@ -173,29 +173,27 @@ bool CRenderWareSA::ModelInfoTXDLoadTextures(SReplacementTextures* pReplacementT
         return false;
 
     // Try to load it
-    if (auto* pTxd = ReadTXD(strFilename, buffer); pTxd)
+    RwTexDictionary* pTxd = ReadTXD(strFilename, buffer);
+    if (pTxd)
     {
         // Get the list of textures into our own list
         GetTxdTextures(pReplacementTextures->textures, pTxd);
 
-        for (RwTexture* pTexture : pReplacementTextures->textures)
+        for (uint i = 0; i < pReplacementTextures->textures.size(); i++)
         {
-            if (!pTexture)
-                continue;
-
-            pTexture->txd = nullptr;
+            pReplacementTextures->textures[i]->txd = NULL;
             if (bFilteringEnabled)
-                pTexture->flags = 0x1102;  // Enable filtering (otherwise textures are pixely)
+                pReplacementTextures->textures[i]->flags = 0x1102;            // Enable filtering (otherwise textures are pixely)
         }
 
         // Make the txd forget it has any textures and destroy it
         pTxd->textures.root.next = &pTxd->textures.root;
         pTxd->textures.root.prev = &pTxd->textures.root;
         RwTexDictionaryDestroy(pTxd);
-        pTxd = nullptr;
+        pTxd = NULL;
 
         // We succeeded if we got any textures
-        return !pReplacementTextures->textures.empty();
+        return pReplacementTextures->textures.size() > 0;
     }
 
     return false;
@@ -230,14 +228,17 @@ bool CRenderWareSA::ModelInfoTXDAddTextures(SReplacementTextures* pReplacementTe
     //
     // Add section for this txd
     //
-    auto& perTxdInfo = pReplacementTextures->perTxdList.emplace_back();
+    pReplacementTextures->perTxdList.push_back(SReplacementTextures::SPerTxd());
+    SReplacementTextures::SPerTxd& perTxdInfo = pReplacementTextures->perTxdList.back();
 
     perTxdInfo.usTxdId = pInfo->usTxdId;
-    perTxdInfo.bTexturesAreCopies = !pReplacementTextures->usedInTxdIds.empty();
+    perTxdInfo.bTexturesAreCopies = (pReplacementTextures->usedInTxdIds.size() > 0);
 
     // Copy / clone textures
-    for (RwTexture* pNewTexture : pReplacementTextures->textures)
+    for (std::vector<RwTexture*>::iterator iter = pReplacementTextures->textures.begin(); iter != pReplacementTextures->textures.end(); iter++)
     {
+        RwTexture* pNewTexture = *iter;
+
         // Use a copy if not first txd
         if (perTxdInfo.bTexturesAreCopies)
         {
@@ -257,8 +258,10 @@ bool CRenderWareSA::ModelInfoTXDAddTextures(SReplacementTextures* pReplacementTe
     //
     // Add each texture to the target txd
     //
-    for (RwTexture* pNewTexture : perTxdInfo.usingTextures)
+    for (std::vector<RwTexture*>::iterator iter = perTxdInfo.usingTextures.begin(); iter != perTxdInfo.usingTextures.end(); iter++)
     {
+        RwTexture* pNewTexture = *iter;
+
         // If there is a name clash with an existing texture, replace it
         RwTexture* pExistingTexture = RwTexDictionaryFindNamedTexture(pInfo->pTxd, pNewTexture->name);
         if (pExistingTexture)
@@ -290,8 +293,10 @@ bool CRenderWareSA::ModelInfoTXDAddTextures(SReplacementTextures* pReplacementTe
 void CRenderWareSA::ModelInfoTXDRemoveTextures(SReplacementTextures* pReplacementTextures)
 {
     // For each using txd
-    for (auto& perTxdInfo : pReplacementTextures->perTxdList)
+    for (uint i = 0; i < pReplacementTextures->perTxdList.size(); i++)
     {
+        SReplacementTextures::SPerTxd& perTxdInfo = pReplacementTextures->perTxdList[i];
+
         // Get textures info
         ushort              usTxdId = perTxdInfo.usTxdId;
         CModelTexturesInfo* pInfo = MapFind(ms_ModelTexturesInfoMap, usTxdId);
@@ -303,21 +308,15 @@ void CRenderWareSA::ModelInfoTXDRemoveTextures(SReplacementTextures* pReplacemen
         TextureSwapMap swapMap;
         swapMap.reserve(perTxdInfo.usingTextures.size());
 
-        for (std::size_t idx = 0; idx < perTxdInfo.usingTextures.size(); ++idx)
+        for (size_t idx = 0; idx < perTxdInfo.usingTextures.size(); ++idx)
         {
             RwTexture* pOldTexture = perTxdInfo.usingTextures[idx];
             if (!pOldTexture)
                 continue;
 
             RwTexture* pOriginalTexture = (idx < perTxdInfo.replacedOriginals.size()) ? perTxdInfo.replacedOriginals[idx] : nullptr;
-            
-            if (pOriginalTexture && !SharedUtil::IsReadablePointer(pOriginalTexture, sizeof(RwTexture)))
-                pOriginalTexture = nullptr;
-                
             swapMap[pOldTexture] = pOriginalTexture;
 
-            // Always restore original textures to prevent them from becoming orphaned and GC'd
-            // This is crucial for proper cleanup even when multiple replacement sets use the same TXD
             if (pOriginalTexture && !RwTexDictionaryContainsTexture(pInfo->pTxd, pOriginalTexture))
                 RwTexDictionaryAddTexture(pInfo->pTxd, pOriginalTexture);
         }
@@ -331,7 +330,7 @@ void CRenderWareSA::ModelInfoTXDRemoveTextures(SReplacementTextures* pReplacemen
 
             for (ushort modelId : pReplacementTextures->usedInModelIds)
             {
-                auto* pModelInfo = dynamic_cast<CModelInfoSA*>(pGame->GetModelInfo(modelId));
+                CModelInfoSA* pModelInfo = dynamic_cast<CModelInfoSA*>(pGame->GetModelInfo(modelId));
                 if (!pModelInfo)
                     continue;
 
@@ -349,14 +348,9 @@ void CRenderWareSA::ModelInfoTXDRemoveTextures(SReplacementTextures* pReplacemen
         }
 
         // Remove replacement textures
-        for (RwTexture* pOldTexture : perTxdInfo.usingTextures)
+        for (uint i = 0; i < perTxdInfo.usingTextures.size(); i++)
         {
-            if (!pOldTexture)
-                continue;
-            
-            if (!SharedUtil::IsReadablePointer(pOldTexture, sizeof(RwTexture)))
-                continue;
-                
+            RwTexture* pOldTexture = perTxdInfo.usingTextures[i];
             RwTexDictionaryRemoveTexture(pInfo->pTxd, pOldTexture);
             dassert(!RwTexDictionaryContainsTexture(pInfo->pTxd, pOldTexture));
             if (perTxdInfo.bTexturesAreCopies)
@@ -368,23 +362,12 @@ void CRenderWareSA::ModelInfoTXDRemoveTextures(SReplacementTextures* pReplacemen
         }
 
         perTxdInfo.usingTextures.clear();
-        
-        // Free replaced textures that aren't in originalTextures (prevents leak)
-        for (RwTexture* pReplacedTexture : perTxdInfo.replacedOriginals)
-        {
-            if (pReplacedTexture && SharedUtil::IsReadablePointer(pReplacedTexture, sizeof(RwTexture)) && !ListContains(pInfo->originalTextures, pReplacedTexture))
-                RwTextureDestroy(pReplacedTexture);
-        }
-
         perTxdInfo.replacedOriginals.clear();
 
         // Ensure there are original named textures in the txd
-        for (RwTexture* pOriginalTexture : pInfo->originalTextures)
+        for (uint i = 0; i < pInfo->originalTextures.size(); i++)
         {
-            // Skip null/invalid textures (can happen during shutdown)
-            if (!pOriginalTexture)
-                continue;
-                
+            RwTexture* pOriginalTexture = pInfo->originalTextures[i];
             if (!RwTexDictionaryFindNamedTexture(pInfo->pTxd, pOriginalTexture->name))
                 RwTexDictionaryAddTexture(pInfo->pTxd, pOriginalTexture);
         }
@@ -399,101 +382,29 @@ void CRenderWareSA::ModelInfoTXDRemoveTextures(SReplacementTextures* pReplacemen
         #ifdef MTA_DEBUG
             std::vector<RwTexture*> currentTextures;
             GetTxdTextures(currentTextures, pInfo->pTxd);
-            
-            auto formatTextures = [](const std::vector<RwTexture*>& textures) -> std::string {
-                std::ostringstream result;
-                for (size_t i = 0; i < textures.size(); ++i)
-                {
-                    const auto* pTex = textures[i];
-                    const bool isValid = pTex && SharedUtil::IsReadablePointer(pTex, sizeof(RwTexture));
-                    const bool isLast = (i == textures.size() - 1);
-                    
-                    if (isValid)
-                        result << pTex->name << "[0x" << std::hex << pTex << std::dec << "]";
-                    else
-                        result << "INVALID[0x" << std::hex << pTex << std::dec << "]";
-                    
-                    if (!isLast)
-                        result << ", ";
-                }
-                return result.str();
-            };
-            
-            // Allow size mismatch in case texture removal was skipped due to invalid pointers
-            if (currentTextures.size() != pInfo->originalTextures.size())
+            assert(currentTextures.size() == pInfo->originalTextures.size());
+            for (uint i = 0; i < pInfo->originalTextures.size(); i++)
             {
-                std::ostringstream debugMsg;
-                debugMsg << "TXD " << pInfo->usTxdId << ": texture count mismatch (current=" 
-                         << currentTextures.size() << ", expected=" << pInfo->originalTextures.size() << ")\n";
-                debugMsg << "  Current textures: " << formatTextures(currentTextures);
-                debugMsg << "\n  Expected textures: " << formatTextures(pInfo->originalTextures);
-                debugMsg << "\n";
-                OutputDebugString(debugMsg.str().c_str());
-            }
-            else
-            {
-                // First pass: validate all original textures are present
-                for (const auto* pOriginalTexture : pInfo->originalTextures)
-                {
-                    if (!pOriginalTexture)
-                        continue;
-                    if (!ListContains(currentTextures, pOriginalTexture))
-                    {
-                        const char* texName = SharedUtil::IsReadablePointer(pOriginalTexture, sizeof(RwTexture)) 
-                            ? pOriginalTexture->name : "INVALID";
-                        std::ostringstream oss;
-                        oss << "Original texture not found in TXD " << pInfo->usTxdId 
-                            << " - texture '" << texName << "' [0x" << std::hex << pOriginalTexture << std::dec 
-                            << "] was removed or replaced unexpectedly";
-                        const std::string assertMsg = oss.str();
-                        assert(false && assertMsg.c_str());
-                    }
-                }
-                
-                // Second pass: remove original textures from current list to find extras
-                for (auto* pOriginalTexture : pInfo->originalTextures)
-                {
-                    if (pOriginalTexture)
-                        ListRemove(currentTextures, pOriginalTexture);
-                }
-                
-                // Check for leaked textures
-                if (!currentTextures.empty())
-                {
-                    std::ostringstream oss;
-                    oss << "Extra textures remain in TXD " << pInfo->usTxdId 
-                        << " after removing all originals - indicates texture leak. Remaining: "
-                        << formatTextures(currentTextures);
-                    const std::string assertMsg = oss.str();
-                    assert(false && assertMsg.c_str());
-                }
+                RwTexture* pOriginalTexture = pInfo->originalTextures[i];
+                assert(ListContains(currentTextures, pOriginalTexture));
+                ListRemove(currentTextures, pOriginalTexture);
             }
+            assert(currentTextures.empty());
 
-            const int32_t refsCount = CTxdStore_GetNumRefs(pInfo->usTxdId);
-            if (refsCount <= 0)
-            {
-                std::ostringstream oss;
-                oss << "TXD " << pInfo->usTxdId << " has invalid ref count " 
-                    << refsCount << " - should be > 0 before cleanup";
-                const std::string assertMsg = oss.str();
-                assert(false && assertMsg.c_str());
-            }
+            int32_t refsCount = CTxdStore_GetNumRefs(pInfo->usTxdId);
+            assert(refsCount > 0 && "Should have at least one TXD reference here");
         #endif
-            // Clear original textures to prevent dangling pointers after TXD ref removal
-            // The textures themselves are owned by the TXD and will be cleaned up when ref count hits zero
-            pInfo->originalTextures.clear();
-            
             // Remove info
             CTxdStore_RemoveRef(pInfo->usTxdId);
             MapRemove(ms_ModelTexturesInfoMap, usTxdId);
         }
     }
 
     // Destroy replacement textures
-    for (RwTexture* pTexture : pReplacementTextures->textures)
+    for (uint i = 0; i < pReplacementTextures->textures.size(); i++)
     {
-        if (pTexture)
-            DestroyTexture(pTexture);
+        RwTexture* pOldTexture = pReplacementTextures->textures[i];
+        DestroyTexture(pOldTexture);
     }
     pReplacementTextures->textures.clear();
 }
diff --git a/Client/game_sa/CRenderWareSA.cpp b/Client/game_sa/CRenderWareSA.cpp
@@ -737,9 +737,6 @@ void CRenderWareSA::DestroyTexture(RwTexture* pTex)
 
 void CRenderWareSA::RwTexDictionaryRemoveTexture(RwTexDictionary* pTXD, RwTexture* pTex)
 {
-    if (!pTex || !pTXD)
-        return;
-        
     if (pTex->txd != pTXD)
         return;
 
@@ -755,17 +752,6 @@ short CRenderWareSA::CTxdStore_GetTxdRefcount(unsigned short usTxdID)
 
 bool CRenderWareSA::RwTexDictionaryContainsTexture(RwTexDictionary* pTXD, RwTexture* pTex)
 {
-    // Avoid crashes with freed/invalid textures and TXDs
-    if (!pTex || !pTXD)
-        return false;
-
-    // Prevent crash when texture/TXD has been freed but pointer still exists
-    if (!SharedUtil::IsReadablePointer(pTex, sizeof(RwTexture)))
-        return false;
-    
-    if (!SharedUtil::IsReadablePointer(pTXD, sizeof(RwTexDictionary)))
-        return false;
-    
     return pTex->txd == pTXD;
 }
 
diff --git a/Client/game_sa/CRenderWareSA.h b/Client/game_sa/CRenderWareSA.h
@@ -16,7 +16,7 @@
 #include "CRenderWareSA.ShaderSupport.h"
 
 class CMatchChannelManager;
-struct CModelTexturesInfo;
+class CModelTexturesInfo;
 struct RpAtomic;
 struct SShaderReplacementStats;
 struct STexInfo;
