Allow specific garrison points to receive only specific units, for instance catapults on ships, or having both visible garrison points for fortresses and garrisonning elephants.

Reviewed by: @Freagarach,
Comments by: @elexis, @Angen
Fixes #3488

This was SVN commit r23630.

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

@@ -1,4 +1,4 @@
-//Number of rounds of firing per 2 seconds
+// Number of rounds of firing per 2 seconds.
 const roundCount = 10;
 const attackType = "Ranged";
 
@@ -36,18 +36,26 @@ BuildingAI.prototype.OnGarrisonedUnitsChanged = function(msg)
 
 	for (let ent of msg.added)
 	{
+		if (msg.visible[ent])
+			continue;
+
 		let cmpIdentity = Engine.QueryInterface(ent, IID_Identity);
 		if (!cmpIdentity)
 			continue;
+
 		if (MatchesClassList(cmpIdentity.GetClassesList(), classes))
 			++this.archersGarrisoned;
 	}
 
 	for (let ent of msg.removed)
 	{
+		if (msg.visible[ent])
+			continue;
+
 		let cmpIdentity = Engine.QueryInterface(ent, IID_Identity);
 		if (!cmpIdentity)
 			continue;
+
 		if (MatchesClassList(cmpIdentity.GetClassesList(), classes))
 			--this.archersGarrisoned;
 	}

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

@@ -1,10 +1,10 @@
 function GarrisonHolder() {}
 
 GarrisonHolder.prototype.Schema =
-	"<element name='Max' a:help='Maximum number of entities which can be garrisoned inside this holder'>" +
+	"<element name='Max' a:help='Maximum number of entities which can be garrisoned in this holder'>" +
 		"<data type='positiveInteger'/>" +
 	"</element>" +
-	"<element name='List' a:help='Classes of entities which are allowed to garrison inside this holder (from Identity)'>" +
+	"<element name='List' a:help='Classes of entities which are allowed to garrison in this holder (from Identity)'>" +
 		"<attribute name='datatype'>" +
 			"<value>tokens</value>" +
 		"</attribute>" +
@@ -45,6 +45,14 @@ GarrisonHolder.prototype.Schema =
 						"<element name='Z'>" +
 							"<data type='decimal'/>" +
 						"</element>" +
+						"<optional>" +
+							"<element name='AllowedClasses' a:help='If specified, only entities matching the given classes will be able to use this visible garrison point.'>" +
+								"<attribute name='datatype'>" +
+									"<value>tokens</value>" +
+								"</attribute>" +
+								"<text/>" +
+							"</element>" +
+						"</optional>"+
 						"<optional>" +
 							"<element name='Angle' a:help='Angle in degrees relative to the garrisonHolder direction'>" +
 								"<data type='decimal'/>" +
@@ -68,20 +76,21 @@ GarrisonHolder.prototype.Init = function()
 	this.timer = undefined;
 	this.allowGarrisoning = new Map();
 	this.visibleGarrisonPoints = [];
-	if (this.template.VisibleGarrisonPoints)
+	if (!this.template.VisibleGarrisonPoints)
-	{
+		return;
-		let points = this.template.VisibleGarrisonPoints;
+
-		for (let point in points)
+	let points = this.template.VisibleGarrisonPoints;
-			this.visibleGarrisonPoints.push({
+	for (let point in points)
-				"offset": {
+		this.visibleGarrisonPoints.push({
-					"x": +points[point].X,
+			"offset": {
-					"y": +points[point].Y,
+				"x": +points[point].X,
-					"z": +points[point].Z
+				"y": +points[point].Y,
-				},
+				"z": +points[point].Z
-				"angle": points[point].Angle ? +points[point].Angle * Math.PI / 180 : null,
+			},
-				"entity": null
+			"allowedClasses": points[point].AllowedClasses,
-			});
+			"angle": points[point].Angle ? +points[point].Angle * Math.PI / 180 : null,
-	}
+			"entity": null
+		});
 };
 
 /**
@@ -178,11 +187,28 @@ GarrisonHolder.prototype.IsAllowedToGarrison = function(ent)
 	return MatchesClassList(entityClasses, this.template.List._string) && !!Engine.QueryInterface(ent, IID_Garrisonable);
 };
 
+/**
+ * @param {number} entity - The entity's id.
+ * @param {Object|undefined} visibleGarrisonPoint - The vgp object.
+ * @return {boolean} - Whether the unit is allowed be visible on that garrison point.
+ */
+GarrisonHolder.prototype.AllowedToVisibleGarrisoning = function(entity, visibleGarrisonPoint)
+{
+	if (!visibleGarrisonPoint)
+		return false;
+
+	if (!visibleGarrisonPoint.allowedClasses)
+		return true;
+
+	let cmpIdentity = Engine.QueryInterface(entity, IID_Identity);
+	return cmpIdentity && MatchesClassList(cmpIdentity.GetClassesList(), visibleGarrisonPoint.allowedClasses._string);
+};
+
 /**
  * Garrison a unit inside. The timer for AutoHeal is started here.
  * @param {number} vgpEntity - The visual garrison point that will be used.
  * If vgpEntity is given, this visualGarrisonPoint will be used for the entity.
- * @return {boolean} Whether the entity was garrisonned.
+ * @return {boolean} - Whether the entity was garrisoned.
  */
 GarrisonHolder.prototype.Garrison = function(entity, vgpEntity)
 {
@@ -193,16 +219,14 @@ GarrisonHolder.prototype.Garrison = function(entity, vgpEntity)
 	if (!this.PerformGarrison(entity))
 		return false;
 
-	let visibleGarrisonPoint = vgpEntity;
+	let visibleGarrisonPoint;
-	if (!visibleGarrisonPoint)
+	if (vgpEntity && this.AllowedToVisibleGarrisoning(entity, vgpEntity))
-		for (let vgp of this.visibleGarrisonPoints)
+		visibleGarrisonPoint = vgpEntity;
-		{
-			if (vgp.entity)
-				continue;
-			visibleGarrisonPoint = vgp;
-			break;
-		}
 
+	if (!visibleGarrisonPoint)
+		visibleGarrisonPoint = this.visibleGarrisonPoints.find(vgp => !vgp.entity && this.AllowedToVisibleGarrisoning(entity, vgp));
+
+	let isVisiblyGarrisoned = false;
 	if (visibleGarrisonPoint)
 	{
 		visibleGarrisonPoint.entity = entity;
@@ -224,10 +248,21 @@ GarrisonHolder.prototype.Garrison = function(entity, vgpEntity)
 		let cmpUnitAI = Engine.QueryInterface(entity, IID_UnitAI);
 		if (cmpUnitAI)
 			cmpUnitAI.SetTurretStance();
+
+		isVisiblyGarrisoned = true;
 	}
 	else
 		cmpPosition.MoveOutOfWorld();
 
+	// Should only be called after the garrison has been performed else the visible Garrison Points are not updated yet.
+	Engine.PostMessage(this.entity, MT_GarrisonedUnitsChanged, {
+		"added": [entity],
+		"removed": [],
+		"visible": {
+			[entity]: isVisiblyGarrisoned,
+		}
+	});
+
 	return true;
 };
 
@@ -268,7 +303,6 @@ GarrisonHolder.prototype.PerformGarrison = function(entity)
 	if (cmpAura && cmpAura.HasGarrisonAura())
 		cmpAura.ApplyGarrisonAura(this.entity);
 
-	Engine.PostMessage(this.entity, MT_GarrisonedUnitsChanged, { "added": [entity], "removed": [] });
 	return true;
 };
 
@@ -313,6 +347,11 @@ GarrisonHolder.prototype.Eject = function(entity, forced)
 	let cmpEntPosition = Engine.QueryInterface(entity, IID_Position);
 	let cmpEntUnitAI = Engine.QueryInterface(entity, IID_UnitAI);
 
+	// Needs to be set before the visible garrison points are cleared.
+	let visible = {
+		[entity]: this.IsVisiblyGarrisoned(entity)
+	};
+
 	for (let vgp of this.visibleGarrisonPoints)
 	{
 		if (vgp.entity != entity)
@@ -345,7 +384,11 @@ GarrisonHolder.prototype.Eject = function(entity, forced)
 	if (cmpPosition)
 		cmpEntPosition.SetYRotation(cmpPosition.GetPosition().horizAngleTo(pos));
 
-	Engine.PostMessage(this.entity, MT_GarrisonedUnitsChanged, { "added": [], "removed": [entity] });
+	Engine.PostMessage(this.entity, MT_GarrisonedUnitsChanged, {
+		"added": [],
+		"removed": [entity],
+		"visible": visible
+	});
 
 	return true;
 };
@@ -581,7 +624,13 @@ GarrisonHolder.prototype.OnGlobalOwnershipChanged = function(msg)
 		if (cmpHealth && cmpHealth.GetHitpoints() == 0)
 		{
 			this.entities.splice(entityIndex, 1);
-			Engine.PostMessage(this.entity, MT_GarrisonedUnitsChanged, { "added": [], "removed": [msg.entity] });
+			Engine.PostMessage(this.entity, MT_GarrisonedUnitsChanged, {
+				"added": [],
+				"removed": [msg.entity],
+				"visible": {
+					[msg.entity]: this.IsVisiblyGarrisoned(msg.entity)
+				}
+			});
 			this.UpdateGarrisonFlag();
 
 			for (let point of this.visibleGarrisonPoints)
@@ -670,10 +719,29 @@ GarrisonHolder.prototype.EjectOrKill = function(entities)
 	}
 
 	if (killedEntities.length)
-		Engine.PostMessage(this.entity, MT_GarrisonedUnitsChanged, { "added": [], "removed": killedEntities });
+	{
+		let visibleEntitiesIds = {};
+		for (let ent of killedEntities)
+			visibleEntitiesIds[ent] = this.IsVisiblyGarrisoned(ent);
+		Engine.PostMessage(this.entity, MT_GarrisonedUnitsChanged, {
+			"added": [],
+			"removed": killedEntities,
+			"visible": visibleEntitiesIds
+		});
+	}
 	this.UpdateGarrisonFlag();
 };
 
+/**
+ * Gives insight about the unit type of garrisoning.
+ * @param {number} entity - The entity's id.
+ * @return {boolean} - Whether the entity is visible on the garrison holder.
+ */
+GarrisonHolder.prototype.IsVisiblyGarrisoned = function(entity)
+{
+	return this.visibleGarrisonPoints.some(point => point.entity == entity);
+};
+
 /**
  * Whether an entity is ejectable.
  * @param {number} entity - The entity-ID to be tested.

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

@@ -33,8 +33,8 @@ AddMock(garrisonHolderId, IID_Health, {
 });
 
 AddMock(player, IID_Player, {
-	"IsAlly": id => true,
+	"IsAlly": id => id != enemyPlayer,
-	"IsMutualAlly": id => true,
+	"IsMutualAlly": id => id != enemyPlayer,
 	"GetPlayerID": () => player
 });
 
@@ -55,7 +55,7 @@ AddMock(SYSTEM_ENTITY, IID_PlayerManager, {
 for (let i = 24; i <= 34; ++i)
 {
 	AddMock(i, IID_Identity, {
-		"GetClassesList": () => "Infantry+Cavalry",
+		"GetClassesList": () => ["Infantry", "Cavalry"],
 		"GetSelectionGroupName": () => "mace_infantry_archer_a"
 	});
 
@@ -88,7 +88,7 @@ for (let i = 24; i <= 34; ++i)
 }
 
 AddMock(33, IID_Identity, {
-	"GetClassesList": () => "Infantry+Cavalry",
+	"GetClassesList": () => ["Infantry", "Cavalry"],
 	"GetSelectionGroupName": () => "spart_infantry_archer_a"
 });
 
@@ -168,3 +168,127 @@ TS_ASSERT_EQUALS(cmpGarrisonHolder.GetGarrisonedEntitiesCount(), 3);
 TS_ASSERT_EQUALS(cmpGarrisonHolder.IsFull(), false);
 TS_ASSERT_EQUALS(cmpGarrisonHolder.UnloadAll(), true);
 TS_ASSERT_UNEVAL_EQUALS(cmpGarrisonHolder.GetEntities(), []);
+
+let siegeEngineId = 44;
+AddMock(siegeEngineId, IID_Identity, {
+	"GetClassesList": () => ["Siege"]
+});
+let archerId = 45;
+AddMock(archerId, IID_Identity, {
+	"GetClassesList": () => ["Infantry", "Ranged"]
+});
+
+// Test visible garrisoning restrictions.
+cmpGarrisonHolder = ConstructComponent(garrisonHolderId, "GarrisonHolder", {
+	"Max": 10,
+	"List": { "_string": "Infantry+Ranged Siege Cavalry" },
+	"EjectHealth": 0.1,
+	"EjectClassesOnDestroy": { "_string": "Infantry" },
+	"BuffHeal": 1,
+	"LoadingRange": 2.1,
+	"Pickup": false,
+	"VisibleGarrisonPoints": {
+		"archer1": {
+			"X": 12,
+			"Y": 5,
+			"Z": 6
+		},
+		"archer2": {
+			"X": 15,
+			"Y": 5,
+			"Z": 6,
+			"AllowedClasses": { "_string": "Siege Trader" }
+		},
+		"archer3": {
+			"X": 15,
+			"Y": 5,
+			"Z": 6,
+			"AllowedClasses": { "_string": "Siege Infantry+Ranged Infantry+Cavalry" }
+		}
+	}
+});
+
+AddMock(32, IID_Identity, {
+	"GetClassesList": () => ["Trader"]
+});
+
+TS_ASSERT_EQUALS(cmpGarrisonHolder.Garrison(32), false);
+TS_ASSERT_EQUALS(cmpGarrisonHolder.AllowedToVisibleGarrisoning(siegeEngineId, cmpGarrisonHolder.visibleGarrisonPoints[0]), true);
+TS_ASSERT_EQUALS(cmpGarrisonHolder.AllowedToVisibleGarrisoning(siegeEngineId, cmpGarrisonHolder.visibleGarrisonPoints[1]), true);
+TS_ASSERT_EQUALS(cmpGarrisonHolder.AllowedToVisibleGarrisoning(siegeEngineId, cmpGarrisonHolder.visibleGarrisonPoints[2]), true);
+TS_ASSERT_EQUALS(cmpGarrisonHolder.AllowedToVisibleGarrisoning(archerId, cmpGarrisonHolder.visibleGarrisonPoints[0]), true);
+TS_ASSERT_EQUALS(cmpGarrisonHolder.AllowedToVisibleGarrisoning(archerId, cmpGarrisonHolder.visibleGarrisonPoints[1]), false);
+TS_ASSERT_EQUALS(cmpGarrisonHolder.AllowedToVisibleGarrisoning(archerId, cmpGarrisonHolder.visibleGarrisonPoints[2]), true);
+TS_ASSERT_EQUALS(cmpGarrisonHolder.AllowedToVisibleGarrisoning(33, cmpGarrisonHolder.visibleGarrisonPoints[0]), true);
+TS_ASSERT_EQUALS(cmpGarrisonHolder.AllowedToVisibleGarrisoning(33, cmpGarrisonHolder.visibleGarrisonPoints[1]), false);
+TS_ASSERT_EQUALS(cmpGarrisonHolder.AllowedToVisibleGarrisoning(33, cmpGarrisonHolder.visibleGarrisonPoints[2]), true);
+
+// If an entity gets renamed (e.g. promotion, upgrade)
+// and is no longer able to be visibly garrisoned it
+// should be garisoned instead or ejected.
+AddMock(siegeEngineId, IID_Position, {
+	"GetHeightOffset": () => 0,
+	"GetPosition": () => new Vector3D(4, 3, 25),
+	"GetRotation": () => new Vector3D(4, 0, 6),
+	"GetTurretParent": () => INVALID_ENTITY,
+	"IsInWorld": () => true,
+	"JumpTo": (posX, posZ) => {},
+	"MoveOutOfWorld": () => {},
+	"SetTurretParent": (entity, offset) => {},
+	"SetHeightOffset": height => {}
+});
+let currentSiegePlayer = player;
+AddMock(siegeEngineId, IID_Ownership, {
+	"GetOwner": () => currentSiegePlayer
+});
+AddMock(siegeEngineId, IID_Garrisonable, {});
+let cavalryId = 46;
+AddMock(cavalryId, IID_Identity, {
+	"GetClassesList": () => ["Infantry", "Ranged"]
+});
+AddMock(cavalryId, IID_Position, {
+	"GetHeightOffset": () => 0,
+	"GetPosition": () => new Vector3D(4, 3, 25),
+	"GetRotation": () => new Vector3D(4, 0, 6),
+	"GetTurretParent": () => INVALID_ENTITY,
+	"IsInWorld": () => true,
+	"JumpTo": (posX, posZ) => {},
+	"MoveOutOfWorld": () => {},
+	"SetTurretParent": (entity, offset) => {},
+	"SetHeightOffset": height => {}
+});
+
+let currentCavalryPlayer = player;
+AddMock(cavalryId, IID_Ownership, {
+	"GetOwner": () => currentCavalryPlayer
+});
+AddMock(cavalryId, IID_Garrisonable, {});
+TS_ASSERT(cmpGarrisonHolder.Garrison(siegeEngineId));
+TS_ASSERT_EQUALS(cmpGarrisonHolder.GetGarrisonedEntitiesCount(), 1);
+TS_ASSERT(cmpGarrisonHolder.IsVisiblyGarrisoned(siegeEngineId));
+cmpGarrisonHolder.OnGlobalEntityRenamed({
+	"entity": siegeEngineId,
+	"newentity": cavalryId
+});
+TS_ASSERT_EQUALS(cmpGarrisonHolder.GetGarrisonedEntitiesCount(), 1);
+TS_ASSERT(!cmpGarrisonHolder.IsVisiblyGarrisoned(siegeEngineId));
+TS_ASSERT(!cmpGarrisonHolder.IsVisiblyGarrisoned(archerId));
+
+// Eject enemy units.
+currentCavalryPlayer = enemyPlayer;
+cmpGarrisonHolder.OnGlobalOwnershipChanged({
+	"entity": cavalryId,
+	"to": enemyPlayer
+});
+TS_ASSERT_EQUALS(cmpGarrisonHolder.GetGarrisonedEntitiesCount(), 0);
+
+// Visibly garrisoned units should get ejected if they change players.
+TS_ASSERT(cmpGarrisonHolder.Garrison(siegeEngineId));
+TS_ASSERT(cmpGarrisonHolder.IsVisiblyGarrisoned(siegeEngineId));
+TS_ASSERT_EQUALS(cmpGarrisonHolder.GetGarrisonedEntitiesCount(), 1);
+currentSiegePlayer = enemyPlayer;
+cmpGarrisonHolder.OnGlobalOwnershipChanged({
+	"entity": siegeEngineId,
+	"to": enemyPlayer
+});
+TS_ASSERT_EQUALS(cmpGarrisonHolder.GetGarrisonedEntitiesCount(), 0);