bug: Lure behavior for sheep not working

[skaldarnar]

Context

The https://github.com/Terasology/WildAnimals/blob/develop/assets/behaviors/lure.behavior is currently used for the sheep mobs.
It is intended to give players a way to lure sheep together and eventually flock them in a pen for domestication. Sheep are supposed to
follow a player who is holding a grass item.

Problem

Unfortunately, this luring behavior does not work properly after the refactoring of behaviors onto https://github.com/Terasology/FlexiblePathfinding. The culprit seems to be somewhere in the guard behavior node:

WildAnimals/assets/behaviors/lure.behavior

Lines 4 to 15 in c011fab

	guard: {
	componentPresent: "Behaviors:FindNearbyPlayers",
	values: ["N charactersWithinRange nonEmpty"],
	child: {
	sequence: [
	{ sleep: { time: 0.1f } },
	check_luring_item_in_use,
	followCharacter,
	{ lookup: { tree: "Behaviors:follow" } }
	]
	}
	}

Goal

Sheep should follow the nearest player in range that is holding a grass item.

If no such player exists, sheep should perform the stray behavior.

Hints

This will require some debugging of the GuardAction from the Terasology engine repo, and in particular the parent class ConditionAction.

[Jacob-Rueckert]

Debug Procedure:
Looked into CheckLuringItemInUse Action
Added a Log to the Stray Behaviour

Results where when a player is in range the sheep keeps checking if player has a item in hand and doesnt go into stray when holding not the right item.
When the player is out of range, stray behaviour occurs, but sheep has no visible movement. (Bug in Stray?)

Next Steps:

• Looking what happens when holding the right item and going out of range
• Looking what happens when following and going out of range
• Investigating where it jumps to when checkLuringItemInUse returns FAILURE
  • Possibly before Guard (Dynamic)
  • Possibly in Guard (Guard)
  • Possibly after Guard (Sequence)

FollowUp:

• Checking why stray doesnt show visible movement when called

[jdrueckert]

@Jacob-Rueckert I did some digging based on what you found out

when a player is in range the sheep keeps checking if player has a item in hand and doesnt go into stray when holding not the right item

I have the assumption that the guard does not process the child result (BehaviorState) and I think what I found in the engine's behavior code proves that assumption.
GuardAction only overrides prune and inherits all other methods from ConditionAction it extends, including ConditionAction::modify.

The modify method checks the condition, but doesn't make use of the input parameter BehaviorState result which it gets passed to from DecoratorNode based on previous execution of the node's child.
This means that when the item lookup fails and the parent node is rechecked and should communicate the children's failure further up the tree, it actually doesn't but starts executing the children again because the condition (player in range) still holds true. That's why the sheep never goes into the other child (stray) while the player is in range.

I tried to fix this like so:

--- a/engine/src/main/java/org/terasology/engine/logic/behavior/actions/conditions/ConditionAction.java
+++ b/engine/src/main/java/org/terasology/engine/logic/behavior/actions/conditions/ConditionAction.java
@@ -50,7 +50,7 @@ public void construct(Actor actor) {
     @Override
     public BehaviorState modify(Actor actor, BehaviorState result) {
         try {
-            if (!condition(actor)) {
+            if (!condition(actor) || result.equals(BehaviorState.FAILURE)) {
                 return BehaviorState.FAILURE;
             }
             return BehaviorState.SUCCESS;

which helps in so far that the sheep moves, but something is still weird with the pathfinding while the player is in range: instead of selecting a target, finding a path to it and moving along the path like it does if the player is out of range, the sheep only walks in a straight line forward until it gets stuck somewhere because it doesn't jump (or do any actual pathfinding and movement along a path at all) in this state, so we definitely do need some additional investigation.

I'll open a PR with the preliminary fix so you can try it out on your end as well and see what I mean and can continue investigating.