Improve collision impl

Author: Dutchman101

Client/game_sa/CModelInfoSA.cpp

@@ -1540,6 +1540,7 @@ bool CModelInfoSA::SetCustomModel(RpClump* pClump)
             success = pGame->GetRenderWare()->ReplaceWeaponModel(pClump, static_cast<unsigned short>(m_dwModelID));
             break;
         case eModelInfoType::VEHICLE:
+            // ReplaceVehicleModele handles collision preservation internally
             success = pGame->GetRenderWare()->ReplaceVehicleModel(pClump, static_cast<unsigned short>(m_dwModelID));
             break;
         case eModelInfoType::ATOMIC:
@@ -1562,9 +1563,8 @@ void CModelInfoSA::RestoreOriginalModel()
     {
         pGame->GetStreaming()->RemoveModel(m_dwModelID);
     }
-
     // Reset the stored custom vehicle clump
-    m_pCustomClump = NULL;
+    m_pCustomClump = nullptr;
 }
 
 void CModelInfoSA::SetColModel(CColModel* pColModel)

Client/game_sa/CPoolsSA.cpp

@@ -88,27 +88,23 @@ CVehicle* CPoolsSA::AddVehicle(CClientVehicle* pClientVehicle, std::uint16_t mod
 
     CBaseModelInfoSAInterface* pModelInterface = pModelInfo->GetInterface();
 
+    // Ensure collision model pointer exists
     if (!pModelInterface->pColModel)
     {
         // Collision model pointer is NULL - try loading
         pGame->GetStreaming()->LoadAllRequestedModels(false, "CPoolsSA::AddVehicle");
 
-        // Re-check after loading - still NULL means loading failed
-        if (!pModelInterface->pColModel)
-            return nullptr;
-    }
-    
-    // Check if collision data (m_pColData) is loaded
-    if (!pModelInterface->pColModel->m_data)
-    {
-        // Collision data not loaded - force load
-        pGame->GetStreaming()->LoadAllRequestedModels(false, "CPoolsSA::AddVehicle");
+        // Re-fetch interface as loading may have invalidated pointer
+        pModelInterface = pModelInfo->GetInterface();
 
-        // Re-check after loading - still not loaded means loading failed
-        if (!pModelInterface->pColModel->m_data)
+        // Still NULL means model has no collision (or loading failed) - block creation
+        if (!pModelInterface->pColModel)
             return nullptr;
     }
 
+    // Note: Vehicles with custom DFFs (no embedded collision) are handled in CModelInfoSA::SetCustomModel
+    // where collision is reloaded after SetClump to restore pool-managed collision data
+
     MemSetFast((void*)VAR_CVehicle_Variation1, variation, 1);
     MemSetFast((void*)VAR_CVehicle_Variation2, variation2, 1);
 

Client/game_sa/CRenderWareSA.cpp

@@ -437,6 +437,22 @@ bool CRenderWareSA::ReplaceModel(RpClump* pNew, unsigned short usModelID, DWORD
 
             CBaseModelInfoSAInterface* pModelInfoInterface = pModelInfo->GetInterface();
             CBaseModelInfo_SetClump(pModelInfoInterface, pNewClone);
+            
+            // Re-fetch interface pointer after SetClump (may relocate/change)
+            pModelInfoInterface = pModelInfo->GetInterface();
+            
+            // Fix for custom DFF without embedded collision:
+            // SetClump clears pColModel when DFF has no collision data, but vehicles need collision from .col pool.
+            // Solution: Remove + Request + Load to restore pool-managed collision from data/vehicles.col
+            if (dwSetClumpFunction == FUNC_LoadVehicleModel && !pModelInfoInterface->pColModel)
+            {
+                pGame->GetStreaming()->RemoveModel(usModelID);
+                pGame->GetStreaming()->RequestModel(usModelID, 0x16);
+                pGame->GetStreaming()->LoadAllRequestedModels(false, "CRenderWareSA::ReplaceVehicleModel");
+                // Re-fetch interface pointer after model reload
+                pModelInfoInterface = pModelInfo->GetInterface();
+            }
+            
             RpClumpDestroy(pOldClump);
         }
     }
@@ -470,69 +486,75 @@ bool CRenderWareSA::ReplacePedModel(RpClump* pNew, unsigned short usModelID)
     return ReplaceModel(pNew, usModelID, FUNC_LoadPedModel);
 }
 
-// Reads and parses a COL3 file
+// Reads and parses a COL file (versions 1-4: COLL, COL2, COL3, COL4)
 CColModel* CRenderWareSA::ReadCOL(const SString& buffer)
 {
-    if (buffer.size() < sizeof(ColModelFileHeader) + 16)
-        return NULL;
+    // Validate minimum buffer size
+    if (buffer.size() < sizeof(ColModelFileHeader) + 16) [[unlikely]]
+        return nullptr;
 
-    const ColModelFileHeader& header = *(ColModelFileHeader*)buffer.data();
+    const auto& header = *reinterpret_cast<const ColModelFileHeader*>(buffer.data());
 
-    // Validate version string is null-terminated
-    bool versionValid = false;
-    for (std::size_t i = 0; i < sizeof(header.version); ++i)
-    {
-        if (header.version[i] == '\0')
-        {
-            versionValid = true;
-            break;
-        }
-    }
-    if (!versionValid)
+    // Validate version field contains valid COL magic number
+    // Version is 4-char fixed string (not null-terminated): "COLL", "COL2", "COL3", "COL4"
+    constexpr std::array<std::array<char, 4>, 4> validVersions = {{
+        {'C', 'O', 'L', 'L'},
+        {'C', 'O', 'L', '2'},
+        {'C', 'O', 'L', '3'},
+        {'C', 'O', 'L', '4'}
+    }};
+    
+    const bool isValidVersion = std::any_of(validVersions.begin(), validVersions.end(),
+        [&header](const auto& valid) { 
+            return std::equal(valid.begin(), valid.end(), header.version);
+        });
+
+    if (!isValidVersion) [[unlikely]]
     {
-        AddReportLog(8622, "ReadCOL: Invalid header - version field not null-terminated");
+        // Explicitly limit to 4 characters
+        AddReportLog(8622, SString("ReadCOL: Invalid version '%c%c%c%c' - expected COLL, COL2, COL3, or COL4",
+            header.version[0], header.version[1], header.version[2], header.version[3]));
         return nullptr;
     }
 
-    // Load the col model
-    if (header.version[0] == 'C' && header.version[1] == 'O' && header.version[2] == 'L')
+    // Ensure name field is null-terminated to prevent buffer overrun
+    const auto* nameEnd = static_cast<const char*>(std::memchr(header.name, '\0', sizeof(header.name)));
+    if (!nameEnd) [[unlikely]]
     {
-        // Validate name field is null-terminated to prevent buffer overrun
-        bool nameValid = false;
-        for (std::size_t i = 0; i < sizeof(header.name); ++i)
-        {
-            if (header.name[i] == '\0')
-            {
-                nameValid = true;
-                break;
-            }
-        }
-        if (!nameValid)
-            AddReportLog(8623, "ReadCOL: Name field not null-terminated, may be truncated");
-
-        unsigned char* pModelData = (unsigned char*)buffer.data() + sizeof(ColModelFileHeader);
+        AddReportLog(8623, "ReadCOL: Name field not null-terminated, may be truncated");
+        return nullptr;
+    }
 
-        // Create a new CColModel
-        CColModelSA* pColModel = new CColModelSA();
+    // Buffer is not modified by us, but GTA's functions expect non-const
+    auto* pModelData = const_cast<unsigned char*>(reinterpret_cast<const unsigned char*>(buffer.data())) + sizeof(ColModelFileHeader);
 
-        if (header.version[3] == 'L')
-        {
-            LoadCollisionModel(pModelData, pColModel->GetInterface(), NULL);
-        }
-        else if (header.version[3] == '2')
-        {
-            LoadCollisionModelVer2(pModelData, header.size - 0x18, pColModel->GetInterface(), NULL);
-        }
-        else if (header.version[3] == '3')
-        {
-            LoadCollisionModelVer3(pModelData, header.size - 0x18, pColModel->GetInterface(), NULL);
-        }
+    // Create a new CColModel
+    auto* pColModel = new CColModelSA();
 
-        // Return the collision model
-        return pColModel;
+    // Load appropriate collision version
+    switch (header.version[3])
+    {
+        case 'L':
+            LoadCollisionModel(pModelData, pColModel->GetInterface(), nullptr);
+            break;
+        case '2':
+            LoadCollisionModelVer2(pModelData, header.size - 0x18, pColModel->GetInterface(), nullptr);
+            break;
+        case '3':
+            LoadCollisionModelVer3(pModelData, header.size - 0x18, pColModel->GetInterface(), nullptr);
+            break;
+        case '4':
+            // COL4 format has same structure as COL3 with one extra uint32 field in header
+            // Must use Ver4 loader for correct offset calculations
+            LoadCollisionModelVer4(pModelData, header.size - 0x18, pColModel->GetInterface(), nullptr);
+            break;
+        default:
+            // Should never reach here due to validation above
+            delete pColModel;
+            return nullptr;
     }
 
-    return NULL;
+    return pColModel;
 }
 
 // Loads all atomics from a clump into a container struct and returns the number of atomics it loaded

Client/game_sa/CRenderWareSA.h

@@ -54,7 +54,7 @@ class CRenderWareSA : public CRenderWare
     // Destroys a texture
     void DestroyTexture(RwTexture* pTex);
 
-    // Reads and parses a COL3 file with an optional collision key name
+    // Reads and parses a COL file (versions 1-4: COLL, COL2, COL3, COL4)
     CColModel* ReadCOL(const SString& buffer);
 
     // Replaces a CColModel for a specific object identified by the object id (usModelID)

Client/game_sa/CVehicleSA.cpp

@@ -1784,20 +1784,20 @@ void* CVehicleSA::GetPrivateSuspensionLines()
 void CVehicleSA::CopyGlobalSuspensionLinesToPrivate()
 {
     CModelInfo* pModelInfo = pGame->GetModelInfo(GetModelIndex());
-    if (!pModelInfo) [[unlikely]]
+    if (!pModelInfo)
         return;
 
     // Protect collision model from streaming GC
     CColModelGuard guard(static_cast<CModelInfoSA*>(pModelInfo));
-    if (!guard.IsValid()) [[unlikely]]
+    if (!guard.IsValid())
         return;
 
     CColDataSA* pColData = guard.GetColData();
-    if (!pColData || !pColData->m_suspensionLines) [[unlikely]]
+    if (!pColData || !pColData->m_suspensionLines)
         return;
 
     void* pPrivateLines = GetPrivateSuspensionLines();
-    if (!pPrivateLines) [[unlikely]]
+    if (!pPrivateLines)
         return;
 
     // Determine copy size based on vehicle type
@@ -1828,13 +1828,13 @@ void CVehicleSA::CopyGlobalSuspensionLinesToPrivate()
 void CVehicleSA::RecalculateSuspensionLines()
 {
     CHandlingEntry* pHandlingEntry = GetHandlingData();
-    if (!pHandlingEntry) [[unlikely]]
+    if (!pHandlingEntry)
         return;
 
     const std::uint32_t dwModel = GetModelIndex();
 
     CModelInfo* pModelInfo = pGame->GetModelInfo(dwModel);
-    if (!pModelInfo) [[unlikely]]
+    if (!pModelInfo)
         return;
 
     // Only for vehicles with suspension lines
@@ -1848,11 +1848,11 @@ void CVehicleSA::RecalculateSuspensionLines()
 
     // Protect collision model before accessing suspension data
     CColModelGuard guard(static_cast<CModelInfoSA*>(pModelInfo));
-    if (!guard.IsValid()) [[unlikely]]
+    if (!guard.IsValid())
         return;
 
     CVehicleSAInterface* pVehicleInterface = GetVehicleInterface();
-    if (!pVehicleInterface) [[unlikely]]
+    if (!pVehicleInterface)
         return;
 
     // Safe to call now - collision is protected by guard

Client/game_sa/gamesa_renderware.h

@@ -208,6 +208,8 @@ typedef void(__cdecl* LoadCollisionModel_t)(unsigned char*, CColModelSAInterface
 typedef void(__cdecl* LoadCollisionModelVer2_t)(unsigned char*, unsigned int, CColModelSAInterface*, const char*);
 typedef void(__cdecl* LoadCollisionModelVer3_t)(unsigned char*, unsigned int, CColModelSAInterface*,
                                                 const char*);            // buf, bufsize, ccolmodel&, keyname
+typedef void(__cdecl* LoadCollisionModelVer4_t)(unsigned char*, unsigned int, CColModelSAInterface*,
+                                                const char*);            // buf, bufsize, ccolmodel&, keyname
 typedef bool(__cdecl* CTxdStore_LoadTxd_t)(unsigned int id, RwStream* filename);
 typedef void(__cdecl* CTxdStore_RemoveTxd_t)(unsigned int id);
 typedef void(__cdecl* CTxdStore_RemoveRef_t)(unsigned int id);
@@ -222,6 +224,7 @@ RWFUNC(LoadModel_t LoadModel, (LoadModel_t)0xDEAD)
 RWFUNC(LoadCollisionModel_t LoadCollisionModel, (LoadCollisionModel_t)0xDEAD)
 RWFUNC(LoadCollisionModelVer2_t LoadCollisionModelVer2, (LoadCollisionModelVer2_t)0xDEAD)
 RWFUNC(LoadCollisionModelVer3_t LoadCollisionModelVer3, (LoadCollisionModelVer3_t)0xDEAD)
+RWFUNC(LoadCollisionModelVer4_t LoadCollisionModelVer4, (LoadCollisionModelVer4_t)0xDEAD)
 RWFUNC(CTxdStore_LoadTxd_t CTxdStore_LoadTxd, (CTxdStore_LoadTxd_t)0xDEAD)
 RWFUNC(CTxdStore_GetTxd_t CTxdStore_GetTxd, (CTxdStore_GetTxd_t)0xDEAD)
 RWFUNC(CTxdStore_RemoveTxd_t CTxdStore_RemoveTxd, (CTxdStore_RemoveTxd_t)0xDEAD)

Client/game_sa/gamesa_renderware.hpp

@@ -96,6 +96,7 @@ void InitRwFunctions()
     LoadCollisionModel = (LoadCollisionModel_t)0x00537580;
     LoadCollisionModelVer2 = (LoadCollisionModelVer2_t)0x00537EE0;
     LoadCollisionModelVer3 = (LoadCollisionModelVer3_t)0x00537CE0;
+    LoadCollisionModelVer4 = (LoadCollisionModelVer4_t)0x00537AE0;
     CTxdStore_LoadTxd = (CTxdStore_LoadTxd_t)0x00731DD0;
     CTxdStore_GetTxd = (CTxdStore_GetTxd_t)0x00408340;
     CTxdStore_RemoveTxd = (CTxdStore_RemoveTxd_t)0x00731E90;

Client/mods/deathmatch/logic/CClientColModel.cpp

@@ -157,5 +157,16 @@ void CClientColModel::InternalRestore(unsigned short usModel)
 // Return true if data looks like COL file contents
 bool CClientColModel::IsCOLData(const SString& strData)
 {
-    return strData.length() > 32 && memcmp(strData, "COL", 3) == 0 && strData[7] == 0;
+    // COL file format: version[4] + size[4] + name[24] = minimum 32 bytes
+    // Version is 4-char string: "COLL", "COL2", "COL3", or "COL4" (not null-terminated)
+    if (strData.length() < 32)
+        return false;
+
+    // Check if starts with "COL" magic number
+    if (memcmp(strData, "COL", 3) != 0)
+        return false;
+
+    // Validate 4th character is valid version indicator
+    const char versionChar = strData[3];
+    return versionChar == 'L' || versionChar == '2' || versionChar == '3' || versionChar == '4';
 }