Improve subterranean harvester pathfinding (#1535)

This PR should allow `MovementZone=Subterannean` harvesters to access
docks that are in areas closed off from normal pathfinding such as
cliffs and islands. Whether or not it fixes all issues with such
harvesters is another thing.

Author: Starkku

CREDITS.md

@@ -280,6 +280,7 @@ This page lists all the individual contributions to the project by their author.
   - Changes / fixes to `Vertical` projectile logic and customizing projectile initial facing behavior
   - Bugfixes to map trigger action `125 Build At...`
   - Owner change during buildup bugfix
+  - Subterranean harvester pathfinding fix
 - **Morton (MortonPL)**:
   - `XDrawOffset` for animations
   - Shield passthrough & absorption

Phobos.vcxproj

@@ -40,7 +40,6 @@
     <ClCompile Include="src\Ext\Sidebar\SWSidebar\ToggleSWButtonClass.cpp" />
     <ClCompile Include="src\Ext\TechnoType\Hooks.MatrixOp.cpp" />
     <ClCompile Include="src\Ext\Techno\Hooks.Airstrike.cpp" />
-    <ClCompile Include="src\Ext\Unit\Hooks.Harvester.cpp" />
     <ClCompile Include="src\Ext\Unit\Hooks.Sinking.cpp" />
     <ClCompile Include="src\Misc\Hooks.AlphaImage.cpp" />
     <ClCompile Include="src\New\Entity\AttachEffectClass.cpp" />
@@ -91,6 +90,7 @@
     <ClCompile Include="src\Ext\TEvent\Body.cpp" />
     <ClCompile Include="src\Ext\Trigger\Hooks.cpp" />
     <ClCompile Include="src\Ext\Unit\Hooks.Crushing.cpp" />
+    <ClCompile Include="src\Ext\Unit\Hooks.Harvester.cpp" />
     <ClCompile Include="src\Locomotion\TestLocomotionClass.cpp" />
     <ClCompile Include="src\Misc\Hooks.Gamespeed.cpp" />
     <ClCompile Include="src\Misc\Hooks.Ares.cpp" />

docs/Fixed-or-Improved-Logics.md

@@ -270,6 +270,7 @@ This page describes all ingame logics that are fixed or improved in Phobos witho
 - Iron Curtain/Custom Tint Support for SHP Turreted Vehicles.
 - Reactivate unused trigger events 2, 53, and 54.
 - Fixed the bug that vehicle fall on infantry will make all cell content has been removed.
+- Fixed `MovementZone=Subterannean` harvesters being unable to find docks if in area enclosed by water, cliffs etc.
 
 ## Fixes / interactions with other extensions
 

docs/Whats-New.md

@@ -490,6 +490,7 @@ Vanilla fixes:
 - Reactivate unused trigger events 2, 53, and 54 (by FlyStar)
 - Fixed the bug that vehicle fall on infantry will make all cell content has been removed (by NetsuNegi)
 - Fixed buildings that have their owner changed during buildup skipping buildup and sometimes not correctly clearing the state (by Starkku)
+- Fixed `MovementZone=Subterannean` harvesters being unable to find docks if in area enclosed by water, cliffs etc.
 
 Phobos fixes:
 - Fixed the bug that `AllowAirstrike=no` cannot completely prevent air strikes from being launched against it (by NetsuNegi)

src/Ext/Techno/Body.Update.cpp

@@ -225,6 +225,63 @@ void TechnoExt::ExtData::DepletedAmmoActions()
 	}
 }
 
+// Subterranean harvester factory exit state machine.
+void TechnoExt::ExtData::UpdateSubterraneanHarvester()
+{
+	auto const pThis = static_cast<UnitClass*>(this->OwnerObject());
+
+	// Unnecessary for AI players.
+	if (!pThis->Owner->IsControlledByHuman())
+		return;
+
+	switch (this->SubterraneanHarvStatus)
+	{
+	case 0: // No state to handle.
+		break;
+	case 1: // Unit has been created.
+		// If we're still in the factory do not advance.
+		if (pThis->HasAnyLink())
+			break;
+
+		pThis->ClearNavigationList();
+
+		// If we have rally point available, move to it and advance to next state, otherwise end here.
+		if (this->SubterraneanHarvRallyPoint)
+		{
+			pThis->SetDestination(this->SubterraneanHarvRallyPoint, false);
+			pThis->QueueMission(Mission::Move, true);
+			this->SubterraneanHarvRallyPoint = nullptr;
+			this->SubterraneanHarvStatus = 2;
+			break;
+		}
+		else
+		{
+			this->SubterraneanHarvStatus = 0;
+		}
+
+		break;
+	case 2: // Out of factory and on move.
+		// If we're still moving don't start harvesting.
+		if (pThis->Destination || pThis->CurrentMission == Mission::Move)
+			break;
+
+		// If harvester stops moving and becomes anything except idle, reset the state machine.
+		if (pThis->CurrentMission != Mission::Guard)
+		{
+			this->SubterraneanHarvStatus = 0;
+			break;
+		}
+
+		// Go harvest ore.
+		pThis->ClearNavigationList();
+		pThis->QueueMission(Mission::Harvest, true);
+		this->SubterraneanHarvStatus = 0;
+		break;
+	default:
+		break;
+	}
+}
+
 // TODO : Merge into new AttachEffects
 bool TechnoExt::ExtData::CheckDeathConditions(bool isInLimbo)
 {

src/Ext/Techno/Body.cpp

@@ -886,6 +886,8 @@ void TechnoExt::ExtData::Serialize(T& Stm)
 		.Process(this->AE)
 		.Process(this->PreviousType)
 		.Process(this->AnimRefCount)
+		.Process(this->SubterraneanHarvStatus)
+		.Process(this->SubterraneanHarvRallyPoint)
 		.Process(this->ReceiveDamage)
 		.Process(this->LastKillWasTeamTarget)
 		.Process(this->PassengerDeletionTimer)

src/Ext/Techno/Body.h

@@ -32,6 +32,8 @@ class TechnoExt
 		TechnoTypeClass* PreviousType; // Type change registered in TechnoClass::AI on current frame and used in FootClass::AI on same frame and reset after.
 		std::vector<EBolt*> ElectricBolts; // EBolts are not serialized so do not serialize this either.
 		int AnimRefCount; // Used to keep track of how many times this techno is referenced in anims f.ex Invoker, ParentBuilding etc., for pointer invalidation.
+		int SubterraneanHarvStatus; // 0 = none, 1 = created, 2 = out from factory
+		AbstractClass* SubterraneanHarvRallyPoint;
 		bool ReceiveDamage;
 		bool LastKillWasTeamTarget;
 		CDTimerClass PassengerDeletionTimer;
@@ -110,6 +112,8 @@ class TechnoExt
 			, PreviousType { nullptr }
 			, ElectricBolts {}
 			, AnimRefCount { 0 }
+			, SubterraneanHarvStatus { 0 }
+			, SubterraneanHarvRallyPoint { nullptr }
 			, ReceiveDamage { false }
 			, LastKillWasTeamTarget { false }
 			, PassengerDeletionTimer {}
@@ -172,6 +176,7 @@ class TechnoExt
 		void ApplyInterceptor();
 		bool CheckDeathConditions(bool isInLimbo = false);
 		void DepletedAmmoActions();
+		void UpdateSubterraneanHarvester();
 		void EatPassengers();
 		void UpdateTiberiumEater();
 		void UpdateShield();

src/Ext/Techno/Hooks.Misc.cpp

@@ -1,5 +1,6 @@
 #include "Body.h"
 
+#include <EventClass.h>
 #include <SpawnManagerClass.h>
 #include <TunnelLocomotionClass.h>
 #include <JumpjetLocomotionClass.h>
@@ -846,13 +847,78 @@ DEFINE_HOOK(0x73D6E6, UnitClass_Unload_Subterranean, 0x6)
 
 	GET(UnitClass*, pThis, ESI);
 
-	if (pThis->Type->Locomotor == LocomotionClass::CLSIDs::Tunnel)
+	if (auto const pLoco = locomotion_cast<TunnelLocomotionClass*>(pThis->Locomotor))
 	{
-		auto const pLoco = static_cast<TunnelLocomotionClass*>(pThis->Locomotor.GetInterfacePtr());
-
 		if (pLoco->State != TunnelLocomotionClass::State::Idle)
 			return ReturnFromFunction;
 	}
 
 	return 0;
 }
+
+#pragma region Events
+
+DEFINE_HOOK(0x4C7512, EventClass_Execute_StopCommand, 0x6)
+{
+	GET(TechnoClass* const, pThis, ESI);
+
+	if (auto const pUnit = abstract_cast<UnitClass*>(pThis))
+	{
+		// issue #112 Make FireOnce=yes work on other TechnoType
+		// Author: Starkku
+		if (pUnit->CurrentMission == Mission::Unload && pUnit->Type->DeployFire && !pUnit->Type->IsSimpleDeployer)
+		{
+			pUnit->SetTarget(nullptr);
+			pThis->QueueMission(Mission::Guard, true);
+		}
+
+		// Explicit stop command should reset subterranean harvester state machine.
+		auto const pExt = TechnoExt::ExtMap.Find(pUnit);
+		pExt->SubterraneanHarvStatus = 0;
+		pExt->SubterraneanHarvRallyPoint = nullptr;
+	}
+
+	return 0;
+}
+
+DEFINE_HOOK(0x4C7462, EventClass_Execute_MegaMission_MoveCommand, 0x5)
+{
+	enum { SkipGameCode = 0x4C74C0 };
+
+	GET(TechnoClass*, pTechno, EDI);
+
+	if (pTechno->WhatAmI() != AbstractType::Unit)
+		return 0;
+
+	GET(EventClass*, pThis, ESI);
+	auto const mission = static_cast<Mission>(pThis->MegaMission.Mission);
+	auto const pExt = TechnoExt::ExtMap.Find(pTechno);
+
+	if (mission == Mission::Move)
+	{
+		// Explicitly reset subterranean harvester state machine.
+		pExt->SubterraneanHarvStatus = 0;
+		pExt->SubterraneanHarvRallyPoint = nullptr;
+
+		// Do not explicitly reset target for KeepTargetOnMove vehicles when issued move command.
+		if (pExt->TypeExtData->KeepTargetOnMove && pTechno->Target)
+		{
+			GET(AbstractClass*, pTarget, EBX);
+
+			if (!pTarget && pTechno->IsCloseEnoughToAttack(pTechno->Target))
+			{
+				auto const pDestination = pThis->MegaMission.Destination.As_Abstract();
+				pTechno->SetDestination(pDestination, true);
+				pExt->KeepTargetOnMove = true;
+
+				return SkipGameCode;
+			}
+		}
+	}
+
+	pExt->KeepTargetOnMove = false;
+
+	return 0;
+}
+
+#pragma endregion

src/Ext/Techno/Hooks.cpp

@@ -1,7 +1,6 @@
 #include "Body.h"
 
 #include <AircraftClass.h>
-#include <EventClass.h>
 #include <ScenarioClass.h>
 #include <TunnelLocomotionClass.h>
 #include <JumpjetLocomotionClass.h>
@@ -78,6 +77,7 @@ DEFINE_HOOK(0x736480, UnitClass_AI, 0x6)
 	auto const pExt = TechnoExt::ExtMap.Find(pThis);
 	pExt->UpdateKeepTargetOnMove();
 	pExt->DepletedAmmoActions();
+	pExt->UpdateSubterraneanHarvester();
 
 	return 0;
 }
@@ -717,39 +717,6 @@ DEFINE_HOOK(0x414665, AircraftClass_Draw_ExtraSHP, 0x6)
 	return Continue;
 }
 
-// Do not explicitly reset target for KeepTargetOnMove vehicles when issued move command.
-DEFINE_HOOK(0x4C7462, EventClass_Execute_KeepTargetOnMove, 0x5)
-{
-	enum { SkipGameCode = 0x4C74C0 };
-
-	GET(TechnoClass*, pTechno, EDI);
-
-	if (pTechno->WhatAmI() != AbstractType::Unit)
-		return 0;
-
-	GET(EventClass*, pThis, ESI);
-	auto const mission = static_cast<Mission>(pThis->MegaMission.Mission);
-	auto const pExt = TechnoExt::ExtMap.Find(pTechno);
-
-	if (mission == Mission::Move && pExt->TypeExtData->KeepTargetOnMove && pTechno->Target)
-	{
-		GET(AbstractClass*, pTarget, EBX);
-
-		if (!pTarget && pTechno->IsCloseEnoughToAttack(pTechno->Target))
-		{
-			auto const pDestination = pThis->MegaMission.Destination.As_Abstract();
-			pTechno->SetDestination(pDestination, true);
-			pExt->KeepTargetOnMove = true;
-
-			return SkipGameCode;
-		}
-	}
-
-	pExt->KeepTargetOnMove = false;
-
-	return 0;
-}
-
 #pragma region BuildingTypeSelectable
 
 namespace BuildingTypeSelectable

src/Ext/Unit/Hooks.DeployFire.cpp

@@ -3,23 +3,7 @@
 #include <Ext/Techno/Body.h>
 #include <Utilities/Macro.h>
 
-// issue #112 Make FireOnce=yes work on other TechnoTypes
-// Author: Starkku
-DEFINE_HOOK(0x4C7512, EventClass_Execute_StopUnitDeployFire, 0x6)
-{
-	GET(TechnoClass* const, pThis, ESI);
-
-	auto const pUnit = abstract_cast<UnitClass*>(pThis);
-	if (pUnit && pUnit->CurrentMission == Mission::Unload && pUnit->Type->DeployFire && !pUnit->Type->IsSimpleDeployer)
-	{
-		pUnit->SetTarget(nullptr);
-		pThis->QueueMission(Mission::Guard, true);
-	}
-
-	return 0;
-}
-
-DEFINE_HOOK(0x4C77E4, EventClass_Execute_UnitDeployFire, 0x6)
+DEFINE_HOOK(0x4C77E4, EventClass_Execute_DeployCommand, 0x6)
 {
 	enum { DoNotExecute = 0x4C8109 };