Fixes & improvements to electric bolt handling (#1587)

- Reverts the previous, partial or non-functional fixes for crash at
`0x4C2C19`.
- Replaces the code for handling tracking of electric bolts and their
owner technos to support multiple electric bolt instances per techno
(vanilla tracks this with single pointer in UnitClass only). This
tracking code also correctly works for units in limbo which should
prevent the aforementioned crash from occuring at all.
- This tracking is used to determine whether or not the electric bolt
updates its source coordinate based on FLH on every frame, allowing it
to follow techno's rotation, movement etc. By default this is only
enabled for vehicles just as before but can be overridden per weapon by
explicitly setting value for `Bolt.FollowFLH`. It now correctly takes
burst into accord thanks to changes mentioned in previous point.
 
Fixes #1585

Author: Starkku

CREDITS.md

@@ -256,6 +256,7 @@ This page lists all the individual contributions to the project by their author.
   - Custom exit cell for infantry factory
   - Vehicles keeping target on move command
   - `IsSonic` wave drawing crash fix
+  - Customizable electric bolt duration and electric bolt-related fixes
 - **Morton (MortonPL)**:
   - `XDrawOffset` for animations
   - Shield passthrough & absorption

docs/Fixed-or-Improved-Logics.md

@@ -194,6 +194,7 @@ This page describes all ingame logics that are fixed or improved in Phobos witho
 - Fixed issues caused by incorrect reference removal (f.ex. If the unit cloaks/enters transport, it cannot gain experience from previously launched spawners/C4/projectiles).
 - Fixed an issue that caused `IsSonic=true` wave drawing to crash the game if the wave traveled over a certain distance.
 - Buildings with foundation bigger than 1x1 can now recycle spawner correctly.
+- Electric bolts that are supposed to update their position based on units current firing coords (by default, those fired by vehicles) now do so correctly for more than one concurrent electric bolt.
 
 ## Fixes / interactions with other extensions
 
@@ -1670,6 +1671,7 @@ FireOnce.ResetSequence=true  ; boolean
 - You can now specify individual bolts you want to disable for `IsElectricBolt=true` weapons. Note that this is only a visual change.
 - By default `IsElectricBolt=true` effect draws a bolt with 8 arcs. This can now be customized per WeaponType with `Bolt.Arcs`. Value of 0 results in a straight line being drawn.
 - `Bolt.Duration` can be specified to explicitly set the overall duration of the visual electric bolt effect. Only values in range of 1 to 31 are accepted, values outside this range are clamped into it.
+- `Bolt.FollowFLH` can be used to override the default behaviour where the electric bolt source coordinates change to match the unit's firing coord on every frame (making it follow unit's movement, rotation etc). Defaults to true on vehicles, false for everything else.
 
 In `rulesmd.ini`:
 ```ini
@@ -1679,10 +1681,11 @@ Bolt.Disable2=false    ; boolean
 Bolt.Disable3=false    ; boolean
 Bolt.Arcs=8            ; integer
 Bolt.Duration=17       ; integer, game frames
+Bolt.FollowFLH=        ; boolean
 ```
 
 ```{note}
-Due to technical constraints, these features do not work with electric bolts created from support weapon of [Ares' Prism Forwarding](https://ares-developers.github.io/Ares-docs/new/buildings/prismforwarding.html).
+Due to technical constraints, these features do not work with electric bolts created from support weapon of [Ares' Prism Forwarding](https://ares-developers.github.io/Ares-docs/new/buildings/prismforwarding.html) or those from AirburstWeapon.
 ```
 
 ### Single-color lasers

docs/Whats-New.md

@@ -328,6 +328,7 @@ New:
 - No turret unit turn to the target (by CrimRecya & TaranDahl)
 - Damage multiplier for different houses (by CrimRecya)
 - Customizable duration for electric bolts (by Starkku)
+- Customizable FLH tracking for electric bolts (by Starkku)
 - Extended gattling rate down logic (by CrimRecya)
 - Sell or undeploy building on impact (by CrimRecya)
 - No rearm and reload in EMP or temporal (by CrimRecya)
@@ -350,6 +351,7 @@ Vanilla fixes:
 - Prevent the units with locomotors that cause problems from entering the tank bunker (by TaranDahl)
 - Fixed an issue that harvesters with amphibious movement zone can not automatically return to refineries with `WaterBound` on water surface (by NetsuNegi)
 - Buildings with foundation bigger than 1x1 can now recycle spawned correctly (by TaranDahl)
+- Electric bolts that are supposed to update their position based on units current firing coords (by default, those fired by vehicles) now do so correctly for more than one concurrent electric bolt (by Starkku)
 
 Fixes / interactions with other extensions:
 - Allowed `AuxBuilding` and Ares' `SW.Aux/NegBuildings` to count building upgrades (by Ollerus)

src/Ext/Bullet/Body.cpp

@@ -268,7 +268,9 @@ inline void BulletExt::SimulatedFiringElectricBolt(BulletClass* pBullet)
 	{
 		pEBolt->AlternateColor = pWeapon->IsAlternateColor;
 		//TODO Weapon's Bolt.Color1, Bolt.Color2, Bolt.Color3(Ares)
-		WeaponTypeExt::BoltWeaponMap[pEBolt] = WeaponTypeExt::ExtMap.Find(pWeapon);
+		auto& weaponStruct = WeaponTypeExt::BoltWeaponMap[pEBolt];
+		weaponStruct.Weapon = WeaponTypeExt::ExtMap.Find(pWeapon);
+		weaponStruct.BurstIndex = 0;
 		pEBolt->Fire(pBullet->SourceCoords, pBullet->TargetCoords, 0);
 	}
 }

src/Ext/Techno/Body.cpp

@@ -40,6 +40,18 @@ TechnoExt::ExtData::~ExtData()
 		auto& vec = HouseExt::ExtMap.Find(pThis->Owner)->OwnedCountedHarvesters;
 		vec.erase(std::remove(vec.begin(), vec.end(), pThis), vec.end());
 	}
+
+	this->ClearElectricBolts();
+}
+
+void TechnoExt::ExtData::ClearElectricBolts()
+{
+	for (auto const pBolt : this->ElectricBolts)
+	{
+		pBolt->Owner = nullptr;
+	}
+
+	this->ElectricBolts.clear();
 }
 
 bool TechnoExt::IsActiveIgnoreEMP(TechnoClass* pThis)
@@ -552,6 +564,7 @@ void TechnoExt::ExtData::Serialize(T& Stm)
 		.Process(this->AttachedEffects)
 		.Process(this->AE)
 		.Process(this->PreviousType)
+		.Process(this->ElectricBolts)
 		.Process(this->AnimRefCount)
 		.Process(this->ReceiveDamage)
 		.Process(this->PassengerDeletionTimer)
@@ -597,6 +610,8 @@ void TechnoExt::ExtData::SaveToStream(PhobosStreamWriter& Stm)
 {
 	Extension<TechnoClass>::SaveToStream(Stm);
 	this->Serialize(Stm);
+
+	this->ClearElectricBolts();
 }
 
 bool TechnoExt::LoadGlobals(PhobosStreamReader& Stm)

src/Ext/Techno/Body.h

@@ -29,6 +29,7 @@ class TechnoExt
 		std::vector<std::unique_ptr<AttachEffectClass>> AttachedEffects;
 		AttachEffectTechnoProperties AE;
 		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;
 		int AnimRefCount; // Used to keep track of how many times this techno is referenced in anims f.ex Invoker, ParentBuilding etc., for pointer invalidation.
 		bool ReceiveDamage;
 		bool LastKillWasTeamTarget;
@@ -75,6 +76,7 @@ class TechnoExt
 			, AttachedEffects {}
 			, AE {}
 			, PreviousType { nullptr }
+			, ElectricBolts {}
 			, AnimRefCount { 0 }
 			, ReceiveDamage { false }
 			, LastKillWasTeamTarget { false }
@@ -153,6 +155,7 @@ class TechnoExt
 	private:
 		template <typename T>
 		void Serialize(T& Stm);
+		void ClearElectricBolts();
 	};
 
 	class ExtContainer final : public Container<TechnoExt>

src/Ext/WeaponType/Body.cpp

@@ -81,6 +81,7 @@ void WeaponTypeExt::ExtData::LoadFromINIFile(CCINIClass* const pINI)
 	this->Bolt_Disable3.Read(exINI, pSection, "Bolt.Disable3");
 	this->Bolt_Arcs.Read(exINI, pSection, "Bolt.Arcs");
 	this->Bolt_Duration.Read(exINI, pSection, "Bolt.Duration");
+	this->Bolt_FollowFLH.Read(exINI, pSection, "Bolt.FollowFLH");
 
 	this->RadType.Read<true>(exINI, pSection, "RadType");
 
@@ -139,6 +140,7 @@ void WeaponTypeExt::ExtData::Serialize(T& Stm)
 		.Process(this->Bolt_Disable3)
 		.Process(this->Bolt_Arcs)
 		.Process(this->Bolt_Duration)
+		.Process(this->Bolt_FollowFLH)
 		.Process(this->Strafing)
 		.Process(this->Strafing_Shots)
 		.Process(this->Strafing_SimulateBurst)

src/Ext/WeaponType/Body.h

@@ -30,6 +30,7 @@ class WeaponTypeExt
 		Valueable<bool> Bolt_Disable3;
 		Valueable<int> Bolt_Arcs;
 		Valueable<int> Bolt_Duration;
+		Nullable<bool> Bolt_FollowFLH;
 		Nullable<bool> Strafing;
 		Nullable<int> Strafing_Shots;
 		Valueable<bool> Strafing_SimulateBurst;
@@ -82,6 +83,7 @@ class WeaponTypeExt
 			, Bolt_Disable3 { false }
 			, Bolt_Arcs { 8 }
 			, Bolt_Duration { 17 }
+			, Bolt_FollowFLH {}
 			, Strafing { }
 			, Strafing_Shots {}
 			, Strafing_SimulateBurst { false }
@@ -152,13 +154,19 @@ class WeaponTypeExt
 		~ExtContainer();
 	};
 
+	struct EBoltWeaponStruct
+	{
+		WeaponTypeExt::ExtData* Weapon;
+		int BurstIndex;
+	};
+
 	static ExtContainer ExtMap;
 
 	static bool LoadGlobals(PhobosStreamReader& Stm);
 	static bool SaveGlobals(PhobosStreamWriter& Stm);
 
 	static double OldRadius;
-	static PhobosMap<EBolt*, const WeaponTypeExt::ExtData*> BoltWeaponMap;
+	static PhobosMap<EBolt*, EBoltWeaponStruct> BoltWeaponMap;
 	static const WeaponTypeExt::ExtData* BoltWeaponType;
 
 	static void DetonateAt(WeaponTypeClass* pThis, AbstractClass* pTarget, TechnoClass* pOwner, HouseClass* pFiringHouse = nullptr);
@@ -168,4 +176,5 @@ class WeaponTypeExt
 	static int GetRangeWithModifiers(WeaponTypeClass* pThis, TechnoClass* pFirer);
 	static int GetRangeWithModifiers(WeaponTypeClass* pThis, TechnoClass* pFirer, int range);
 	static int GetTechnoKeepRange(WeaponTypeClass* pThis, TechnoClass* pFirer, bool isMinimum);
+
 };

src/Ext/WeaponType/Hook.EBolt.cpp

@@ -1,20 +1,24 @@
 #include "Body.h"
 #include <EBolt.h>
+#include <Ext/Techno/Body.h>
 #include <Utilities/Macro.h>
 #include <Helpers/Macro.h>
 
-PhobosMap<EBolt*, const WeaponTypeExt::ExtData*> WeaponTypeExt::BoltWeaponMap;
+PhobosMap<EBolt*, WeaponTypeExt::EBoltWeaponStruct> WeaponTypeExt::BoltWeaponMap;
 const WeaponTypeExt::ExtData* WeaponTypeExt::BoltWeaponType = nullptr;
 
 DEFINE_HOOK(0x6FD494, TechnoClass_FireEBolt_SetExtMap_AfterAres, 0x7)
 {
-	GET_STACK(WeaponTypeClass*, pWeapon, STACK_OFFSET(0x30, 0x8));
+	GET(TechnoClass*, pThis, EDI);
 	GET(EBolt*, pBolt, EAX);
+	GET_STACK(WeaponTypeClass*, pWeapon, STACK_OFFSET(0x30, 0x8));
 
 	if (pWeapon)
 	{
 		auto const pWeaponExt = WeaponTypeExt::ExtMap.Find(pWeapon);
-		WeaponTypeExt::BoltWeaponMap[pBolt] = pWeaponExt;
+		auto& weaponStruct = WeaponTypeExt::BoltWeaponMap[pBolt];
+		weaponStruct.Weapon = pWeaponExt;
+		weaponStruct.BurstIndex = pThis->CurrentBurstIndex;
 		pBolt->Lifetime = 1 << (Math::clamp(pWeaponExt->Bolt_Duration, 1, 31) - 1);
 	}
 
@@ -35,7 +39,7 @@ DEFINE_HOOK(0x4C20BC, EBolt_DrawArcs, 0xB)
 	enum { DoLoop = 0x4C20C7, Break = 0x4C2400 };
 
 	GET_STACK(EBolt*, pBolt, 0x40);
-	WeaponTypeExt::BoltWeaponType = WeaponTypeExt::BoltWeaponMap.get_or_default(pBolt);
+	WeaponTypeExt::BoltWeaponType = WeaponTypeExt::BoltWeaponMap.get_or_default(pBolt).Weapon;
 
 	GET_STACK(int, plotIndex, STACK_OFFSET(0x408, -0x3E0));
 
@@ -47,7 +51,7 @@ DEFINE_HOOK(0x4C20BC, EBolt_DrawArcs, 0xB)
 DEFINE_HOOK(0x4C24E4, Ebolt_DrawFist_Disable, 0x8)
 {
 	GET_STACK(EBolt*, pBolt, 0x40);
-	WeaponTypeExt::BoltWeaponType = WeaponTypeExt::BoltWeaponMap.get_or_default(pBolt);
+	WeaponTypeExt::BoltWeaponType = WeaponTypeExt::BoltWeaponMap.get_or_default(pBolt).Weapon;
 
 	return (WeaponTypeExt::BoltWeaponType && WeaponTypeExt::BoltWeaponType->Bolt_Disable1) ? 0x4C2515 : 0;
 }
@@ -61,3 +65,94 @@ DEFINE_HOOK(0x4C26EE, Ebolt_DrawThird_Disable, 0x8)
 {
 	return (WeaponTypeExt::BoltWeaponType && WeaponTypeExt::BoltWeaponType->Bolt_Disable3) ? 0x4C2710 : 0;
 }
+
+#pragma region EBoltTrackingFixes
+
+class EBoltFake final : public EBolt
+{
+	public:
+		void _SetOwner(TechnoClass* pTechno, int weaponIndex);
+		void _RemoveFromOwner();
+};
+
+void EBoltFake::_SetOwner(TechnoClass* pTechno, int weaponIndex)
+{
+	if (pTechno && pTechno->IsAlive)
+	{
+		auto const pWeapon = pTechno->GetWeapon(weaponIndex)->WeaponType;
+		auto const pWeaponExt = WeaponTypeExt::ExtMap.Find(pWeapon);
+
+		if (!pWeaponExt->Bolt_FollowFLH.Get(pTechno->WhatAmI() == AbstractType::Unit))
+			return;
+
+		this->Owner = pTechno;
+		this->WeaponSlot = weaponIndex;
+
+		auto const pExt = TechnoExt::ExtMap.Find(pTechno);
+		pExt->ElectricBolts.push_back(this);
+	}
+}
+
+void EBoltFake::_RemoveFromOwner()
+{
+	auto const pExt = TechnoExt::ExtMap.Find(this->Owner);
+	auto& vec = pExt->ElectricBolts;
+	vec.erase(std::remove(vec.begin(), vec.end(), this), vec.end());
+	this->Owner = nullptr;
+}
+
+DEFINE_FUNCTION_JUMP(LJMP, 0x4C2BD0, EBoltFake::_SetOwner); // Failsafe in case called in another module
+
+DEFINE_HOOK(0x6FD5D6, TechnoClass_InitEBolt, 0x6)
+{
+	enum { SkipGameCode = 0x6FD60B };
+
+	GET(TechnoClass*, pThis, ESI);
+	GET(EBolt*, pBolt, EAX);
+	GET(int, weaponIndex, EBX);
+
+	if (pBolt)
+		((EBoltFake*)pBolt)->_SetOwner(pThis, weaponIndex);
+
+	return SkipGameCode;
+}
+
+DEFINE_HOOK(0x4C285D, EBolt_DrawAll_BurstIndex, 0x5)
+{
+	enum { SkipGameCode = 0x4C2882 };
+
+	GET(TechnoClass*, pTechno, ECX);
+	GET_STACK(EBolt*, pThis, STACK_OFFSET(0x34, -0x24));
+
+	int burstIndex = pTechno->CurrentBurstIndex;
+	pTechno->CurrentBurstIndex = WeaponTypeExt::BoltWeaponMap[pThis].BurstIndex;
+	auto const fireCoords = pTechno->GetFLH(pThis->WeaponSlot, CoordStruct::Empty);
+	pTechno->CurrentBurstIndex = burstIndex;
+	R->EAX(&fireCoords);
+
+	return SkipGameCode;
+}
+
+DEFINE_HOOK(0x4C299F, EBolt_DrawAll_EndOfLife, 0x6)
+{
+	enum { SkipGameCode = 0x4C29B9 };
+
+	GET(EBolt*, pThis, EAX);
+
+	if (pThis->Owner)
+		((EBoltFake*)pThis)->_RemoveFromOwner();
+
+	return SkipGameCode;
+}
+
+DEFINE_HOOK(0x4C2A02, EBolt_DestroyVector, 0x6)
+{
+	enum { SkipGameCode = 0x4C2A08 };
+
+	GET(EBolt*, pThis, EAX);
+
+	((EBoltFake*)pThis)->_RemoveFromOwner();
+
+	return SkipGameCode;
+}
+#pragma endregion

src/Misc/Hooks.BugFixes.cpp

@@ -1563,10 +1563,3 @@ DEFINE_HOOK(0x75EE49, WaveClass_DrawSonic_CrashFix, 0x7)
 
 	return 0;
 }
-
-// EIP 004C2C19 crash has 2 causes: the Owner of an EBolt being invalid, and the ElectricBolt of a Unit being invalid
-// Vanilla doesn't have InvalidatePointer for EBolt, so it's made into this way to clear the pointer on EBolt
-// now we'll clear Owner for EBolt in AnnounceInvalidPointer so there won't be nullptr when EBolt trying to access an Owner
-// in this case, we can also dismiss ElectricBolt on Unit, to prevent the crash that caused by its invalidation
-DEFINE_JUMP(LJMP, 0x6FD5F2, 0x6FD5FC)
-DEFINE_JUMP(LJMP, 0x6FD600, 0x6FD606)

src/Phobos.Ext.cpp

@@ -240,13 +240,6 @@ DEFINE_HOOK(0x7258D0, AnnounceInvalidPointer, 0x6)
 
 	PhobosTypeRegistry::InvalidatePointer(pInvalid, removed);
 
-	// Fix EBolt Owner not being invalidated
-	for (auto const pBolt : EBolt::Array)
-	{
-		if (pBolt->Owner == pInvalid)
-			pBolt->Owner = nullptr;
-	}
-
 	return 0;
 }