[Minor] Fix create building map trigger action (#1848)

Fixes an issue with map trigger action 125 causing buildups to play
incorrectly sometimes. A separate issue remains where any building that
is mid-buildup will interrupt buildup if it changes owner but this is
not exclusive to buildings created through the trigger action.

Also re-enabled use fo the third parameter but this time it turns
buildups off even if otherwise available if set to != 0.

Author: Starkku

CREDITS.md

@@ -277,6 +277,7 @@ This page lists all the individual contributions to the project by their author.
   - Restored parabombs
   - Delayed fire weapons
   - Changes / fixes to `Vertical` projectile logic and customizing projectile initial facing behavior
+  - Bugfixes to map trigger action `125 Create Building At`
 - **Morton (MortonPL)**:
   - `XDrawOffset` for animations
   - Shield passthrough & absorption

docs/Whats-New.md

@@ -10,6 +10,7 @@ You can use the migration utility (can be found on [Phobos supplementaries repo]
 
 ### From vanilla
 
+- Map trigger action `125 Build At...` now plays buildup by default if available, this can be toggled off using the third parameter (values other than 0). See [required changes for `fadata.ini`](#for-map-editor-final-alert-2) on how to enable the parameter in map editor. 
 - `IsSimpleDeployer` units now obey deploying facing constraint even without deploying animation. To disable this, set `DeployDir` (defaults to `[AudioVisual] -> DeployDir`) to -1.
 - `Vertical=true` projectiles now default to completely downwards initial trajectory/facing regardless of if their projectile image has `Voxel=true` or not. This behavior can be reverted by setting `VerticalInitialFacing=false` on projectile in `rulesmd.ini`.
 - `Vertical=true` projectiles no longer move horizontally if fired by aircraft by default. To re-enable this behaviour set `Vertical.AircraftFix=false` on the projectile.
@@ -129,6 +130,7 @@ HideLightFlashEffects=false      ; boolean
   102=Horizontal position,0
   103=Vertical position,0
   104=Banner ID,0
+  105=No buildup,0
 
   [EventsRA2]
   500=Local variable is greater than,48,6,0,0,[LONG DESC],0,1,500,1
@@ -176,7 +178,7 @@ HideLightFlashEffects=false      ; boolean
 
   [ActionsRA2]
   41=Play animation at a waypoint...,0,25,69,0,0,0,1,0,0,[LONG DESC].,0,1,41
-  125=Build at...,-10,47,0,65,0,0,1,0,0,[LONG DESC],0,1,125
+  125=Build at...,-10,47,105,65,0,0,1,0,0,[LONG DESC],0,1,125
   500=Save game,-4,13,0,0,0,0,0,0,0,[LONG DESC],0,1,500,1
   501=Edit variable,0,56,55,6,54,0,0,0,0,[LONG DESC],0,1,501,1
   502=Generate random number,0,56,57,58,54,0,0,0,0,[LONG DESC],0,1,502,1
@@ -478,6 +480,7 @@ Phobos fixes:
 - Fixed `AmbientDamage.Warhead` not working for waves (by Starkku)
 - Fixed `SkirmishUnlimitedColors` not being checked if Phobos runs without Ares active (by Starkku)
 - Fixed number of `*.ApplyFirepowerMult` options (f.ex anim damage, crit) ignoring veterancy firepower modifier (by Starkku)
+- Fixed map trigger action `125 Build At...` not always playing buildups correctly (by Starkku)
 
 Fixes / interactions with other extensions:
 - Allowed `AuxBuilding` and Ares' `SW.Aux/NegBuildings` to count building upgrades (by Ollerus)

src/Ext/TAction/Hooks.cpp

@@ -28,50 +28,50 @@ DEFINE_HOOK(0x6DD8B0, TActionClass_Execute, 0x6)
 	return handled ? 0x6DD910 : 0;
 }
 
-// TODO: Sometimes Buildup anims plays while the building image is already there in faster gamespeed.
 // Bugfix: TAction 125 Build At could neither display the buildups nor be AI-repairable in singleplayer mode
+// Sep 9, 2025 - Starkku: Fixed issues with buildups potentially ending up in infinite loops etc.
+// A separate issue remains where buildup sequence will interrupt if building's house changes mid-buildup,
+// but this applies to all buildings and not just ones created through the trigger.
+// Also restored Param3 to control the buildup display, only this time it is inverted (set to >0 to disable buildups).
 DEFINE_HOOK(0x6E427D, TActionClass_CreateBuildingAt, 0x9)
 {
 	GET(TActionClass*, pThis, ESI);
-	GET(BuildingTypeClass*, pBldType, ECX);
+	GET(BuildingTypeClass*, pBuildingType, ECX);
 	GET(HouseClass*, pHouse, EDI);
 	REF_STACK(CoordStruct, coord, STACK_OFFSET(0x24, -0x18));
 
-	const bool bPlayBuildUp = pBldType->LoadBuildup();
-	//Param3 can be used for other purposes in the future
-	bool bCreated = false;
-	if (auto pBld = static_cast<BuildingClass*>(pBldType->CreateObject(pHouse)))
+	const bool playBuildup = pThis->Param3 == 0 && pBuildingType->LoadBuildup();
+	bool created = false;
+
+	if (auto pBuilding = static_cast<BuildingClass*>(pBuildingType->CreateObject(pHouse)))
 	{
-		if (bPlayBuildUp)
-		{
-			pBld->BeginMode(BStateType::Construction);
-			pBld->QueueMission(Mission::Construction, false);
-		}
-		else
-		{
-			pBld->BeginMode(BStateType::Idle);
-			pBld->QueueMission(Mission::Guard, false);
-		}
+		// Set before unlimbo cause otherwise it will call BuildingClass::Place.
+		pBuilding->QueueMission(Mission::Construction, false);
+		pBuilding->NextMission();
 
-		if (!pBld->ForceCreate(coord))
+		if (!pBuilding->ForceCreate(coord))
 		{
-			pBld->UnInit();
+			pBuilding->UnInit();
 		}
 		else
 		{
-			if (!bPlayBuildUp)
-				pBld->Place(false);
-
-			pBld->IsReadyToCommence = true;
+			// Reset mission and build state if we're not going to play buildup afterwards.
+			if (!playBuildup)
+			{
+				pBuilding->BeginMode(BStateType::Idle);
+				pBuilding->QueueMission(Mission::Guard, false);
+				pBuilding->NextMission();
+				pBuilding->Place(false); // Manually call this now.
+			}
 
 			if (SessionClass::IsCampaign() && !pHouse->IsControlledByHuman())
-				pBld->ShouldRebuild = pThis->Param4 > 0;
+				pBuilding->ShouldRebuild = pThis->Param4 > 0;
 
-			bCreated = true;
+			created = true;
 		}
 	}
 
-	R->AL(bCreated);
+	R->AL(created);
 	return 0x6E42C1;
 }