Allow players to set rally points on allied buildings

Instead of storing a single flat list of positions and data per
building, RallyPoint now stores them keyed by player ID. This lets
mutual allies independently set and display rally points on each
other's structures.

The GUI now allows selecting allied buildings with a rally point
and only shows the viewing player's own rally point data.
GuiInterface gets an OnUpdate handler to keep displayed positions
in sync when rally point targets move.

GetRallyPointCommands now takes raw position and data arrays instead
of a component reference. The network command field is also renamed
from "entities" to "structures".

Fixes #3115

binaries/data/mods/public/gui/credits/texts/programming.json

@@ -199,6 +199,7 @@
 				{ "nick": "mattlott", "name": "Matt Lott" },
 				{ "nick": "maveric", "name": "Anton Protko" },
 				{ "nick": "mbusy", "name": "Maxime Busy" },
+				{ "nick": "mehmedarslan", "name": "Mehmed Faheim Arslan/Sai Kaushik" },
 				{ "nick": "Mentula" },
 				{ "nick": "Micnasty", "name": "Travis Gorkin" },
 				{ "name": "Mikołaj \"Bajter\" Korcz" },

binaries/data/mods/public/gui/session/input.js

@@ -263,7 +263,7 @@ function determineAction(x, y, fromMiniMap)
 	if (!entState)
 		return undefined;
 
-	if (!selection.every(ownsEntity) &&
+	if (!selection.every(ent => ownsEntity(ent) || isMutualAllyBuilding(ent)) &&
 	    !(g_SimState.players[g_ViewedPlayer] &&
 	      g_SimState.players[g_ViewedPlayer].controlsAll))
 		return undefined;
@@ -318,6 +318,13 @@ function ownsEntity(ent)
 	return entState && entState.player == g_ViewedPlayer;
 }
 
+function isMutualAllyBuilding(ent)
+{
+	const entState = GetEntityState(ent);
+	return entState && entState.rallyPoint &&
+		g_Players[entState.player]?.isMutualAlly[g_ViewedPlayer];
+}
+
 function isAttackMovePressed()
 {
 	return Engine.HotkeyIsPressed("session.attackmove") ||

binaries/data/mods/public/gui/session/session.js

@@ -331,6 +331,8 @@ async function init(initData, hotloadData)
 		resumeGame();
 	});
 
+	g_DiplomacyColors.updateDisplayedPlayerColors();
+
 	const promise = Promise.race([new Promise((_, reject) =>
 	{
 		if (g_IsNetworked)

binaries/data/mods/public/gui/session/unit_actions.js

@@ -1125,7 +1125,7 @@ var g_UnitActions =
 
 				Engine.PostNetworkCommand({
 					"type": "set-rallypoint",
-					"entities": selection,
+					"structures": selection,
 					"x": position.x,
 					"z": position.z,
 					"data": action.data,
@@ -1355,7 +1355,7 @@ var g_UnitActions =
 			{
 				Engine.PostNetworkCommand({
 					"type": "unset-rallypoint",
-					"entities": selection
+					"structures": selection
 				});
 
 				// Remove displayed rally point

binaries/data/mods/public/simulation/ai/common-api/entity.js

@@ -996,13 +996,13 @@ export const Entity = Class({
 	"setRallyPoint": function(target, command)
 	{
 		const data = { "command": command, "target": target.id() };
-		Engine.PostCommand(PlayerID, { "type": "set-rallypoint", "entities": [this.id()], "x": target.position()[0], "z": target.position()[1], "data": data });
+		Engine.PostCommand(PlayerID, { "type": "set-rallypoint", "structures": [this.id()], "x": target.position()[0], "z": target.position()[1], "data": data });
 		return this;
 	},
 
 	"unsetRallyPoint": function()
 	{
-		Engine.PostCommand(PlayerID, { "type": "unset-rallypoint", "entities": [this.id()] });
+		Engine.PostCommand(PlayerID, { "type": "unset-rallypoint", "structures": [this.id()] });
 		return this;
 	},
 

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

@@ -397,7 +397,7 @@ GuiInterface.prototype.GetEntityState = function(player, ent)
 
 	const cmpRallyPoint = Engine.QueryInterface(ent, IID_RallyPoint);
 	if (cmpRallyPoint)
-		ret.rallyPoint = { "position": cmpRallyPoint.GetPositions()[0] }; // undefined or {x,z} object
+		ret.rallyPoint = { "position": cmpRallyPoint.GetPositions(player)[0] }; // undefined or {x,z} object
 
 	const cmpGarrisonHolder = Engine.QueryInterface(ent, IID_GarrisonHolder);
 	if (cmpGarrisonHolder)
@@ -1035,6 +1035,16 @@ GuiInterface.prototype.GetNonGaiaEntities = function()
 	return Engine.QueryInterface(SYSTEM_ENTITY, IID_RangeManager).GetNonGaiaEntities();
 };
 
+/**
+ * @param {number} entity - The entityID to verify.
+ * @param {number} player - The playerID to check against.
+ * @return {boolean}.
+ */
+function IsOwnedByPlayerOrMutualAlly(entity, player)
+{
+	return IsOwnedByPlayer(player, entity) || IsOwnedByMutualAllyOfPlayer(player, entity);
+}
+
 /**
  * Displays the rally points of a given list of entities (carried in cmd.entities).
  *
@@ -1046,14 +1056,12 @@ GuiInterface.prototype.GetNonGaiaEntities = function()
  */
 GuiInterface.prototype.DisplayRallyPoint = function(player, cmd)
 {
-	const cmpPlayer = QueryPlayerIDInterface(player);
-
 	// If there are some rally points already displayed, first hide them.
-	for (const ent of this.entsRallyPointsDisplayed)
+	for (const { ent } of this.entsRallyPointsDisplayed)
 	{
 		const cmpRallyPointRenderer = Engine.QueryInterface(ent, IID_RallyPointRenderer);
 		if (cmpRallyPointRenderer)
-			cmpRallyPointRenderer.SetDisplayed(false);
+			cmpRallyPointRenderer.Reset();
 	}
 
 	this.entsRallyPointsDisplayed = [];
@@ -1071,11 +1079,8 @@ GuiInterface.prototype.DisplayRallyPoint = function(player, cmd)
 		if (!cmpRallyPoint)
 			continue;
 
-		// Verify the owner.
+		if (!IsOwnedByPlayerOrMutualAlly(ent, player))
-		const cmpOwnership = Engine.QueryInterface(ent, IID_Ownership);
+			continue;
-		if (!(cmpPlayer && cmpPlayer.CanControlAllUnits()))
-			if (!cmpOwnership || cmpOwnership.GetOwner() != player)
-				continue;
 
 		// If the command was passed an explicit position, use that and
 		// override the real rally point position; otherwise use the real position.
@@ -1083,8 +1088,8 @@ GuiInterface.prototype.DisplayRallyPoint = function(player, cmd)
 		if (cmd.x && cmd.z)
 			pos = cmd;
 		else
-			// May return undefined if no rally point is set.
+			// may return undefined if no rally point is set.
-			pos = cmpRallyPoint.GetPositions()[0];
+			pos = cmpRallyPoint.GetPositions(player)[0];
 
 		if (pos)
 		{
@@ -1093,23 +1098,52 @@ GuiInterface.prototype.DisplayRallyPoint = function(player, cmd)
 			if ("queued" in cmd)
 			{
 				if (cmd.queued == true)
+				{
+					// check by re adding all existing positions before appending the new queued one.
+					const existingPositions = cmpRallyPoint.GetPositions(player);
+					for (const posi of existingPositions)
+						cmpRallyPointRenderer.AddPosition(new Vector2D(posi.x, posi.z));
 					cmpRallyPointRenderer.AddPosition(new Vector2D(pos.x, pos.z));
+				}
 				else
 					cmpRallyPointRenderer.SetPosition(new Vector2D(pos.x, pos.z));
 			}
 			else if (!cmpRallyPointRenderer.IsSet())
+			{
 				// Rebuild the renderer when not set (when reading saved game or in case of building update).
-				for (const posi of cmpRallyPoint.GetPositions())
+				const positions = cmpRallyPoint.GetPositions(player);
+				for (const posi of positions)
 					cmpRallyPointRenderer.AddPosition(new Vector2D(posi.x, posi.z));
+			}
 
 			cmpRallyPointRenderer.SetDisplayed(true);
 
 			// Remember which entities have their rally points displayed so we can hide them again.
-			this.entsRallyPointsDisplayed.push(ent);
+			this.entsRallyPointsDisplayed.push({ ent, player });
 		}
 	}
 };
 
+GuiInterface.prototype.OnUpdate = function()
+{
+	for (const { ent, player } of this.entsRallyPointsDisplayed)
+	{
+		const cmpRallyPointRenderer = Engine.QueryInterface(ent, IID_RallyPointRenderer);
+		if (!cmpRallyPointRenderer)
+			continue;
+
+		const cmpRallyPoint = Engine.QueryInterface(ent, IID_RallyPoint);
+		if (!cmpRallyPoint)
+			continue;
+
+		const positions = cmpRallyPoint.GetPositions(player);
+
+		// Update renderer positions so the path follows moving targets.
+		for (let i = 0; i < positions.length; i++)
+			cmpRallyPointRenderer.UpdatePosition(i, new Vector2D(positions[i].x, positions[i].z));
+	}
+};
+
 GuiInterface.prototype.AddTargetMarker = function(player, cmd)
 {
 	const ent = Engine.AddLocalEntity(cmd.template);

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

@@ -5,67 +5,68 @@ RallyPoint.prototype.Schema =
 
 RallyPoint.prototype.Init = function()
 {
-	this.pos = [];
+	this.perPlayer = {};
-	this.data = [];
 };
 
-RallyPoint.prototype.AddPosition = function(x, z)
+RallyPoint.prototype.GetOwner = function()
 {
-	this.pos.push({
+	return Engine.QueryInterface(this.entity, IID_Ownership)?.GetOwner();
-		"x": x,
-		"z": z
-	});
 };
 
-RallyPoint.prototype.HasPositions = function()
+RallyPoint.prototype.AddPosition = function(x, z, player = this.GetOwner())
 {
-	return this.pos.length > 0;
+	if (!this.perPlayer[player])
+		this.perPlayer[player] = { "pos": [], "data": [] };
+	this.perPlayer[player].pos.push({ "x": x, "z": z });
+};
+
+RallyPoint.prototype.HasPositions = function(player = this.GetOwner())
+{
+	return !!this.perPlayer[player]?.pos.length;
 };
 
 RallyPoint.prototype.GetFirstPosition = function()
 {
-	return this.pos.length ? Vector2D.from3D(this.pos[0]) : new Vector2D(-1, -1);
+	const pos = this.perPlayer[this.GetOwner()]?.pos;
+	return pos?.length ? Vector2D.from3D(pos[0]) : new Vector2D(-1, -1);
 };
 
-RallyPoint.prototype.GetPositions = function()
+RallyPoint.prototype.GetPositions = function(player = this.GetOwner())
 {
-	// Update positions for moving target entities
+	const playerEntry = this.perPlayer[player];
+	if (!playerEntry)
+		return [];
 
-	var cmpRangeManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_RangeManager);
+	const cmpRangeManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_RangeManager);
-	var cmpOwnership = Engine.QueryInterface(this.entity, IID_Ownership);
 
-	// We must not affect the simulation state here (modifications of the
+	// We must not affect the simulation state here, so copy the state
-	// RallyPointRenderer are allowed though), so copy the state
+	const ret = [];
-	var ret = [];
+	for (let i = 0; i < playerEntry.pos.length; i++)
-	for (var i = 0; i < this.pos.length; i++)
 	{
-		ret.push(this.pos[i]);
+		ret.push(playerEntry.pos[i]);
 
 		// Update the rallypoint coordinates if the target is alive
-		if (!this.data[i] || !this.data[i].target || !this.TargetIsAlive(this.data[i].target))
+		if (!playerEntry.data[i]?.target || !this.TargetIsAlive(playerEntry.data[i].target))
 			continue;
 
-		// and visible
+		// and visible to the player who set this rally point
-		if (cmpRangeManager && cmpOwnership &&
+		if (cmpRangeManager &&
-				cmpRangeManager.GetLosVisibility(this.data[i].target, cmpOwnership.GetOwner()) != "visible")
+				cmpRangeManager.GetLosVisibility(playerEntry.data[i].target, player) != "visible")
 			continue;
 
 		// Get the actual position of the target entity
-		var cmpPosition = Engine.QueryInterface(this.data[i].target, IID_Position);
+		const cmpPosition = Engine.QueryInterface(playerEntry.data[i].target, IID_Position);
-		if (!cmpPosition || !cmpPosition.IsInWorld())
+		if (!cmpPosition?.IsInWorld())
 			continue;
 
-		var targetPosition = cmpPosition.GetPosition2D();
+		const targetPosition = cmpPosition.GetPosition2D();
 		if (!targetPosition)
 			continue;
 
-		if (this.pos[i].x == targetPosition.x && this.pos[i].z == targetPosition.y)
+		if (playerEntry.pos[i].x == targetPosition.x && playerEntry.pos[i].z == targetPosition.y)
 			continue;
 
 		ret[i] = { "x": targetPosition.x, "z": targetPosition.y };
-		var cmpRallyPointRenderer = Engine.QueryInterface(this.entity, IID_RallyPointRenderer);
-		if (cmpRallyPointRenderer)
-			cmpRallyPointRenderer.UpdatePosition(i, targetPosition);
 	}
 
 	return ret;
@@ -73,31 +74,24 @@ RallyPoint.prototype.GetPositions = function()
 
 // Extra data for the rally point, should have a command property and then helpful data for that command
 // See getActionInfo in gui/input.js
-RallyPoint.prototype.AddData = function(data)
+RallyPoint.prototype.AddData = function(data, player = this.GetOwner())
 {
-	this.data.push(data);
+	if (!this.perPlayer[player])
+		this.perPlayer[player] = { "pos": [], "data": [] };
+	this.perPlayer[player].data.push(data);
 };
 
 // Returns an array with the data associated with this rally point.  Each element has the structure:
 // {"type": "walk/gather/garrison/...", "target": targetEntityId, "resourceType": "tree/fruit/ore/..."} where target
 // and resourceType (specific resource type) are optional, also target may be an invalid entity, check for existence.
-RallyPoint.prototype.GetData = function()
+RallyPoint.prototype.GetData = function(player = this.GetOwner())
 {
-	return this.data;
+	return this.perPlayer[player]?.data ?? [];
 };
 
-RallyPoint.prototype.Unset = function()
+RallyPoint.prototype.Unset = function(player = this.GetOwner())
 {
-	this.pos = [];
+	delete this.perPlayer[player];
-	this.data = [];
-};
-
-RallyPoint.prototype.Reset = function()
-{
-	this.Unset();
-	var cmpRallyPointRenderer = Engine.QueryInterface(this.entity, IID_RallyPointRenderer);
-	if (cmpRallyPointRenderer)
-		cmpRallyPointRenderer.Reset();
 };
 
 /**
@@ -107,50 +101,49 @@ RallyPoint.prototype.Reset = function()
  */
 RallyPoint.prototype.OrderToRallyPoint = function(entity, ignore = [])
 {
-	const cmpOwnership = Engine.QueryInterface(this.entity, IID_Ownership);
-	if (!cmpOwnership)
-		return;
-	const owner = cmpOwnership.GetOwner();
-
 	const cmpEntOwnership = Engine.QueryInterface(entity, IID_Ownership);
-	if (!cmpEntOwnership || cmpEntOwnership.GetOwner() != owner)
+	if (!cmpEntOwnership)
+		return;
+	const entOwner = cmpEntOwnership.GetOwner();
+
+	if (!this.HasPositions(entOwner))
 		return;
 
-	const commands = GetRallyPointCommands(this, [entity]);
+	const playerEntry = this.perPlayer[entOwner];
+	const commands = GetRallyPointCommands(playerEntry.pos, playerEntry.data, [entity]);
 	if (!commands.length ||
 		commands[0].target == this.entity && ignore.includes(commands[0].type))
 		return;
 
 	for (const command of commands)
-		ProcessCommand(owner, command);
+		ProcessCommand(entOwner, command);
 };
 
 RallyPoint.prototype.OnGlobalEntityRenamed = function(msg)
 {
-	for (const data of this.data)
+	for (const playerEntry of Object.values(this.perPlayer))
-	{
+		for (const data of playerEntry.data)
-		if (!data)
+		{
-			continue;
+			if (data?.target == msg.entity)
-		if (data.target && data.target == msg.entity)
+				data.target = msg.newentity;
-			data.target = msg.newentity;
+			if (data?.source == msg.entity)
-		if (data.source && data.source == msg.entity)
+				data.source = msg.newentity;
-			data.source = msg.newentity;
+		}
-	}
 
 	if (msg.entity != this.entity)
 		return;
 
 	const cmpRallyPointNew = Engine.QueryInterface(msg.newentity, IID_RallyPoint);
 	if (cmpRallyPointNew)
-	{
+		for (const player in this.perPlayer)
-		const rallyCoords = this.GetPositions();
-		const rallyData = this.GetData();
-		for (let i = 0; i < rallyCoords.length; ++i)
 		{
-			cmpRallyPointNew.AddPosition(rallyCoords[i].x, rallyCoords[i].z);
+			const playerEntry = this.perPlayer[player];
-			cmpRallyPointNew.AddData(rallyData[i]);
+			for (let i = 0; i < playerEntry.pos.length; ++i)
+			{
+				cmpRallyPointNew.AddPosition(playerEntry.pos[i].x, playerEntry.pos[i].z, +player);
+				cmpRallyPointNew.AddData(playerEntry.data[i], +player);
+			}
 		}
-	}
 };
 
 RallyPoint.prototype.OnOwnershipChanged = function(msg)
@@ -159,7 +152,7 @@ RallyPoint.prototype.OnOwnershipChanged = function(msg)
 	if (msg.from == INVALID_PLAYER || msg.to == INVALID_PLAYER)
 		return;
 
-	this.Reset();
+	this.perPlayer = {};
 };
 
 /**

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

@@ -229,7 +229,7 @@ Trainer.prototype.Item.prototype.Spawn = function()
 	const cmpRallyPoint = Engine.QueryInterface(this.trainer, IID_RallyPoint);
 	if (cmpRallyPoint)
 	{
-		const data = cmpRallyPoint.GetData()[0];
+		const data = cmpRallyPoint.GetData(this.player)[0];
 		if (data?.target && data.target == this.trainer && data.command == "garrison")
 			autoGarrison = true;
 	}
@@ -294,7 +294,7 @@ Trainer.prototype.Item.prototype.Spawn = function()
 	}
 
 	if (spawnedEnts.length && cmpRallyPoint)
-		for (const com of GetRallyPointCommands(cmpRallyPoint, spawnedEnts))
+		for (const com of GetRallyPointCommands(cmpRallyPoint.GetPositions(this.player), cmpRallyPoint.GetData(this.player), spawnedEnts))
 		{
 			// Tag this command as coming from a rally point
 			com.fromRallyPoint = true;

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

@@ -8,6 +8,7 @@ function initialRallyPointTest(test_function)
 	ResetState();
 
 	const entityID = 123;
+	AddMock(entityID, IID_Ownership, { "GetOwner": () => 1 });
 	const cmpRallyPoint = ConstructComponent(entityID, "RallyPoint", {});
 
 	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetData(), []);
@@ -50,12 +51,6 @@ initialRallyPointTest((cmpRallyPoint) =>
 	return true;
 });
 
-initialRallyPointTest((cmpRallyPoint) =>
-{
-	cmpRallyPoint.Reset();
-	return true;
-});
-
 // Construction
 initialRallyPointTest((cmpRallyPoint) =>
 {
@@ -83,3 +78,135 @@ initialRallyPointTest((cmpRallyPoint) =>
 	cmpRallyPoint.OnOwnershipChanged({ "from": 2, "to": 0 });
 	return true;
 });
+
+// Per-player rally point tests
+{
+	ResetState();
+	const entityID = 123;
+	let ownerPlayer = 1;
+	AddMock(entityID, IID_Ownership, { "GetOwner": () => ownerPlayer });
+	const cmpRallyPoint = ConstructComponent(entityID, "RallyPoint", {});
+	const player2 = 2;
+	const player3 = 3;
+
+	// Initially no per-player positions
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetPositions(player2), []);
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetData(player2), []);
+	TS_ASSERT(!cmpRallyPoint.HasPositions(player2));
+
+	// Add per-player rally point for player 2
+	cmpRallyPoint.AddPosition(10, 20, player2);
+	cmpRallyPoint.AddData({ "command": "walk" }, player2);
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetPositions(player2), [{ "x": 10, "z": 20 }]);
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetData(player2), [{ "command": "walk" }]);
+	TS_ASSERT(cmpRallyPoint.HasPositions(player2));
+
+	// Add a second waypoint for player 2
+	cmpRallyPoint.AddPosition(30, 40, player2);
+	cmpRallyPoint.AddData({ "command": "garrison" }, player2);
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetPositions(player2),
+		[{ "x": 10, "z": 20 }, { "x": 30, "z": 40 }]);
+
+	// Player 3 is unaffected
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetPositions(player3), []);
+	TS_ASSERT(!cmpRallyPoint.HasPositions(player3));
+
+	// Add per-player rally point for player 3
+	cmpRallyPoint.AddPosition(50, 60, player3);
+	cmpRallyPoint.AddData({ "command": "walk" }, player3);
+	TS_ASSERT(cmpRallyPoint.HasPositions(player3));
+
+	// Unset clears player 2 positions and data
+	cmpRallyPoint.Unset(player2);
+	TS_ASSERT(!cmpRallyPoint.HasPositions(player2));
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetPositions(player2), []);
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetData(player2), []);
+	// Player 3 is unaffected
+	TS_ASSERT(cmpRallyPoint.HasPositions(player3));
+
+	// Unset removes player 3 entry
+	cmpRallyPoint.Unset(player3);
+	TS_ASSERT(!cmpRallyPoint.HasPositions(player3));
+
+	// Per-player data is cleared on ownership change
+	cmpRallyPoint.AddPosition(10, 20, player2);
+	cmpRallyPoint.AddData({ "command": "walk" }, player2);
+	TS_ASSERT(cmpRallyPoint.HasPositions(player2));
+	cmpRallyPoint.OnOwnershipChanged({ "from": 1, "to": 2 });
+	ownerPlayer = 2;
+	TS_ASSERT(!cmpRallyPoint.HasPositions(player2));
+
+	// The owner's rally point entry does not affect allied players' entries
+	cmpRallyPoint.AddPosition(100, 200);
+	cmpRallyPoint.AddData({ "command": "walk" });
+	cmpRallyPoint.AddPosition(300, 400, player3);
+	cmpRallyPoint.AddData({ "command": "walk" }, player3);
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetPositions(), [{ "x": 100, "z": 200 }]);
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetPositions(player3), [{ "x": 300, "z": 400 }]);
+	// Unset does not affect per-player data
+	cmpRallyPoint.Unset();
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetPositions(), []);
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetPositions(player3), [{ "x": 300, "z": 400 }]);
+}
+
+// Ownership change construction/destruction preserves per-player data
+{
+	ResetState();
+	const entityID = 123;
+	const cmpRallyPoint = ConstructComponent(entityID, "RallyPoint", {});
+	const player2 = 2;
+
+	cmpRallyPoint.AddPosition(10, 20, player2);
+	cmpRallyPoint.AddData({ "command": "walk" }, player2);
+
+	// Construction: from INVALID_PLAYER should not clear per-player data
+	cmpRallyPoint.OnOwnershipChanged({ "from": INVALID_PLAYER, "to": 1 });
+	TS_ASSERT(cmpRallyPoint.HasPositions(player2));
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetPositions(player2), [{ "x": 10, "z": 20 }]);
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetData(player2), [{ "command": "walk" }]);
+
+	// Destruction: to INVALID_PLAYER should not clear per-player data
+	cmpRallyPoint.OnOwnershipChanged({ "from": 1, "to": INVALID_PLAYER });
+	TS_ASSERT(cmpRallyPoint.HasPositions(player2));
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPoint.GetPositions(player2), [{ "x": 10, "z": 20 }]);
+}
+
+// OnGlobalEntityRenamed migrates per-player rally point data to the new entity
+{
+	ResetState();
+	const oldEntityID = 123;
+	const newEntityID = 456;
+	const player2 = 2;
+	const player3 = 3;
+
+	AddMock(oldEntityID, IID_Ownership, { "GetOwner": () => 1 });
+	AddMock(newEntityID, IID_Ownership, { "GetOwner": () => 1 });
+	const cmpRallyPointOld = ConstructComponent(oldEntityID, "RallyPoint", {});
+	const cmpRallyPointNew = ConstructComponent(newEntityID, "RallyPoint", {});
+
+	cmpRallyPointOld.AddPosition(100, 200);
+	cmpRallyPointOld.AddData({ "command": "walk" });
+	cmpRallyPointOld.AddPosition(10, 20, player2);
+	cmpRallyPointOld.AddData({ "command": "walk" }, player2);
+	cmpRallyPointOld.AddPosition(30, 40, player3);
+	cmpRallyPointOld.AddData({ "command": "garrison" }, player3);
+
+	cmpRallyPointOld.OnGlobalEntityRenamed({ "entity": oldEntityID, "newentity": newEntityID });
+
+	// New entity receives owner and per-player rally point data
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPointNew.GetPositions(), [{ "x": 100, "z": 200 }]);
+	TS_ASSERT(cmpRallyPointNew.HasPositions(player2));
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPointNew.GetPositions(player2), [{ "x": 10, "z": 20 }]);
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPointNew.GetData(player2), [{ "command": "walk" }]);
+	TS_ASSERT(cmpRallyPointNew.HasPositions(player3));
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPointNew.GetPositions(player3), [{ "x": 30, "z": 40 }]);
+	TS_ASSERT_UNEVAL_EQUALS(cmpRallyPointNew.GetData(player3), [{ "command": "garrison" }]);
+
+	// Rename for an unrelated entity does not migrate to new entity
+	ResetState();
+	const cmpRP1 = ConstructComponent(oldEntityID, "RallyPoint", {});
+	const cmpRP2 = ConstructComponent(newEntityID, "RallyPoint", {});
+	cmpRP1.AddPosition(10, 20, player2);
+	cmpRP1.OnGlobalEntityRenamed({ "entity": 999, "newentity": newEntityID });
+	TS_ASSERT(!cmpRP2.HasPositions(player2));
+}

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

@@ -434,27 +434,31 @@ var g_Commands = {
 
 	"set-rallypoint": function(player, cmd, data)
 	{
-		for (const ent of data.entities)
+		const structures = FilterEntityListWithAllies(cmd.structures || [], player, data.controlAllUnits);
+		for (const structure of structures)
 		{
-			var cmpRallyPoint = Engine.QueryInterface(ent, IID_RallyPoint);
+			const cmpRallyPoint = Engine.QueryInterface(structure, IID_RallyPoint);
-			if (cmpRallyPoint)
+			if (!cmpRallyPoint)
-			{
+				continue;
-				if (!cmd.queued)
-					cmpRallyPoint.Unset();
 
-				cmpRallyPoint.AddPosition(cmd.x, cmd.z);
+			if (!cmd.queued)
-				cmpRallyPoint.AddData(clone(cmd.data));
+				cmpRallyPoint.Unset(player);
-			}
+
+			cmpRallyPoint.AddPosition(cmd.x, cmd.z, player);
+			cmpRallyPoint.AddData(clone(cmd.data), player);
 		}
 	},
 
 	"unset-rallypoint": function(player, cmd, data)
 	{
-		for (const ent of data.entities)
+		const structures = FilterEntityListWithAllies(cmd.structures || [], player, data.controlAllUnits);
+		for (const structure of structures)
 		{
-			var cmpRallyPoint = Engine.QueryInterface(ent, IID_RallyPoint);
+			const cmpRallyPoint = Engine.QueryInterface(structure, IID_RallyPoint);
-			if (cmpRallyPoint)
+			if (!cmpRallyPoint)
-				cmpRallyPoint.Reset();
+				continue;
+
+			cmpRallyPoint.Unset(player);
 		}
 	},
 

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

@@ -1,18 +1,16 @@
 // Returns an array of commands suitable for ProcessCommand() based on the rally point data.
 // This assumes that the rally point has a valid position.
-function GetRallyPointCommands(cmpRallyPoint, spawnedEnts)
+function GetRallyPointCommands(rallyPos, data, spawnedEnts)
 {
-	const data = cmpRallyPoint.GetData();
-	const rallyPos = cmpRallyPoint.GetPositions();
 	const ret = [];
 	for (let i = 0; i < rallyPos.length; ++i)
 	{
 		// Look and see if there is a command in the rally point data, otherwise just walk there.
-		let command = data[i] && data[i].command ? data[i].command : "walk";
+		let command = data[i]?.command ?? "walk";
 
 		// If a target was set and the target no longer exists, or no longer
 		// has a valid position, then just walk to the rally point.
-		if (data[i] && data[i].target)
+		if (data[i]?.target)
 		{
 			const cmpPosition = Engine.QueryInterface(data[i].target, IID_Position);
 			if (!cmpPosition || !cmpPosition.IsInWorld())