[Minor] Fix desync caused by units with DeploysInto (#1525)

Game removes deploying vehicles from map temporarily to check if there's
enough space to deploy into a building when displaying allow/disallow
deploy cursor. This can cause desyncs if there are certain types of
units around the deploying unit because the OccupationFlags may be
accidentally cleared, or the order of the objects linked list may be
scrambled.

About implementation:
This is a hook separated from #1479 .
This solution completely abandons the method of removing and then
adding, and solves the problem by directly excluding specific units.

Other related repairs:
Fix unit's self-destruct when using trigger 107 to teleport the MCV.
Fix reconnection error when moving MCV with Teleport locomotion.

Author: CrimRecya

CREDITS.md

@@ -434,6 +434,8 @@ This page lists all the individual contributions to the project by their author.
   - Fix an issue that units on the slope tilted at an excessive angle
   - Fix an issue that the first passenger who call the transport ship no longer board the transport ship when the land units call for boarding
   - Fix an issue that impassable invisible barrier generated by the behavior of infantry continuously entering vehicles
+  - Fix an issue that MCV will self-destruct when using trigger 107 to teleport
+  - Fix an issue that moving MCV with Teleport locomotion will cause reconnection error
 - **Ollerus**:
   - Build limit group enhancement
   - Customizable rocker amplitude

docs/Fixed-or-Improved-Logics.md

@@ -213,6 +213,8 @@ This page describes all ingame logics that are fixed or improved in Phobos witho
 - Fixed an issue that units on the slope tilted at an excessive angle.
 - Fixed an issue that impassable invisible barrier generated by the behavior of infantry continuously entering vehicles.
 - Fixed an issue that teleport units board transport vehicles on the bridge will create an impassable invisible barrier, which may cause the game to freeze or even crash.
+- Fixed an issue that MCV will self-destruct when using trigger 107 to teleport
+- Fixed an issue that moving MCV with Teleport locomotion will cause reconnection error
 
 ## Fixes / interactions with other extensions
 

docs/Whats-New.md

@@ -371,6 +371,10 @@ Vanilla fixes:
 - Fixed an issue that the first passenger who call the transport ship no longer board the transport ship when the land units call for boarding (by CrimRecya)
 - Fixed an issue that impassable invisible barrier generated by the behavior of infantry continuously entering vehicles (by CrimRecya)
 - Fixed an issue that teleport units board transport vehicles on the bridge will create an impassable invisible barrier, which may cause the game to freeze or even crash (by NetsuNegi)
+- Fixed an issue that moving MCV with Teleport locomotion will cause reconnection error (by CrimRecya)
+
+Phobos fixes:
+- Fixed an issue that MCV will self-destruct when using trigger 107 to teleport (by CrimRecya)
 
 Fixes / interactions with other extensions:
 - Allowed `AuxBuilding` and Ares' `SW.Aux/NegBuildings` to count building upgrades (by Ollerus)

src/Ext/Techno/Body.cpp

@@ -12,6 +12,7 @@
 #include <Utilities/AresFunctions.h>
 
 TechnoExt::ExtContainer TechnoExt::ExtMap;
+UnitClass* TechnoExt::Deployer = nullptr;
 
 TechnoExt::ExtData::~ExtData()
 {
@@ -400,21 +401,16 @@ bool TechnoExt::CanDeployIntoBuilding(UnitClass* pThis, bool noDeploysIntoDefaul
 	if (!pDeployType)
 		return noDeploysIntoDefaultValue;
 
-	bool canDeploy = true;
 	auto mapCoords = CellClass::Coord2Cell(pThis->GetCoords());
 
 	if (pDeployType->GetFoundationWidth() > 2 || pDeployType->GetFoundationHeight(false) > 2)
 		mapCoords += CellStruct { -1, -1 };
 
-	pThis->Mark(MarkType::Up);
-
-	pThis->Locomotor->Mark_All_Occupation_Bits(MarkType::Up);
-
-	if (!pDeployType->CanCreateHere(mapCoords, pThis->Owner))
-		canDeploy = false;
-
-	pThis->Locomotor->Mark_All_Occupation_Bits(MarkType::Down);
-	pThis->Mark(MarkType::Down);
+	// The vanilla game used an inappropriate approach here, resulting in potential risk of desync.
+	// Now, through additional checks, we can directly exclude the unit who want to deploy.
+	TechnoExt::Deployer = pThis;
+	const bool canDeploy = pDeployType->CanCreateHere(mapCoords, pThis->Owner);
+	TechnoExt::Deployer = nullptr;
 
 	return canDeploy;
 }

src/Ext/Techno/Body.h

@@ -51,7 +51,6 @@ class TechnoExt
 		bool LastRearmWasFullDelay;
 		bool CanCloakDuringRearm; // Current rearm timer was started by DecloakToFire=no weapon.
 		int WHAnimRemainingCreationInterval;
-		bool CanCurrentlyDeployIntoBuilding; // Only set on UnitClass technos with DeploysInto set in multiplayer games, recalculated once per frame so no need to serialize.
 		WeaponTypeClass* LastWeaponType;
 		CellClass* FiringObstacleCell; // Set on firing if there is an obstacle cell between target and techno, used for updating WaveClass target etc.
 		bool IsDetachingForCloak; // Used for checking animation detaching, set to true before calling Detach_All() on techno when this anim is attached to and to false after when cloaking only.
@@ -98,7 +97,6 @@ class TechnoExt
 			, LastRearmWasFullDelay { false }
 			, CanCloakDuringRearm { false }
 			, WHAnimRemainingCreationInterval { 0 }
-			, CanCurrentlyDeployIntoBuilding { false }
 			, LastWeaponType {}
 			, FiringObstacleCell {}
 			, IsDetachingForCloak { false }
@@ -167,6 +165,8 @@ class TechnoExt
 
 	static ExtContainer ExtMap;
 
+	static UnitClass* Deployer;
+
 	static bool LoadGlobals(PhobosStreamReader& Stm);
 	static bool SaveGlobals(PhobosStreamWriter& Stm);
 

src/Ext/TerrainType/Hooks.Passable.cpp

@@ -93,75 +93,9 @@ DEFINE_HOOK(0x73FB71, UnitClass_CanEnterCell_PassableTerrain, 0x6)
 }
 
 // Buildable-upon TerrainTypes Hook #1 - Allow placing buildings on top of them.
-DEFINE_HOOK_AGAIN(0x47C80E, CellClass_IsClearTo_Build_BuildableTerrain, 0x5)
-DEFINE_HOOK(0x47C745, CellClass_IsClearTo_Build_BuildableTerrain, 0x5)
-{
-	enum { Skip = 0x47C85F, SkipFlags = 0x47C6A0 };
-
-	GET(CellClass*, pThis, EDI);
-
-	auto pTerrain = pThis->GetTerrain(false);
-
-	if (pTerrain)
-	{
-		if (auto const pTypeExt = TerrainTypeExt::ExtMap.Find(pTerrain->Type))
-		{
-			if (pTypeExt->CanBeBuiltOn)
-			{
-				if (IS_CELL_OCCUPIED(pThis))
-					return Skip;
-				else
-					return SkipFlags;
-			}
-		}
-	}
-
-	return 0;
-}
-
-// Buildable-upon TerrainTypes Hook #2 - Allow placing laser fences on top of them.
-DEFINE_HOOK(0x47C657, CellClass_IsClearTo_Build_BuildableTerrain_LF, 0x6)
-{
-	enum { Skip = 0x47C6A0, Return = 0x47C6D1 };
-
-	GET(CellClass*, pThis, EDI);
-
-	auto pObj = pThis->FirstObject;
-
-	if (pObj)
-	{
-		bool isEligible = true;
-
-		while (true)
-		{
-			isEligible = pObj->WhatAmI() != AbstractType::Building;
-
-			if (auto const pTerrain = abstract_cast<TerrainClass*>(pObj))
-			{
-				isEligible = false;
-				auto const pTypeExt = TerrainTypeExt::ExtMap.Find(pTerrain->Type);
-
-				if (pTypeExt->CanBeBuiltOn)
-					isEligible = true;
-			}
-
-			if (!isEligible)
-				break;
-
-			pObj = pObj->NextObject;
-
-			if (!pObj)
-				return Skip;
-		}
-
-		return Return;
-	}
-
-	return Skip;
-}
-
+// DEFINE_HOOK(0x73FEC1, UnitClass_WhatAction_DeploysIntoDesyncFix, 0x6) in Hooks.DeploysInto.cpp
 
-// Buildable-upon TerrainTypes Hook #3 - Draw laser fence placement even if they are on the way.
+// Buildable-upon TerrainTypes Hook #2 - Draw laser fence placement even if they are on the way.
 DEFINE_HOOK(0x6D57C1, TacticalClass_DrawLaserFencePlacement_BuildableTerrain, 0x9)
 {
 	enum { ContinueChecks = 0x6D57D2, DontDraw = 0x6D59A6 };
@@ -181,7 +115,7 @@ DEFINE_HOOK(0x6D57C1, TacticalClass_DrawLaserFencePlacement_BuildableTerrain, 0x
 	return ContinueChecks;
 }
 
-// Buildable-upon TerrainTypes Hook #4 - Remove them when buildings are placed on them.
+// Buildable-upon TerrainTypes Hook #3 - Remove them when buildings are placed on them.
 DEFINE_HOOK(0x5684B1, MapClass_PlaceDown_BuildableTerrain, 0x6)
 {
 	GET(ObjectClass*, pObject, EDI);

src/Ext/Unit/Hooks.DeploysInto.cpp

@@ -1,3 +1,7 @@
+#include <TerrainClass.h>
+#include <IsometricTileTypeClass.h>
+
+#include <Ext/TerrainType/Body.h>
 #include <Ext/CaptureManager/Body.h>
 #include <Ext/WarheadType/Body.h>
 
@@ -120,38 +124,128 @@ DEFINE_HOOK(0x7396AD, UnitClass_Deploy_CreateBuilding, 0x6)
 
 // Game removes deploying vehicles from map temporarily to check if there's enough
 // space to deploy into a building when displaying allow/disallow deploy cursor.
-// This can cause desyncs if there are certain types of units around the deploying unit.
-// Only reasonable way to solve this is to perform the cell clear check on every client per frame
-// and use that result in cursor display which is client-specific. This is now implemented in multiplayer games only.
+// This can cause desyncs if there are certain types of units around the deploying
+// unit because the OccupationFlags may be accidentally cleared, or the order of
+// the objects linked list may be scrambled.
 #pragma region DeploysIntoDesyncFix
 
-DEFINE_HOOK(0x73635B, UnitClass_AI_DeploysIntoDesyncFix, 0x6)
+DEFINE_HOOK(0x73FEC1, UnitClass_WhatAction_DeploysIntoDesyncFix, 0x6)
 {
-	if (!SessionClass::IsMultiplayer())
-		return 0;
+	enum { SkipGameCode = 0x73FFDF };
 
-	GET(UnitClass*, pThis, ESI);
+	GET(UnitClass* const, pThis, ESI);
+	REF_STACK(Action, action, STACK_OFFSET(0x20, 0x8));
 
-	if (pThis->Type->DeploysInto)
-		TechnoExt::ExtMap.Find(pThis)->CanCurrentlyDeployIntoBuilding = TechnoExt::CanDeployIntoBuilding(pThis);
+	if (!TechnoExt::CanDeployIntoBuilding(pThis))
+		action = Action::NoDeploy;
 
-	return 0;
+	return SkipGameCode;
 }
 
-DEFINE_HOOK(0x73FEC1, UnitClass_WhatAction_DeploysIntoDesyncFix, 0x6)
+// Exclude the specific unit who want to deploy
+// Allow placing buildings on top of TerrainType with CanBeBuiltOn
+DEFINE_HOOK(0x47C640, CellClass_CanThisExistHere_IgnoreSomething, 0x6)
 {
-	if (!SessionClass::IsMultiplayer())
-		return 0;
+	enum { CanNotExistHere = 0x47C6D1, CanExistHere = 0x47C6A0 };
 
-	enum { SkipGameCode = 0x73FFDF };
+	GET(const CellClass* const, pCell, EDI);
+	GET(const BuildingTypeClass* const, pBuildingType, EAX);
+	GET_STACK(HouseClass* const, pOwner, STACK_OFFSET(0x18, 0xC));
 
-	GET(UnitClass*, pThis, ESI);
-	LEA_STACK(Action*, pAction, STACK_OFFSET(0x20, 0x8));
+	if (!Game::IsActive)
+		return CanExistHere;
 
-	if (!TechnoExt::ExtMap.Find(pThis)->CanCurrentlyDeployIntoBuilding)
-		*pAction = Action::NoDeploy;
+	if (pBuildingType->LaserFence)
+	{
+		for (auto pObject = pCell->FirstObject; pObject; pObject = pObject->NextObject)
+		{
+			if (pObject->WhatAmI() == AbstractType::Building)
+			{
+				return CanNotExistHere;
+			}
+			else if (const auto pTerrain = abstract_cast<TerrainClass*>(pObject))
+			{
+				const auto pTypeExt = TerrainTypeExt::ExtMap.Find(pTerrain->Type);
 
-	return SkipGameCode;
+				if (!pTypeExt || !pTypeExt->CanBeBuiltOn)
+					return CanNotExistHere;
+			}
+		}
+	}
+	else if (pBuildingType->LaserFencePost || pBuildingType->Gate)
+	{
+		bool skipFlag = TechnoExt::Deployer ? TechnoExt::Deployer->CurrentMapCoords == pCell->MapCoords : false;
+
+		for (auto pObject = pCell->FirstObject; pObject; pObject = pObject->NextObject)
+		{
+			if (const auto pTerrain = abstract_cast<TerrainClass*>(pObject))
+			{
+				const auto pTypeExt = TerrainTypeExt::ExtMap.Find(pTerrain->Type);
+
+				if (!pTypeExt || !pTypeExt->CanBeBuiltOn)
+					return CanNotExistHere;
+			}
+			else if (pObject->AbstractFlags & AbstractFlags::Techno)
+			{
+				if (pObject == TechnoExt::Deployer)
+				{
+					skipFlag = true;
+				}
+				else if (pObject->WhatAmI() != AbstractType::Building
+					|| pOwner != static_cast<BuildingClass*>(pObject)->Owner
+					|| !static_cast<BuildingClass*>(pObject)->Type->LaserFence)
+				{
+					return CanNotExistHere;
+				}
+			}
+		}
+
+		if (pCell->OccupationFlags & (skipFlag ? 0x1F : 0x3F))
+			return CanNotExistHere;
+	}
+	else if (pBuildingType->ToTile)
+	{
+		const auto isoTileTypeIndex = pCell->IsoTileTypeIndex;
+
+		if (isoTileTypeIndex >= 0 && isoTileTypeIndex < IsometricTileTypeClass::Array.Count
+			&& !IsometricTileTypeClass::Array.Items[isoTileTypeIndex]->Morphable)
+		{
+			return CanNotExistHere;
+		}
+
+		for (auto pObject = pCell->FirstObject; pObject; pObject = pObject->NextObject)
+		{
+			if (pObject->WhatAmI() == AbstractType::Building)
+				return CanNotExistHere;
+		}
+	}
+	else
+	{
+		bool skipFlag = TechnoExt::Deployer ? TechnoExt::Deployer->CurrentMapCoords == pCell->MapCoords : false;
+
+		for (auto pObject = pCell->FirstObject; pObject; pObject = pObject->NextObject)
+		{
+			if (pObject->AbstractFlags & AbstractFlags::Techno)
+			{
+				if (pObject == TechnoExt::Deployer)
+					skipFlag = true;
+				else
+					return CanNotExistHere;
+			}
+			else if (const auto pTerrain = abstract_cast<TerrainClass*>(pObject))
+			{
+				const auto pTypeExt = TerrainTypeExt::ExtMap.Find(pTerrain->Type);
+
+				if (!pTypeExt || !pTypeExt->CanBeBuiltOn)
+					return CanNotExistHere;
+			}
+		}
+
+		if (pCell->OccupationFlags & (skipFlag ? 0x1F : 0x3F))
+			return CanNotExistHere;
+	}
+
+	return CanExistHere; // Continue check the overlays .etc
 }
 
 #pragma endregion