Fix renamed entities on TurretPoint

Some units would retain an undue HeightOffset after renaming
while being on a TurretPoint.

Fixes #8651
Fixes #2004

binaries/data/mods/public/simulation/components/TurretHolder.js

@@ -74,12 +74,14 @@ class TurretHolder
 	/**
 	 * @param {number} entity - The entity to check for.
 	 * @param {Object} turretPoint - The turret point to use.
+	 * @param {boolean} [forReplacement=false] - Whether this check is for replacement
+	 *        (if true, occupied turret points are allowed).
 	 *
 	 * @return {boolean} - Whether the entity is allowed to occupy the specified turret point.
 	 */
-	AllowedToOccupyTurretPoint(entity, turretPoint)
+	AllowedToOccupyTurretPoint(entity, turretPoint, forReplacement = false)
 	{
-		if (!turretPoint || turretPoint.entity)
+		if (!turretPoint || turretPoint.entity && !forReplacement)
 			return false;
 
 		if (!IsOwnedByMutualAllyOfEntity(entity, this.entity))

binaries/data/mods/public/simulation/components/Turretable.js

@@ -18,6 +18,14 @@ Turretable.prototype.GetRange = function(type, target)
 	return cmpTurretHolder ? cmpTurretHolder.LoadingRange() : { "min": 0, "max": 1 };
 };
 
+/**
+ * @return {number} - The turret point name of the entity this entity is turreted on.
+ */
+Turretable.prototype.GetTurretPointName = function()
+{
+	return this.turretPointName || "";
+};
+
 /**
  * @return {number} - The entity ID of the entity this entity is turreted on.
  */
@@ -73,6 +81,7 @@ Turretable.prototype.OccupyTurret = function(target, turretPointName = "", eject
 
 	this.holder = target;
 	this.ejectable = ejectable;
+	this.turretPointName = turretPointName;
 
 	const cmpUnitAI = Engine.QueryInterface(this.entity, IID_UnitAI);
 	if (cmpUnitAI)

binaries/data/mods/public/simulation/components/tests/test_Foundation.js

@@ -19,6 +19,8 @@ Engine.LoadComponentScript("interfaces/TerritoryDecay.js");
 Engine.LoadComponentScript("interfaces/Trigger.js");
 Engine.LoadComponentScript("interfaces/Timer.js");
 Engine.LoadComponentScript("interfaces/UnitAI.js");
+Engine.LoadComponentScript("interfaces/Turretable.js");
+Engine.LoadComponentScript("interfaces/TurretHolder.js");
 Engine.LoadComponentScript("AutoBuildable.js");
 Engine.LoadComponentScript("Foundation.js");
 Engine.LoadComponentScript("Timer.js");
@@ -101,6 +103,12 @@ function testFoundation(...mocks)
 		"MoveOutOfWorld": () => {}
 	});
 
+	AddMock(foundationEnt, IID_Turretable, {
+		"IsTurreted": () => false,
+		"HolderID": () => 0,
+		"LeaveTurret": () => true
+	});
+
 	AddMock(previewEnt, IID_Ownership, {
 		"SetOwner": owner => { TS_ASSERT_EQUALS(owner, player); },
 	});
@@ -140,6 +148,13 @@ function testFoundation(...mocks)
 		"SetHeightOffset": () => {}
 	});
 
+	AddMock(newEnt, IID_Turretable, {
+		"IsTurreted": () => false,
+		"HolderID": () => 0,
+		"CanOccupy": (target) => true,
+		"LeaveTurret": () => true
+	});
+
 	for (const mock of mocks)
 		AddMock(...mock);
 
@@ -263,6 +278,12 @@ AddMock(foundationEnt2, IID_Health, {
 	},
 });
 
+AddMock(foundationEnt2, IID_Turretable, {
+	"IsTurreted": () => false,
+	"HolderID": () => 0,
+	"LeaveTurret": () => true
+});
+
 const cmpBuildableAuto = ConstructComponent(foundationEnt2, "AutoBuildable", {
 	"Rate": "1.0"
 });

binaries/data/mods/public/simulation/components/tests/test_Pack.js

@@ -17,6 +17,8 @@ Engine.LoadComponentScript("interfaces/StatusEffectsReceiver.js");
 Engine.LoadComponentScript("interfaces/TerritoryDecay.js");
 Engine.LoadComponentScript("interfaces/Timer.js");
 Engine.LoadComponentScript("interfaces/UnitAI.js");
+Engine.LoadComponentScript("interfaces/Turretable.js");
+Engine.LoadComponentScript("interfaces/TurretHolder.js");
 Engine.LoadComponentScript("Pack.js");
 Engine.RegisterGlobal("MT_EntityRenamed", "entityRenamed");
 
@@ -45,10 +47,23 @@ AddMock(SYSTEM_ENTITY, IID_Timer, {
 	"SetInterval": (entity, iid, funcname, time, repeattime, data) => { timerActivated = true; return 7; }
 });
 
+AddMock(ent, IID_Turretable, {
+	"IsTurreted": () => false,
+	"HolderID": () => 0,
+	"LeaveTurret": () => true
+});
+
+AddMock(newEnt, IID_Turretable, {
+	"IsTurreted": () => false,
+	"HolderID": () => 0,
+	"CanOccupy": (target) => true,
+	"LeaveTurret": () => true
+});
+
 Engine.AddEntity = function(template)
 {
 	TS_ASSERT_EQUALS(template, "finalTemplate");
-	return true;
+	return newEnt;
 };
 
 // Test Packing

binaries/data/mods/public/simulation/components/tests/test_TurretHolder.js

@@ -105,6 +105,28 @@ TS_ASSERT_EQUALS(cmpTurretHolder.AllowedToOccupyTurretPoint(infID, cmpTurretHold
 TS_ASSERT_EQUALS(cmpTurretHolder.AllowedToOccupyTurretPoint(infID, cmpTurretHolder.turretPoints[1]), false);
 TS_ASSERT_EQUALS(cmpTurretHolder.AllowedToOccupyTurretPoint(infID, cmpTurretHolder.turretPoints[2]), false);
 
+// Test forReplacement parameter - empty turret points should allow both
+TS_ASSERT_EQUALS(cmpTurretHolder.AllowedToOccupyTurretPoint(siegeEngineID, cmpTurretHolder.turretPoints[0], false), true);
+TS_ASSERT_EQUALS(cmpTurretHolder.AllowedToOccupyTurretPoint(siegeEngineID, cmpTurretHolder.turretPoints[0], true), true);
+
+// Now occupy some turret points to test replacement logic
+TS_ASSERT(cmpTurretHolder.OccupyTurretPoint(archerID, cmpTurretHolder.turretPoints[0]));
+TS_ASSERT(cmpTurretHolder.OccupyTurretPoint(cavID, cmpTurretHolder.turretPoints[2]));
+
+// Test that occupied turrets block normal occupation but allow replacement
+TS_ASSERT_EQUALS(cmpTurretHolder.AllowedToOccupyTurretPoint(siegeEngineID, cmpTurretHolder.turretPoints[0], false), false);
+TS_ASSERT_EQUALS(cmpTurretHolder.AllowedToOccupyTurretPoint(siegeEngineID, cmpTurretHolder.turretPoints[0], true), true);
+TS_ASSERT_EQUALS(cmpTurretHolder.AllowedToOccupyTurretPoint(infID, cmpTurretHolder.turretPoints[2], false), false);
+TS_ASSERT_EQUALS(cmpTurretHolder.AllowedToOccupyTurretPoint(infID, cmpTurretHolder.turretPoints[2], true), false); // Still false due to class restriction
+
+// Test that class restrictions still apply even for replacement
+TS_ASSERT_EQUALS(cmpTurretHolder.AllowedToOccupyTurretPoint(infID, cmpTurretHolder.turretPoints[1], false), false);
+TS_ASSERT_EQUALS(cmpTurretHolder.AllowedToOccupyTurretPoint(infID, cmpTurretHolder.turretPoints[1], true), false); // Still false due to class restriction
+
+// Clean up for subsequent tests
+TS_ASSERT(cmpTurretHolder.LeaveTurretPoint(archerID));
+TS_ASSERT(cmpTurretHolder.LeaveTurretPoint(cavID));
+
 // Test that one cannot leave a turret that is not occupied.
 TS_ASSERT(!cmpTurretHolder.LeaveTurretPoint(archerID));
 

binaries/data/mods/public/simulation/helpers/Transform.js

@@ -24,6 +24,27 @@ function ChangeEntityTemplate(oldEnt, newTemplate)
 	if (cmpVisual && cmpNewVisual)
 		cmpNewVisual.SetActorSeed(cmpVisual.GetActorSeed());
 
+	const cmpOldTurretable = Engine.QueryInterface(oldEnt, IID_Turretable);
+
+	// If the old entity is turreted, we need to handle it before copying position
+	if (cmpOldTurretable && cmpOldTurretable.IsTurreted())
+	{
+		const cmpNewTurretable = Engine.QueryInterface(newEnt, IID_Turretable);
+
+		// Check if new entity doesn't have Turretable component
+		if (!cmpNewTurretable)
+			cmpOldTurretable.LeaveTurret(true);
+		else
+		{
+			// Check if it's allowed to occupy the turret point
+			const cmpTurretHolderOfOldEnt = Engine.QueryInterface(cmpOldTurretable.HolderID(), IID_TurretHolder);
+
+			if (cmpTurretHolderOfNewEnt &&
+				!cmpTurretHolderOfOldEnt.AllowedToOccupyTurretPoint(newEnt, cmpOldTurretable.GetTurretPointName(), true))
+				cmpOldTurretable.LeaveTurret(true);
+		}
+	}
+
 	var cmpPosition = Engine.QueryInterface(oldEnt, IID_Position);
 	var cmpNewPosition = Engine.QueryInterface(newEnt, IID_Position);
 	if (cmpPosition && cmpNewPosition)