Improve gathering behaviour when a target is not reachable

Units can be stuck in gather states when the target ought to be
collectable from, but it's actually unreachable.

A new FINDINGNEWTARGET intermediate state for GATHER will try and find a
new, different target to collect from. This should generally improve the
behaviour (perfect reachability checks would be required to completely
fix it).
Use this also when the target is knowably unreachable / uncollectible.

Fixes #5529

Differential Revision: https://code.wildfiregames.com/D2120
This was SVN commit r22816.

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

@@ -2090,16 +2090,27 @@ UnitAI.prototype.UnitFsmSpec = {
 				"enter": function() {
 					this.gatheringTarget = this.order.data.target;	// temporary, deleted in "leave".
 
-					// check that we can gather from the resource we're supposed to gather from.
+					// Check that we can gather from the resource we're supposed to gather from.
-					var cmpOwnership = Engine.QueryInterface(this.entity, IID_Ownership);
+					let cmpOwnership = Engine.QueryInterface(this.entity, IID_Ownership);
-					var cmpSupply = Engine.QueryInterface(this.gatheringTarget, IID_ResourceSupply);
+					let cmpSupply = Engine.QueryInterface(this.gatheringTarget, IID_ResourceSupply);
-					var cmpMirage = Engine.QueryInterface(this.gatheringTarget, IID_Mirage);
+					let cmpMirage = Engine.QueryInterface(this.gatheringTarget, IID_Mirage);
 					if ((!cmpMirage || !cmpMirage.Mirages(IID_ResourceSupply)) &&
 					    (!cmpSupply || !cmpSupply.AddGatherer(cmpOwnership.GetOwner(), this.entity)) ||
 					    !this.MoveTo(this.order.data, IID_ResourceGatherer))
 					{
-						// The GATHERING timer will handle finding a valid resource.
+						// If the target's last known position is in FOW, try going there
-						this.SetNextState("GATHERING");
+						// and hope that we might find it then.
+						let lastPos = this.order.data.lastPos;
+						if (this.gatheringTarget != INVALID_ENTITY &&
+						    lastPos && !this.CheckPositionVisible(lastPos.x, lastPos.z))
+						{
+							this.PushOrderFront("Walk", {
+								"x": lastPos.x, "z": lastPos.z,
+								"force": this.order.data.force
+							});
+							return true;
+						}
+						this.SetNextState("FINDINGNEWTARGET");
 						return true;
 					}
 					return false;
@@ -2107,7 +2118,9 @@ UnitAI.prototype.UnitFsmSpec = {
 
 				"MovementUpdate": function(msg) {
 					// The GATHERING timer will handle finding a valid resource.
-					if (msg.likelyFailure || this.CheckRange(this.order.data, IID_ResourceGatherer))
+					if (msg.likelyFailure)
+						this.SetNextState("FINDINGNEWTARGET");
+					else if (this.CheckRange(this.order.data, IID_ResourceGatherer))
 						this.SetNextState("GATHERING");
 				},
 
@@ -2160,8 +2173,8 @@ UnitAI.prototype.UnitFsmSpec = {
 						cmpSupply = Engine.QueryInterface(this.gatheringTarget, IID_ResourceSupply);
 					if (!cmpSupply || !cmpSupply.AddGatherer(cmpOwnership.GetOwner(), this.entity))
 					{
-						this.StartTimer(0);
+						this.SetNextState("FINDINGNEWTARGET");
-						return false;
+						return true;
 					}
 
 					// If this order was forced, the player probably gave it, but now we've reached the target
@@ -2179,9 +2192,8 @@ UnitAI.prototype.UnitFsmSpec = {
 						// Try to find another target if the current one stopped existing
 						if (!Engine.QueryInterface(this.gatheringTarget, IID_Identity))
 						{
-							// Let the Timer logic handle this
+							this.SetNextState("FINDINGNEWTARGET");
-							this.StartTimer(0);
+							return true;
-							return false;
 						}
 
 						// No rate, give up on gathering
@@ -2233,74 +2245,89 @@ UnitAI.prototype.UnitFsmSpec = {
 					if (!cmpOwnership)
 						return;
 
+					// TODO: we are leaking information here - if the target died in FOW, we'll know it's dead
+					// straight away.
+					// Seems one would have to listen to ownership changed messages to make it work correctly
+					// but that's likely prohibitively expansive performance wise.
+
 					let cmpSupply = Engine.QueryInterface(this.gatheringTarget, IID_ResourceSupply);
-					if (cmpSupply && cmpSupply.IsAvailable(cmpOwnership.GetOwner(), this.entity))
+					// If we can't gather from the target, find a new one.
-						// Check we can still reach and gather from the target
+					if (!cmpSupply || !cmpSupply.IsAvailable(cmpOwnership.GetOwner(), this.entity) ||
-						if (this.CheckTargetRange(this.gatheringTarget, IID_ResourceGatherer) && this.CanGather(this.gatheringTarget))
+					    !this.CanGather(this.gatheringTarget))
-						{
+					{
-							// Gather the resources:
+						this.SetNextState("FINDINGNEWTARGET");
+						return;
+					}
 
-							let cmpResourceGatherer = Engine.QueryInterface(this.entity, IID_ResourceGatherer);
+					if (!this.CheckTargetRange(this.gatheringTarget, IID_ResourceGatherer))
-
+					{
-							// Try to gather treasure
+						// Try to follow the target
-							if (cmpResourceGatherer.TryInstantGather(this.gatheringTarget))
+						if (this.MoveToTargetRange(this.gatheringTarget, IID_ResourceGatherer))
-								return;
+							this.SetNextState("APPROACHING");
-
+						// Our target is no longer visible - go to its last known position first
-							// If we've already got some resources but they're the wrong type,
+						// and then hopefully it will become visible.
-							// drop them first to ensure we're only ever carrying one type
+						else if (!this.CheckTargetVisible(this.gatheringTarget) && this.order.data.lastPos)
-							if (cmpResourceGatherer.IsCarryingAnythingExcept(resourceType.generic))
+							this.PushOrderFront("Walk", {
-								cmpResourceGatherer.DropResources();
+								"x": this.order.data.lastPos.x,
-
+								"z": this.order.data.lastPos.z,
-							this.FaceTowardsTarget(this.order.data.target);
+								"force": this.order.data.force
-
+							});
-							// Collect from the target
-							let status = cmpResourceGatherer.PerformGather(this.gatheringTarget);
-
-							// If we've collected as many resources as possible,
-							// return to the nearest dropsite
-							if (status.filled)
-							{
-								let nearby = this.FindNearestDropsite(resourceType.generic);
-								if (nearby)
-								{
-									// (Keep this Gather order on the stack so we'll
-									// continue gathering after returning)
-									this.PushOrderFront("ReturnResource", { "target": nearby, "force": false });
-									return;
-								}
-
-								// Oh no, couldn't find any drop sites. Give up on gathering.
-								this.FinishOrder();
-								return;
-							}
-
-							// We can gather more from this target, do so in the next timer
-							if (!status.exhausted)
-								return;
-						}
 						else
-						{
+							this.SetNextState("FINDINGNEWTARGET");
-							// Try to follow the target
+						return;
-							if (this.MoveToTargetRange(this.gatheringTarget, IID_ResourceGatherer))
+					}
-							{
-								this.SetNextState("APPROACHING");
-								return;
-							}
 
-							// Our target is no longer visible - go to its last known position first
+					// Gather the resources:
-							// and then hopefully it will become visible.
+
-							if (!this.CheckTargetVisible(this.gatheringTarget) && this.order.data.lastPos)
+					let cmpResourceGatherer = Engine.QueryInterface(this.entity, IID_ResourceGatherer);
-							{
+
-								this.PushOrderFront("Walk", {
+					// Try to gather treasure
-									"x": this.order.data.lastPos.x,
+					if (cmpResourceGatherer.TryInstantGather(this.gatheringTarget))
-									"z": this.order.data.lastPos.z,
+						return;
-									"force": this.order.data.force
+
-								});
+					// If we've already got some resources but they're the wrong type,
-								return;
+					// drop them first to ensure we're only ever carrying one type
-							}
+					if (cmpResourceGatherer.IsCarryingAnythingExcept(resourceType.generic))
+						cmpResourceGatherer.DropResources();
+
+					this.FaceTowardsTarget(this.order.data.target);
+
+					// Collect from the target
+					let status = cmpResourceGatherer.PerformGather(this.gatheringTarget);
+
+					// If we've collected as many resources as possible,
+					// return to the nearest dropsite
+					if (status.filled)
+					{
+						let nearby = this.FindNearestDropsite(resourceType.generic);
+						if (nearby)
+						{
+							// (Keep this Gather order on the stack so we'll
+							// continue gathering after returning)
+							// However mark our target as invalid if it's exhausted, so we don't waste time
+							// trying to gather from it.
+							if (status.exhausted)
+								this.order.data.target = INVALID_ENTITY;
+							this.PushOrderFront("ReturnResource", { "target": nearby, "force": false });
+							return;
 						}
 
-					// We're already in range, can't get anywhere near it or the target is exhausted.
+						// Oh no, couldn't find any drop sites. Give up on gathering.
+						this.FinishOrder();
+						return;
+					}
+
+					// Find a new target if the current one is exhausted
+					if (status.exhausted)
+						this.SetNextState("FINDINGNEWTARGET");
+				},
+			},
+
+			"FINDINGNEWTARGET": {
+				"enter": function() {
+					let previousTarget = this.order.data.target;
+					let resourceTemplate = this.order.data.template;
+					let resourceType = this.order.data.type;
 
 					// Give up on this order and try our next queued order
 					// but first check what is our next order and, if needed, insert a returnResource order
@@ -2318,10 +2345,12 @@ UnitAI.prototype.UnitFsmSpec = {
 					let initPos = this.order.data.initPos;
 
 					if (this.FinishOrder())
-						return;
+						return true;
 
 					// No remaining orders - pick a useful default behaviour
 
+					// If we have no known initial position of our target, look around our own position
+					// as a fallback.
 					if (!initPos)
 					{
 						let cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
@@ -2336,18 +2365,21 @@ UnitAI.prototype.UnitFsmSpec = {
 					{
 						// Try to find a new resource of the same specific type near the initial resource position:
 						// Also don't switch to a different type of huntable animal
-						let nearby = this.FindNearbyResource(function(ent, type, template) {
+						let nearby = this.FindNearbyResource((ent, type, template) => {
-							return (
+							if (previousTarget == ent)
-								(type.generic == "treasure" && resourceType.generic == "treasure") ||
+								return false;
-								(type.specific == resourceType.specific &&
+
-									(type.specific != "meat" || resourceTemplate == template))
+							if (type.generic == "treasure" && resourceType.generic == "treasure")
-							);
+								return true;
+
+							return type.specific == resourceType.specific &&
+							    (type.specific != "meat" || resourceTemplate == template);
 						}, new Vector2D(initPos.x, initPos.z));
 
 						if (nearby)
 						{
 							this.PerformGather(nearby, false, false);
-							return;
+							return true;
 						}
 
 						// Failing that, try to move there and se if we are more lucky: maybe there are resources in FOW.
@@ -2355,7 +2387,7 @@ UnitAI.prototype.UnitFsmSpec = {
 						if (!this.CheckPointRangeExplicit(initPos.x, initPos.z, 0, 10))
 						{
 							this.GatherNearPosition(initPos.x, initPos.z, resourceType, resourceTemplate);
-							return;
+							return true;
 						}
 					}
 
@@ -2367,10 +2399,11 @@ UnitAI.prototype.UnitFsmSpec = {
 					if (nearby)
 					{
 						this.PushOrderFront("ReturnResource", { "target": nearby, "force": false });
-						return;
+						return true;
 					}
 
-					// No dropsites - just give up
+					// No dropsites - just give up.
+					return true;
 				},
 			},
 		},
@@ -4480,6 +4513,22 @@ UnitAI.prototype.CheckTargetVisible = function(target)
 	return true;
 };
 
+/**
+ * Returns true if the given position is currentl visible (not in FoW/SoD).
+ */
+UnitAI.prototype.CheckPositionVisible = function(x, z)
+{
+	let cmpOwnership = Engine.QueryInterface(this.entity, IID_Ownership);
+	if (!cmpOwnership)
+		return false;
+
+	let cmpRangeManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_RangeManager);
+	if (!cmpRangeManager)
+		return false;
+
+	return cmpRangeManager.GetLosVisibilityPosition(x, z, cmpOwnership.GetOwner()) == "visible";
+};
+
 /**
  * How close to our goal do we consider it's OK to stop if the goal appears unreachable.
  * Currently 3 terrain tiles as that's relatively close but helps pathfinding.

source/simulation2/components/CCmpRangeManager.cpp

@@ -1726,6 +1726,30 @@ public:
 		return GetLosVisibility(handle, player);
 	}
 
+	virtual ELosVisibility GetLosVisibilityPosition(entity_pos_t x, entity_pos_t z, player_id_t player) const
+	{
+		int i = (x / (int)TERRAIN_TILE_SIZE).ToInt_RoundToNearest();
+		int j = (z / (int)TERRAIN_TILE_SIZE).ToInt_RoundToNearest();
+
+		// Reveal flag makes all positioned entities visible and all mirages useless
+		if (GetLosRevealAll(player))
+		{
+			if (LosIsOffWorld(i, j))
+				return VIS_HIDDEN;
+			else
+				return VIS_VISIBLE;
+		}
+
+		// Get visible regions
+		CLosQuerier los(GetSharedLosMask(player), m_LosState, m_TerrainVerticesPerSide);
+
+		if (los.IsVisible(i,j))
+			return VIS_VISIBLE;
+		if (los.IsExplored(i,j))
+			return VIS_FOGGED;
+		return VIS_HIDDEN;
+	}
+
 	i32 PosToLosTilesHelper(entity_pos_t x, entity_pos_t z) const
 	{
 		i32 i = Clamp(

source/simulation2/components/ICmpRangeManager.cpp

@@ -1,4 +1,4 @@
-/* Copyright (C) 2017 Wildfire Games.
+/* Copyright (C) 2019 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -33,6 +33,18 @@ std::string ICmpRangeManager::GetLosVisibility_wrapper(entity_id_t ent, int play
 	}
 }
 
+std::string ICmpRangeManager::GetLosVisibilityPosition_wrapper(entity_pos_t x, entity_pos_t z, int player) const
+{
+	ELosVisibility visibility = GetLosVisibilityPosition(x, z, player);
+	switch (visibility)
+	{
+		case VIS_HIDDEN: return "hidden";
+		case VIS_FOGGED: return "fogged";
+		case VIS_VISIBLE: return "visible";
+		default: return "error"; // should never happen
+	}
+}
+
 BEGIN_INTERFACE_WRAPPER(RangeManager)
 DEFINE_INTERFACE_METHOD_5("ExecuteQuery", std::vector<entity_id_t>, ICmpRangeManager, ExecuteQuery, entity_id_t, entity_pos_t, entity_pos_t, std::vector<int>, int)
 DEFINE_INTERFACE_METHOD_5("ExecuteQueryAroundPos", std::vector<entity_id_t>, ICmpRangeManager, ExecuteQueryAroundPos, CFixedVector2D, entity_pos_t, entity_pos_t, std::vector<int>, int)
@@ -56,6 +68,7 @@ DEFINE_INTERFACE_METHOD_CONST_1("GetLosRevealAll", bool, ICmpRangeManager, GetLo
 DEFINE_INTERFACE_METHOD_CONST_5("GetElevationAdaptedRange", entity_pos_t, ICmpRangeManager, GetElevationAdaptedRange, CFixedVector3D, CFixedVector3D, entity_pos_t, entity_pos_t, entity_pos_t)
 DEFINE_INTERFACE_METHOD_2("ActivateScriptedVisibility", void, ICmpRangeManager, ActivateScriptedVisibility, entity_id_t, bool)
 DEFINE_INTERFACE_METHOD_CONST_2("GetLosVisibility", std::string, ICmpRangeManager, GetLosVisibility_wrapper, entity_id_t, player_id_t)
+DEFINE_INTERFACE_METHOD_CONST_3("GetLosVisibilityPosition", std::string, ICmpRangeManager, GetLosVisibilityPosition_wrapper, entity_pos_t, entity_pos_t, player_id_t)
 DEFINE_INTERFACE_METHOD_1("RequestVisibilityUpdate", void, ICmpRangeManager, RequestVisibilityUpdate, entity_id_t)
 DEFINE_INTERFACE_METHOD_1("SetLosCircular", void, ICmpRangeManager, SetLosCircular, bool)
 DEFINE_INTERFACE_METHOD_CONST_0("GetLosCircular", bool, ICmpRangeManager, GetLosCircular)

source/simulation2/components/ICmpRangeManager.h

@@ -1,4 +1,4 @@
-/* Copyright (C) 2017 Wildfire Games.
+/* Copyright (C) 2019 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -345,6 +345,13 @@ public:
 	virtual ELosVisibility GetLosVisibility(CEntityHandle ent, player_id_t player) const = 0;
 	virtual ELosVisibility GetLosVisibility(entity_id_t ent, player_id_t player) const = 0;
 
+	/**
+	 * Returns the visibility status of the given position, with respect to the given player.
+	 * This respects the GetLosRevealAll flag.
+	 */
+	virtual ELosVisibility GetLosVisibilityPosition(entity_pos_t x, entity_pos_t z, player_id_t player) const = 0;
+
+	/**
 	/**
 	 * Request the update of the visibility cache of ent at next turn.
 	 * Typically used for fogging.
@@ -358,6 +365,12 @@ public:
 	 */
 	std::string GetLosVisibility_wrapper(entity_id_t ent, player_id_t player) const;
 
+	/**
+	 * GetLosVisibilityPosition wrapped for script calls.
+	 * Returns "hidden", "fogged" or "visible".
+	 */
+	std::string GetLosVisibilityPosition_wrapper(entity_pos_t x, entity_pos_t z, player_id_t player) const;
+
 	/**
 	 * Explore all tiles (but leave them in the FoW) for player p
 	 */