Workaround for custom map collision crashes (#1613)

saml1er · codenulls · web-flow · commit 96175175b3cc · 2020-08-23T11:08:24.000+05:00
* Potentional fix for custom maps crashes

* properly set hooks

* some improvements suggested in PR review

* indentation fixes

* Add useful comments to CRect

* Fix purpose in copyright comment

Co-authored-by: saml1er &lt;danishroar@gmail.com&gt;
diff --git a/Client/game_sa/CEntitySA.cpp b/Client/game_sa/CEntitySA.cpp
@@ -16,6 +16,44 @@ extern CGameSA* pGame;
 unsigned long CEntitySA::FUNC_CClumpModelInfo__GetFrameFromId;
 unsigned long CEntitySA::FUNC_RwFrameGetLTM;
 
+void CEntitySAInterface::TransformFromObjectSpace(CVector& outPosn, CVector const& offset)
+{
+    ((void(__thiscall*)(CEntitySAInterface*, CVector&, CVector const&))0x533560)(this, outPosn, offset);
+}
+
+CVector* CEntitySAInterface::GetBoundCentre(CVector* pOutCentre)
+{
+    return ((CVector * (__thiscall*)(CEntitySAInterface*, CVector*))0x534250)(this, pOutCentre);
+}
+
+CRect* CEntitySAInterface::GetBoundRect_(CRect* pRect)
+{
+    CColModelSAInterface* colModel = CModelInfoSAInterface::GetModelInfo(m_nModelIndex)->pColModel;
+    CVector               vecMin = colModel->boundingBox.vecMin;
+    CVector               vecMax = colModel->boundingBox.vecMax;
+    CRect                 rect;
+    CVector               point;
+    TransformFromObjectSpace(point, vecMin);
+    rect.StretchToPoint(point.fX, point.fY);
+    TransformFromObjectSpace(point, vecMax);
+    rect.StretchToPoint(point.fX, point.fY);
+    float maxX = vecMax.fX;
+    vecMax.fX = vecMin.fX;
+    vecMin.fX = maxX;
+    TransformFromObjectSpace(point, vecMin);
+    rect.StretchToPoint(point.fX, point.fY);
+    TransformFromObjectSpace(point, vecMax);
+    rect.StretchToPoint(point.fX, point.fY);
+    *pRect = rect;
+    pRect->FixIncorrectTopLeft();            // Fix #1613: custom map collision crashes in CPhysical class (infinite loop)
+    return pRect;
+}
+
+void CEntitySAInterface::StaticSetHooks()
+{
+    HookInstall(0x534120, &CEntitySAInterface::GetBoundRect_);
+}
+
 CEntitySA::CEntitySA()
 {
     // Set these variables to a constant state
diff --git a/Client/game_sa/CEntitySA.h b/Client/game_sa/CEntitySA.h
@@ -33,6 +33,7 @@
 // not in CEntity really
 #define FUNC_RpAnimBlendClumpGetAssociation                 0x4D6870
 
+class CRect;
 class CEntitySAInterfaceVTBL
 {
 public:
@@ -193,6 +194,11 @@ class CEntitySAInterface
 
     uint8 m_pad0;            // 55
 
+    CRect*      GetBoundRect_(CRect* pRect);
+    void        TransformFromObjectSpace(CVector& outPosn, CVector const& offset);
+    CVector*    GetBoundCentre(CVector* pOutCentre);
+    static void StaticSetHooks();
+
     //
     // Functions to hide member variable misuse
     //
@@ -227,13 +233,12 @@ class CEntitySA : public virtual CEntity
     friend class COffsets;
 
 public:
-    CEntitySA();
-
     CEntitySAInterface* m_pInterface;
 
     DWORD internalID;
     //  VOID                        SetModelAlpha ( int iAlpha );
 
+    CEntitySA();
     CEntitySAInterface* GetInterface() { return m_pInterface; };
     VOID                SetInterface(CEntitySAInterface* intInterface) { m_pInterface = intInterface; };
 
diff --git a/Client/game_sa/CGameSA.cpp b/Client/game_sa/CGameSA.cpp
@@ -200,6 +200,8 @@ CGameSA::CGameSA()
     // Increase matrix array size
     MemPut<int>(0x054F3A1, MAX_OBJECTS * 3);            // Default is 900
 
+    CEntitySAInterface::StaticSetHooks();
+    CPhysicalSAInterface::StaticSetHooks();
     CModelInfoSA::StaticSetHooks();
     CPlayerPedSA::StaticSetHooks();
     CRenderWareSA::StaticSetHooks();
diff --git a/Client/game_sa/CModelInfoSA.cpp b/Client/game_sa/CModelInfoSA.cpp
@@ -14,6 +14,7 @@
 
 extern CGameSA* pGame;
 
+CBaseModelInfoSAInterface** CModelInfoSAInterface::ms_modelInfoPtrs = (CBaseModelInfoSAInterface**)0xA9B0C8;
 CBaseModelInfoSAInterface** ppModelInfo = (CBaseModelInfoSAInterface**)ARRAY_ModelInfo;
 
 std::map<unsigned short, int>                                         CModelInfoSA::ms_RestreamTxdIDMap;
diff --git a/Client/game_sa/CModelInfoSA.h b/Client/game_sa/CModelInfoSA.h
@@ -65,7 +65,18 @@ class CPedModelInfoSAInterface;
 #define     FUNC_SetColModel                0x4C4BC0
 #define     FUNC_AddPedModel                0x4c67a0
 #define     VAR_CTempColModels_ModelPed1    0x968DF0
-/**
+
+
+class CBaseModelInfoSAInterface;
+class CModelInfoSAInterface
+{
+public:
+    // Use GetModelInfo(int index) to get model info by id
+    static CBaseModelInfoSAInterface** ms_modelInfoPtrs;
+    static CBaseModelInfoSAInterface*  GetModelInfo(int index) { return ms_modelInfoPtrs[index]; }
+};
+
+    /**
  * \todo Fill this class with info from R*
  */
 class CBaseModelInfo_SA_VTBL
diff --git a/Client/game_sa/CPhysicalSA.cpp b/Client/game_sa/CPhysicalSA.cpp
@@ -11,6 +11,21 @@
 
 #include "StdInc.h"
 
+CRect* CPhysicalSAInterface::GetBoundRect_(CRect* pRect)
+{
+    CVector boundCentre;
+    CEntitySAInterface::GetBoundCentre(&boundCentre);
+    float fRadius = CModelInfoSAInterface::GetModelInfo(m_nModelIndex)->pColModel->boundingBox.fRadius;
+    *pRect = CRect(boundCentre.fX - fRadius, boundCentre.fY - fRadius, boundCentre.fX + fRadius, boundCentre.fY + fRadius);
+    pRect->FixIncorrectTopLeft();            // Fix #1613: custom map collision crashes in CPhysical class (infinite loop)
+    return pRect;
+}
+
+void CPhysicalSAInterface::StaticSetHooks()
+{
+    HookInstall(0x5449B0, &CPhysicalSAInterface::GetBoundRect_);
+}
+
 void CPhysicalSA::RestoreLastGoodPhysicsState()
 {
     CEntitySA::RestoreLastGoodPhysicsState();
diff --git a/Client/game_sa/CPhysicalSA.h b/Client/game_sa/CPhysicalSA.h
@@ -106,6 +106,9 @@ class CPhysicalSAInterface : public CEntitySAInterface
     float                     m_fLighting;                                // 300
     float                     m_fLighting2;                               // 304
     class CShadowDataSA*      m_pShadowData;                              // 308
+
+    CRect*      GetBoundRect_(CRect* pRect);
+    static void StaticSetHooks();
 };
 static_assert(sizeof(CPhysicalSAInterface) == 0x138, "Invalid size for CPhysicalSAInterface");
 
diff --git a/Client/game_sa/CRadarSA.cpp b/Client/game_sa/CRadarSA.cpp
@@ -175,20 +175,11 @@ VOID CRadarSA::ClearMarkerForEntity(CPed* ped)
     }
 }
 
-struct CRect
-{
-    float fX1, fY1, fX2, fY2;
-};
-
 VOID CRadarSA::DrawAreaOnRadar(float fX1, float fY1, float fX2, float fY2, const SColor color)
 {
     // Convert color to required abgr at the last moment
     unsigned long abgr = color.A << 24 | color.B << 16 | color.G << 8 | color.R;
-    CRect         myRect;
-    myRect.fX1 = fX1;
-    myRect.fY1 = fY1;
-    myRect.fX2 = fX2;
-    myRect.fY2 = fY2;
+    CRect         myRect(fX1, fY1, fX2, fY2);
     DWORD dwFunc = FUNC_DrawAreaOnRadar;
     _asm
     {
diff --git a/Client/game_sa/HookSystem.cpp b/Client/game_sa/HookSystem.cpp
@@ -11,25 +11,6 @@
 
 #include "StdInc.h"
 
-//
-////////////////////////////////////////////////////////////////////
-
-BOOL HookInstall(DWORD dwInstallAddress, DWORD dwHookHandler, int iJmpCodeSize)
-{
-    BYTE JumpBytes[MAX_JUMPCODE_SIZE];
-    MemSetFast(JumpBytes, 0x90, MAX_JUMPCODE_SIZE);
-    if (CreateJump(dwInstallAddress, dwHookHandler, JumpBytes))
-    {
-        MemCpy((PVOID)dwInstallAddress, JumpBytes, iJmpCodeSize);
-        return TRUE;
-    }
-    else
-    {
-        return FALSE;
-    }
-}
-
-////////////////////////////////////////////////////////////////////
 
 BYTE* CreateJump(DWORD dwFrom, DWORD dwTo, BYTE* ByteArray)
 {
diff --git a/Client/game_sa/HookSystem.h b/Client/game_sa/HookSystem.h
@@ -16,7 +16,33 @@
 
 #define     MAX_JUMPCODE_SIZE           20
 
-BOOL  HookInstall(DWORD dwInstallAddress, DWORD dwHookHandler, int iJmpCodeSize);
+template <typename T>
+void* FunctionPointerToVoidP(T func)
+{
+    union
+    {
+        T     a;
+        void* b;
+    } c = {func};
+    return c.b;
+}
+
+template <typename T>
+BOOL HookInstall(DWORD dwInstallAddress, T dwHookHandler, int iJmpCodeSize = 5)
+{
+    BYTE JumpBytes[MAX_JUMPCODE_SIZE];
+    MemSetFast(JumpBytes, 0x90, MAX_JUMPCODE_SIZE);
+    if (CreateJump(dwInstallAddress, (DWORD)FunctionPointerToVoidP(dwHookHandler), JumpBytes))
+    {
+        MemCpy((PVOID)dwInstallAddress, JumpBytes, iJmpCodeSize);
+        return TRUE;
+    }
+    else
+    {
+        return FALSE;
+    }
+}
+
 BYTE* CreateJump(DWORD dwFrom, DWORD dwTo, BYTE* ByteArray);
 
 // Auto detect requirement of US/EU hook installation
diff --git a/Client/game_sa/StdInc.h b/Client/game_sa/StdInc.h
@@ -20,6 +20,7 @@
 
 // SDK includes
 #include <core/CCoreInterface.h>
+#include <CRect.h>
 #include <net/CNet.h>
 #include <game/CGame.h>
 #include <game/CWanted.h>
diff --git a/Shared/sdk/CRect.h b/Shared/sdk/CRect.h
@@ -0,0 +1,91 @@
+/*****************************************************************************
+ *
+ *  PROJECT:     Multi Theft Auto
+ *  LICENSE:     See LICENSE in the top level directory
+ *  FILE:        sdk/CRect.h
+ *  PURPOSE:     This is an actual GTA SA class. MTA has its own version of
+ *               CRect, and it's called CRect2D. CRect behaves similarly to
+ *               CRect2D, except it's used by GTA:SA, and the initial values
+ *               are NOT zero when constructed. 
+ *
+ *  Multi Theft Auto is available from http://www.multitheftauto.com/
+ *
+ *****************************************************************************/
+
+#pragma once
+
+#include "CVector2D.h"
+
+class CRect
+{
+public:
+    float left;              // x1
+    float bottom;            // y1
+    float right;             // x2
+    float top;               // y2
+
+    CRect()
+    {
+        left = 1000000.0f;
+        top = 1000000.0f;
+        right = -1000000.0f;
+        bottom = -1000000.0f;
+    }
+
+    CRect(float left, float top, float right, float bottom)
+    {
+        this->left = left;
+        this->top = top;
+        this->right = right;
+        this->bottom = bottom;
+    }
+
+    void Restrict(CRect* rect)
+    {
+        if (rect->left < left)
+            left = rect->left;
+        if (rect->right > right)
+            right = rect->right;
+        if (rect->top < top)
+            top = rect->top;
+        if (rect->bottom > bottom)
+            bottom = rect->bottom;
+    }
+
+    void StretchToPoint(float x, float y)
+    {
+        if (x < left)
+            left = x;
+        if (x > right)
+            right = x;
+        if (y < top)
+            top = y;
+        if (y > bottom)
+            bottom = y;
+    }
+
+    bool IsCircleInside(CVector2D* point, float radius)
+    {
+        return left - radius <= point->fX && radius + right >= point->fX && top - radius <= point->fY && radius + bottom >= point->fY;
+    }
+
+    void Reset()
+    {
+        left = 0.0f;
+        bottom = 0.0f;
+        right = 0.0f;
+        top = 0.0f;
+    }
+
+    // Fix #1613: custom function. Used for fixing custom map collision crashes in CPhysical class (infinite loop)
+    // The value must be greater than or equal to 1.0f because let 's say if the value of left is 15.5,
+    // when it' s casted to int, it will be rounded off to 15, so we need to subtract at least 0.6 to make make it
+    // 14 when it 's casted to int . It doesn't really matter which value we subtract because the bounding box
+    // values are already corrupted at this point, and we're just trying to avert the crash by making sure
+    // the top and left values are always less than the bottom and right values.
+    void FixIncorrectTopLeft()
+    {
+        if (left > right) left = right - 1.0f;
+        if (top > bottom) top = bottom - 1.0f;
+    }
+};

Original file line number	Diff line number	Diff line change
`@@ -175,20 +175,11 @@ VOID CRadarSA::ClearMarkerForEntity(CPed* ped)`
`175`	`175`	`}`
`176`	`176`	`}`
`177`	`177`
`178`		`-struct CRect`
`179`		`-{`
`180`		`- float fX1, fY1, fX2, fY2;`
`181`		`-};`
`182`		`-`
`183`	`178`	`VOID CRadarSA::DrawAreaOnRadar(float fX1, float fY1, float fX2, float fY2, const SColor color)`
`184`	`179`	`{`
`185`	`180`	`// Convert color to required abgr at the last moment`
`186`	`181`	`unsigned long abgr = color.A << 24 \| color.B << 16 \| color.G << 8 \| color.R;`
`187`		`- CRect myRect;`
`188`		`- myRect.fX1 = fX1;`
`189`		`- myRect.fY1 = fY1;`
`190`		`- myRect.fX2 = fX2;`
`191`		`- myRect.fY2 = fY2;`
	`182`	`+ CRect myRect(fX1, fY1, fX2, fY2);`
`192`	`183`	`DWORD dwFunc = FUNC_DrawAreaOnRadar;`
`193`	`184`	`_asm`
`194`	`185`	`{`