diff --git a/assets/behaviors/common/doRandomMove.behavior b/assets/behaviors/common/doRandomMove.behavior index 12cd6936..3d4425f6 100644 --- a/assets/behaviors/common/doRandomMove.behavior +++ b/assets/behaviors/common/doRandomMove.behavior @@ -1,9 +1,7 @@ { sequence : [ - { - set_target_nearby_block : { moveProbability: 65 } - }, - move_to + { set_target_nearby_block : { moveProbability: 65 } }, + { lookup: { tree: "Behaviors:naiveMoveTo" } } ] } diff --git a/assets/behaviors/common/stray.behavior b/assets/behaviors/common/stray.behavior index 8413cfb7..7e6bb56a 100644 --- a/assets/behaviors/common/stray.behavior +++ b/assets/behaviors/common/stray.behavior @@ -1,31 +1,28 @@ { - - sequence : [ - { - set_speed : { speedMultiplier: 0.3 } - }, - { - animation : { - play: "engine:Walk.animationPool", - loop: "engine:Walk.animationPool" - } - }, - { - lookup: { tree: "Behaviors:doRandomMove" } - }, - { - animation : { - play: "engine:Stand.animationPool", - loop: "engine:Stand.animationPool" - } - }, - { - set_speed : { speedMultiplier: 0 } - }, - { - sleep : { - time : 3 - } - } - ] + sequence : [ + { + set_speed : { speedMultiplier: 0.3 } + }, + { + animation : { + play: "engine:Walk.animationPool", + loop: "engine:Walk.animationPool" + } + }, + { + lookup: { tree: "Behaviors:doRandomMove" } + }, + { + animation : { + play: "engine:Stand.animationPool", + loop: "engine:Stand.animationPool" + } + }, + { + set_speed : { speedMultiplier: 0 } + }, + { + sleep : { time : 3 } + } + ] } diff --git a/assets/behaviors/creatures/critter.behavior b/assets/behaviors/creatures/critter.behavior index 22999722..17bd5847 100644 --- a/assets/behaviors/creatures/critter.behavior +++ b/assets/behaviors/creatures/critter.behavior @@ -1,25 +1,16 @@ { dynamic: [ - { - guard: { - componentPresent: "Behaviors:Fleeing", - child: { - sequence: [ + { + guard: { + componentPresent: "Behaviors:Fleeing", + child: { + sequence: [ check_flee_stop, - { - lookup: { - tree: "Behaviors:flee" - } - } - ] - } - } - }, - { - lookup: { - tree: "Behaviors:stray" + { lookup: { tree: "Behaviors:flee" } } + ] } } - + }, + { lookup: { tree: "Behaviors:stray" } } ] -} \ No newline at end of file +} diff --git a/assets/behaviors/creatures/hostileCritter.behavior b/assets/behaviors/creatures/hostileCritter.behavior index b2c2c314..d8453a61 100644 --- a/assets/behaviors/creatures/hostileCritter.behavior +++ b/assets/behaviors/creatures/hostileCritter.behavior @@ -5,11 +5,10 @@ componentPresent: "Behaviors:FindNearbyPlayers", values: ["N charactersWithinRange nonEmpty"], child: { - sequence: [ - { sleep: {time: 0.1f }}, - followCharacter, - { lookup: {tree: "Behaviors:hostile" }} - + sequence: [ + { sleep: {time: 0.1f }}, + followCharacter, + { lookup: {tree: "Behaviors:hostile" }} ] } } diff --git a/assets/behaviors/naiveMoveTo.behavior b/assets/behaviors/naiveMoveTo.behavior index cc081c4d..63da9475 100644 --- a/assets/behaviors/naiveMoveTo.behavior +++ b/assets/behaviors/naiveMoveTo.behavior @@ -1,10 +1,6 @@ { sequence : [ - { - selector : [ - find_path - ] - }, + find_path, { move_along_path: { child : { move_to: {} diff --git a/assets/prefabs/test/testcharacter.prefab b/assets/prefabs/testCharacter.prefab similarity index 94% rename from assets/prefabs/test/testcharacter.prefab rename to assets/prefabs/testCharacter.prefab index dc64a34b..f5cf5fd2 100644 --- a/assets/prefabs/test/testcharacter.prefab +++ b/assets/prefabs/testCharacter.prefab @@ -3,7 +3,7 @@ "tree" : "naiveMoveTo" }, "persisted" : true, - "location" : {}, + "Location" : {}, "Network" :{}, "Trigger" : { "detectGroups" : ["engine:debris", "engine:sensor"] diff --git a/src/main/java/org/terasology/module/behaviors/actions/CheckAttackStopAction.java b/src/main/java/org/terasology/module/behaviors/actions/CheckAttackStopAction.java index 21fded6f..4491b7a1 100644 --- a/src/main/java/org/terasology/module/behaviors/actions/CheckAttackStopAction.java +++ b/src/main/java/org/terasology/module/behaviors/actions/CheckAttackStopAction.java @@ -3,17 +3,39 @@ package org.terasology.module.behaviors.actions; import org.joml.Vector3f; -import org.terasology.module.behaviors.components.AttackInProximityComponent; -import org.terasology.module.behaviors.components.AttackOnHitComponent; -import org.terasology.module.behaviors.components.FollowComponent; import org.terasology.engine.logic.behavior.BehaviorAction; import org.terasology.engine.logic.behavior.core.Actor; import org.terasology.engine.logic.behavior.core.BaseAction; import org.terasology.engine.logic.behavior.core.BehaviorState; import org.terasology.engine.logic.location.LocationComponent; +import org.terasology.module.behaviors.components.AttackInProximityComponent; +import org.terasology.module.behaviors.components.AttackOnHitComponent; +import org.terasology.module.behaviors.components.FollowComponent; import org.terasology.nui.properties.Range; - +//TODO: Should this rather be a dedicated behavior instead? +// This is a complex action replacing a behavior tree - is that a good idea or not? +// +// StopAttackIfOutOfFollowDistance.behavior +// selector [ // condition && clear +// sequence: [ // AND +// guard: +// componentPresent: "LocationComponent" +// guard: +// componentPresent: "FollowComponent" +// values: [ "N entityToFollow exists" ] +// selector: [ // OR +// guard: +// componentPresent: "AttackOnHitComponent" +// guard: +// componentPresent: "AttackInProximityComponent" +// ] +// check_attack_target_in_reach +// ] +// +// // if failed +// set_attack_target_clear +// ] @BehaviorAction(name = "check_attack_stop") public class CheckAttackStopAction extends BaseAction { @@ -26,26 +48,30 @@ public class CheckAttackStopAction extends BaseAction { */ @Override public BehaviorState modify(Actor actor, BehaviorState state) { - BehaviorState status = getBehaviorStateWithoutReturn(actor); - if (status == BehaviorState.FAILURE) { - if (actor.hasComponent(AttackOnHitComponent.class)) { - AttackOnHitComponent attackOnHitComponent = actor.getComponent(AttackOnHitComponent.class); - attackOnHitComponent.instigator = null; - actor.getEntity().saveComponent(attackOnHitComponent); - } else if (actor.hasComponent(AttackInProximityComponent.class)) { - AttackInProximityComponent attackInProximityComponent = actor.getComponent(AttackInProximityComponent.class); - attackInProximityComponent.instigator = null; - actor.getEntity().saveComponent(attackInProximityComponent); - } - actor.getEntity().removeComponent(FollowComponent.class); + if (isTargetEntityOutOfFollowDistance(actor)) { + clearAttackTarget(actor); + return BehaviorState.FAILURE; } - return status; + return BehaviorState.SUCCESS; } - private BehaviorState getBehaviorStateWithoutReturn(Actor actor) { + private void clearAttackTarget(Actor actor) { + if (actor.hasComponent(AttackOnHitComponent.class)) { + AttackOnHitComponent attackOnHitComponent = actor.getComponent(AttackOnHitComponent.class); + attackOnHitComponent.instigator = null; + actor.getEntity().saveComponent(attackOnHitComponent); + } else if (actor.hasComponent(AttackInProximityComponent.class)) { + AttackInProximityComponent attackInProximityComponent = actor.getComponent(AttackInProximityComponent.class); + attackInProximityComponent.instigator = null; + actor.getEntity().saveComponent(attackInProximityComponent); + } + actor.getEntity().removeComponent(FollowComponent.class); + } + + private boolean isTargetEntityOutOfFollowDistance(Actor actor) { LocationComponent actorLocationComponent = actor.getComponent(LocationComponent.class); if (actorLocationComponent == null) { - return BehaviorState.FAILURE; + return true; } Vector3f actorPosition = actorLocationComponent.getWorldPosition(new Vector3f()); float distance = this.maxDistance; @@ -58,17 +84,17 @@ private BehaviorState getBehaviorStateWithoutReturn(Actor actor) { float maxDistanceSquared = distance * distance; FollowComponent followWish = actor.getComponent(FollowComponent.class); if (followWish == null || followWish.entityToFollow == null) { - return BehaviorState.FAILURE; + return true; } LocationComponent locationComponent = followWish.entityToFollow.getComponent(LocationComponent.class); if (locationComponent == null) { - return BehaviorState.FAILURE; + return true; } if (locationComponent.getWorldPosition(new Vector3f()).distanceSquared(actorPosition) <= maxDistanceSquared) { - return BehaviorState.SUCCESS; + return false; } - return BehaviorState.FAILURE; + return true; } } diff --git a/src/main/java/org/terasology/module/behaviors/actions/FindDummyPathTo.java b/src/main/java/org/terasology/module/behaviors/actions/FindDummyPathTo.java index 845831dc..723ecebc 100644 --- a/src/main/java/org/terasology/module/behaviors/actions/FindDummyPathTo.java +++ b/src/main/java/org/terasology/module/behaviors/actions/FindDummyPathTo.java @@ -23,7 +23,8 @@ public class FindDummyPathTo extends BaseAction { @Override public BehaviorState modify(Actor actor, BehaviorState result) { MinionMoveComponent movement = actor.getComponent(MinionMoveComponent.class); - Vector3i goal = actor.getComponent(MinionMoveComponent.class).getPathGoal(); + + Vector3i goal = movement.getPathGoal(); if (goal == null) { return BehaviorState.FAILURE; diff --git a/src/main/java/org/terasology/module/behaviors/actions/FindPathToNode.java b/src/main/java/org/terasology/module/behaviors/actions/FindPathToNode.java index 294f3c24..e833ba70 100644 --- a/src/main/java/org/terasology/module/behaviors/actions/FindPathToNode.java +++ b/src/main/java/org/terasology/module/behaviors/actions/FindPathToNode.java @@ -4,6 +4,8 @@ import org.joml.Vector3f; import org.joml.Vector3ic; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.terasology.engine.logic.behavior.BehaviorAction; import org.terasology.engine.logic.behavior.core.Actor; import org.terasology.engine.logic.behavior.core.BaseAction; @@ -28,12 +30,20 @@ @BehaviorAction(name = "find_path") public class FindPathToNode extends BaseAction { + private static final Logger logger = LoggerFactory.getLogger(FindPathToNode.class); + @In transient PathfinderSystem pathfinderSystem; @In transient PluginSystem pluginSystem; + // TODO: how do we want to remember state in actions? + // 1. action field as used here + // 2. actor.setValue, actor.getValue as used in SleepAction + // 3. write to, read from component + // 4. ... + private boolean isPathfindingRunning = false; @Override public void construct(Actor actor) { @@ -44,6 +54,8 @@ public void construct(Actor actor) { pluginSystem = CoreRegistry.get(PluginSystem.class); } + logger.debug("Actor {}: construct find_path Action", actor.getEntity().getId()); + MinionMoveComponent minionMoveComponent = actor.getComponent(MinionMoveComponent.class); Vector3ic start = Blocks.toBlockPos(actor.getComponent(LocationComponent.class).getWorldPosition(new Vector3f())); Vector3ic goal = actor.getComponent(MinionMoveComponent.class).getPathGoal(); @@ -53,10 +65,14 @@ public void construct(Actor actor) { config.requester = actor.getEntity(); config.maxTime = 10f; config.maxDepth = 150; - config.goalDistance = minionMoveComponent.pathGoalDistance; + config.goalDistance = minionMoveComponent.goalTolerance; config.plugin = pluginSystem.getMovementPlugin(actor.getEntity()).getJpsPlugin(actor.getEntity()); + isPathfindingRunning = true; + + logger.debug("... [{}]: compute path between {} -> {}", actor.getEntity().getId(), start, goal); int id = pathfinderSystem.requestPath(config, (path, target) -> { + isPathfindingRunning = false; if (path == null || path.size() == 0) { return; } @@ -66,16 +82,27 @@ public void construct(Actor actor) { minionMoveComponent1.setPath(path); actor.save(minionMoveComponent1); }); + if (id == -1) { + // task was not accepted + isPathfindingRunning = false; + } } @Override public BehaviorState modify(Actor actor, BehaviorState result) { + logger.debug("Actor {}: in find_path Action", actor.getEntity().getId()); // this an action node, so it will always be called with BehaviorState.UNDEFINED if (result == BehaviorState.RUNNING) { // this can never happen o.O return result; } + if (isPathfindingRunning) { + logger.debug("... [{}]: ... still searching for path", actor.getEntity().getId()); + return BehaviorState.RUNNING; + } + MinionMoveComponent minionMoveComponent = actor.getComponent(MinionMoveComponent.class); + logger.debug("... [{}]: pathfinding done: {}", actor.getEntity().getId(), minionMoveComponent.getPath()); return minionMoveComponent.getPath().isEmpty() ? BehaviorState.FAILURE : BehaviorState.SUCCESS; } } diff --git a/src/main/java/org/terasology/module/behaviors/actions/LogAction.java b/src/main/java/org/terasology/module/behaviors/actions/LogAction.java index 8e42a0af..c601e566 100644 --- a/src/main/java/org/terasology/module/behaviors/actions/LogAction.java +++ b/src/main/java/org/terasology/module/behaviors/actions/LogAction.java @@ -12,13 +12,18 @@ import org.terasology.nui.properties.TextField; /** - * Logs a message into the console when called and returns SUCCESS + * Logs a debug message when called. Always returns SUCCESS. + *

+ * For example, you can add such a log action at the beginning of a sequence and observe whether/when it is printend. + * 

+ *     { log: { message: "starting 'myBehavior' sequence"} }
+ *
+ * You may need to adjust the logging configuration to make logs on 'DEBUG' level from the {@link LogAction} class visible. */ @API @BehaviorAction(name = "log") public class LogAction extends BaseAction { - public static final Logger logger = LoggerFactory.getLogger(LogAction.class - ); + public static final Logger logger = LoggerFactory.getLogger(LogAction.class); @TextField public String message; @@ -30,7 +35,7 @@ public void construct(Actor actor) { @Override public BehaviorState modify(Actor actor, BehaviorState result) { - logger.debug(String.format("Actor %s logs message: %s ", actor.getEntity().toString(), actor.getValue(getId()))); + logger.debug("Actor {}: {}", actor.getEntity().getId(), actor.getValue(getId())); return BehaviorState.SUCCESS; } } diff --git a/src/main/java/org/terasology/module/behaviors/actions/MoveAlongPathNode.java b/src/main/java/org/terasology/module/behaviors/actions/MoveAlongPathNode.java index 583b5a0d..0262ee4e 100644 --- a/src/main/java/org/terasology/module/behaviors/actions/MoveAlongPathNode.java +++ b/src/main/java/org/terasology/module/behaviors/actions/MoveAlongPathNode.java @@ -2,6 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 package org.terasology.module.behaviors.actions; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.terasology.engine.logic.behavior.BehaviorAction; import org.terasology.engine.logic.behavior.core.Actor; import org.terasology.engine.logic.behavior.core.BaseAction; @@ -24,12 +26,17 @@ @BehaviorAction(name = "move_along_path", isDecorator = true) public class MoveAlongPathNode extends BaseAction { + private static final Logger logger = LoggerFactory.getLogger(MoveAlongPathNode.class); + @Override public BehaviorState modify(Actor actor, BehaviorState result) { + logger.debug("Actor {}: in move_along_path Action", actor.getEntity().getId()); MinionMoveComponent movement = actor.getComponent(MinionMoveComponent.class); if (result == BehaviorState.SUCCESS) { + logger.debug("... [{}]: advance path", actor.getEntity().getId()); movement.advancePath(); if (movement.isPathFinished()) { + logger.debug("... [{}]: finished", actor.getEntity().getId()); movement.resetPath(); actor.save(movement); return BehaviorState.SUCCESS; diff --git a/src/main/java/org/terasology/module/behaviors/actions/MoveToAction.java b/src/main/java/org/terasology/module/behaviors/actions/MoveToAction.java index 55070c70..f6454cae 100644 --- a/src/main/java/org/terasology/module/behaviors/actions/MoveToAction.java +++ b/src/main/java/org/terasology/module/behaviors/actions/MoveToAction.java @@ -16,7 +16,8 @@ import org.terasology.engine.registry.CoreRegistry; import org.terasology.engine.registry.In; import org.terasology.engine.world.WorldProvider; -import org.terasology.engine.world.block.Blocks; +import org.terasology.engine.world.block.BlockRegion; +import org.terasology.joml.geom.AABBf; import org.terasology.module.behaviors.components.MinionMoveComponent; import org.terasology.module.behaviors.plugin.MovementPlugin; import org.terasology.module.behaviors.plugin.WalkingMovementPlugin; @@ -55,6 +56,7 @@ public void construct(Actor actor) { @Override public BehaviorState modify(Actor actor, BehaviorState prevResult) { + logger.debug("Actor {}: in move_to Action", actor.getEntity().getId()); LocationComponent location = actor.getComponent(LocationComponent.class); MinionMoveComponent minionMoveComponent = actor.getComponent(MinionMoveComponent.class); CharacterMovementComponent characterMovementComponent = actor.getComponent(CharacterMovementComponent.class); @@ -63,16 +65,26 @@ public BehaviorState modify(Actor actor, BehaviorState prevResult) { // in practice we just need to adjust the Y so that it's resting on top of the block at the right height Vector3f adjustedMoveTarget = new Vector3f(minionMoveComponent.target); - // this is the result of experimentation and some penwork - // float adjustedY = (float) Math.ceil(adjustedMoveTarget.y - halfHeight) + halfHeight - 0.5f; - // adjustedMoveTarget.setY(adjustedY); - Vector3f position = location.getWorldPosition(new Vector3f()); - if (Blocks.toBlockPos(position).equals(minionMoveComponent.target)) { + + // Make the target area a bit smaller than the actual block to ensure that the entity + // is well within the block boundaries before moving on. + float tolerance = minionMoveComponent.targetTolerance; + AABBf targetArea = new BlockRegion(minionMoveComponent.target) + .getBounds(new AABBf()) + .expand(-tolerance, -tolerance, -tolerance); + if (targetArea.containsPoint(position)) { return BehaviorState.SUCCESS; } - // Cannot find path too long; - if (minionMoveComponent.sequenceNumber > 200) { + // Could not reach target with _n_ movements - abort action; + // + // As we scale the movement with the tick rate (the game time delta) just counting the + // sequence number might be a bad measure for long-running/slow movements. + // With about 200fps this would allow for just 1 second of consecutive movement with an + // upper bound of 200 cycles. Therefore, increasing this number to have a bit more margin. + // + //TODO: find a better measure for "time outs" and identifying long-running actions + if (minionMoveComponent.sequenceNumber > 1000) { minionMoveComponent.resetPath(); actor.save(minionMoveComponent); return BehaviorState.FAILURE; @@ -93,7 +105,7 @@ public BehaviorState modify(Actor actor, BehaviorState prevResult) { // path is no longer valid. However, we instead fall back to a default movement plugin in the hopes // that a gentle nudge in a probably-correct direction will at least make the physics reconcile the // intersection, and hopefully return to properly penetrable blocks. - logger.debug("Movement plugin returned null"); + logger.debug("... [{}]: Movement plugin returned null - falling back to WalkingMovementPlugin", actor.getEntity().getId()); MovementPlugin fallbackPlugin = new WalkingMovementPlugin(world, time); result = fallbackPlugin.move( actor.getEntity(), diff --git a/src/main/java/org/terasology/module/behaviors/actions/SetTargetToNearbyBlockAwayFromInstigatorAction.java b/src/main/java/org/terasology/module/behaviors/actions/SetTargetToNearbyBlockAwayFromInstigatorAction.java index 9d07fb23..966a98d5 100644 --- a/src/main/java/org/terasology/module/behaviors/actions/SetTargetToNearbyBlockAwayFromInstigatorAction.java +++ b/src/main/java/org/terasology/module/behaviors/actions/SetTargetToNearbyBlockAwayFromInstigatorAction.java @@ -1,4 +1,4 @@ -// Copyright 2021 The Terasology Foundation +// Copyright 2022 The Terasology Foundation // SPDX-License-Identifier: Apache-2.0 package org.terasology.module.behaviors.actions; @@ -14,6 +14,7 @@ import org.terasology.engine.logic.behavior.core.BaseAction; import org.terasology.engine.logic.behavior.core.BehaviorState; import org.terasology.engine.logic.location.LocationComponent; +import org.terasology.engine.registry.CoreRegistry; import org.terasology.engine.registry.In; import org.terasology.engine.world.block.BlockRegion; import org.terasology.engine.world.block.Blocks; @@ -31,11 +32,21 @@ public class SetTargetToNearbyBlockAwayFromInstigatorAction extends BaseAction { private static final Logger logger = LoggerFactory.getLogger(SetTargetToNearbyBlockAwayFromInstigatorAction.class); + private static final int RANDOM_BLOCK_ITERATIONS = 10; + private final Random random = new Random(); + @In private PluginSystem movementPluginSystem; + @Override + public void construct(Actor actor) { + if (movementPluginSystem == null) { + movementPluginSystem = CoreRegistry.get(PluginSystem.class); + } + } + @Override public BehaviorState modify(Actor actor, BehaviorState state) { MinionMoveComponent moveComponent = actor.getComponent(MinionMoveComponent.class); diff --git a/src/main/java/org/terasology/module/behaviors/actions/SetTargetToNearbyBlockNode.java b/src/main/java/org/terasology/module/behaviors/actions/SetTargetToNearbyBlockNode.java index e550f007..26af4855 100644 --- a/src/main/java/org/terasology/module/behaviors/actions/SetTargetToNearbyBlockNode.java +++ b/src/main/java/org/terasology/module/behaviors/actions/SetTargetToNearbyBlockNode.java @@ -1,4 +1,4 @@ -// Copyright 2020 The Terasology Foundation +// Copyright 2022 The Terasology Foundation // SPDX-License-Identifier: Apache-2.0 package org.terasology.module.behaviors.actions; @@ -13,6 +13,7 @@ import org.terasology.engine.logic.behavior.core.BaseAction; import org.terasology.engine.logic.behavior.core.BehaviorState; import org.terasology.engine.logic.location.LocationComponent; +import org.terasology.engine.registry.CoreRegistry; import org.terasology.engine.registry.In; import org.terasology.engine.world.block.BlockRegion; import org.terasology.engine.world.block.BlockRegionc; @@ -27,47 +28,81 @@ @BehaviorAction(name = "set_target_nearby_block") public class SetTargetToNearbyBlockNode extends BaseAction { private static final Logger logger = LoggerFactory.getLogger(SetTargetToNearbyBlockNode.class); + private int moveProbability = 100; private transient Random random = new Random(); @In private PluginSystem movementPluginSystem; + @Override + public void construct(Actor actor) { + if (movementPluginSystem == null) { + movementPluginSystem = CoreRegistry.get(PluginSystem.class); + } + } + @Override public BehaviorState modify(Actor actor, BehaviorState result) { + logger.debug("Actor {}: in set_target_nearby_block Action", actor.getEntity().getId()); if (random.nextInt(100) > (99 - moveProbability)) { MinionMoveComponent moveComponent = actor.getComponent(MinionMoveComponent.class); LocationComponent locationComponent = actor.getComponent(LocationComponent.class); JPSPlugin plugin = movementPluginSystem.getMovementPlugin(actor.getEntity()) .getJpsPlugin(actor.getEntity()); if (locationComponent != null) { - Vector3ic target = findRandomNearbyBlock( - Blocks.toBlockPos(locationComponent.getWorldPosition(new Vector3f())), - plugin); + Vector3i startBlock = Blocks.toBlockPos(locationComponent.getWorldPosition(new Vector3f())); + logger.debug("... [{}] start position: {}", actor.getEntity().getId(), startBlock); + Vector3ic target = findRandomNearbyBlock(startBlock, plugin); + if (!plugin.isWalkable(target)) { + logger.debug("... [{}] target position: {} not walkable", actor.getEntity().getId(), target); + return BehaviorState.FAILURE; + } moveComponent.target.set(target); + moveComponent.setPathGoal(new Vector3i(target)); actor.save(moveComponent); + logger.debug("... [{}] target position: {} - distance: {}", actor.getEntity().getId(), target, target.sub(startBlock, new Vector3i()).length()); } else { + logger.debug("... [{}] failed", actor.getEntity().getId()); return BehaviorState.FAILURE; } } return BehaviorState.SUCCESS; } + /** + * TODO: make sure the block is on the ground, i.e., the actor is able to stay at at that location + * obviously, this depends on the entities movement modes, a flying entity can obviously just hover a round... + * => the question now is how a plugin can contribute to the path, but should be excluded from finding a target ... + * + * TODO: can this be solved using different sets of plugins? + * movementTypes: ["walking", "leaping"] + * idleTypes: ["walking"] + * ideally, we can instantiate all plugins once an re-use them in different contexts (but that might prove difficult due to + * concurrency when building up caches?) + * + * @param startBlock + * @param plugin + * @return + */ private Vector3ic findRandomNearbyBlock(Vector3ic startBlock, JPSPlugin plugin) { Vector3i currentBlock = new Vector3i(startBlock); + long currentDistance = 0; + // general distance to startBlock is defined by number of steps (currently at least 3, at max 13) for (int i = 0; i < random.nextInt(10) + 3; i++) { BlockRegionc neighbors = new BlockRegion(currentBlock).expand(1, 1, 1); Vector3ic[] allowedBlocks = Iterators.toArray( Iterators.transform(Iterators.filter(neighbors.iterator(), - (candidate) -> plugin.isReachable(currentBlock, candidate)), + (candidate) -> candidate.distanceSquared(startBlock) > currentDistance + && plugin.isReachable(currentBlock, candidate)), Vector3i::new), Vector3ic.class); if (allowedBlocks.length > 0) { currentBlock.set(allowedBlocks[random.nextInt(allowedBlocks.length)]); } } - logger.debug(String.format("Looking for a block: my block is %s, found destination %s", - startBlock, currentBlock)); + + // TODO: keep history of steps and trace back in case final bock is not walkable return currentBlock; } diff --git a/src/main/java/org/terasology/module/behaviors/components/MinionMoveComponent.java b/src/main/java/org/terasology/module/behaviors/components/MinionMoveComponent.java index a02f5252..40057964 100644 --- a/src/main/java/org/terasology/module/behaviors/components/MinionMoveComponent.java +++ b/src/main/java/org/terasology/module/behaviors/components/MinionMoveComponent.java @@ -14,58 +14,127 @@ public final class MinionMoveComponent implements Component { - // acceptable distance from goal for completion - public double pathGoalDistance = 0; + //TODO: why do we consider this to be a good default? + // would it make sense to modify the walkingPlugin in a way that it allows to "leap" 1 block up by default (without requiring the + // leaping plugin?) the fact that we have to "leap" is mostly just due to being a blocky world, but actually we're "walking up the + // hill", not "jumping up the hill" (kinda like minecraft's auto-step-up feature, just for mobs). + public List movementTypes = Lists.newArrayList("walking", "leaping", "falling"); - public List movementTypes = Lists.newArrayList("walking", "leaping"); public boolean collidedHorizontally; public float lastInput; public int sequenceNumber; + // immediate movement target + //TODO: why is this public? who should change this? + // it feels like this is actually an INTERNAL control value, and having behaviors or systems tinker with like they do now can + // easily mess up any guarantees we would like to give for the fields on this component public Vector3i target = new Vector3i(); + /** + * The maximum distance from the final goal before it is considered to be reached. + * + * TODO: should we use double or float here (cf. targetTolerance) + */ + public double goalTolerance = 0; + /** * The maximum distance from a target before it is considered to be reached */ - public float targetTolerance = 0.2f; + public float targetTolerance = 0.1f; + + // last known goal position + private Vector3i goalPosition = new Vector3i(); // an entity to take the goal position from + //TODO(skaldarnar): What's the difference between MinionMoveComponent#pathGoalEntity and FollowComponent#entityToFollow? + // How do we keep them in sync? Where are systems involved, and where to behaviors update respective fields? + // Personally, I don't think this should be here... private EntityRef pathGoalEntity = null; - // last known goal position - private Vector3i pathGoalPosition = new Vector3i(); - - // generated path to goal + /** + * The path of reachable waypoints to the {@link #goalPosition}. + * + * If empty no path to the target exists, or it was not computed yet. + * If the {@link #goalPosition} is reachable, the path will contain at least that position as last element. + */ private List path = Lists.newArrayList(); // current index along path above private int pathIndex = 0; + //TODO: pathfinding related fields + // goalPosition : final target position + // goalTolerance : maximum distance from 'goalPosition' + // target : next waypoint to move to (from 'path', eventually 'goalPosition' + // targetTolerance : maximum distance from 'target' + // path : list of waypoints to reach 'goal' + // pathIndex : current position (or next target position?) + + //TODO: don't mention `pathIndex` in docstrings as they should be an internal detail? + + /** + * Denote that the given {@code entity}'s location determines the final movement goal. + * + * This will clear the current {@link #path} and reset the {@link #pathIndex}. + * + * Note: Setting the path goal to an entity does NOT compute a path or set the {@link #goalPosition} vector. Only {@link #getPathGoal()} + * will take the entity into consideration when computing the actual path goal. + * + * @param entity the entity to move to; must have a {@link LocationComponent} + */ public void setPathGoal(EntityRef entity) { + //TODO: ensure that entity is not EntityRef.NULL? pathGoalEntity = entity; resetPath(); } + /** + * Set the final movement goal to the given {@code pos}. + * + * This will clear the current {@link #path} and reset the {@link #pathIndex}. It will also set the {@link #pathGoalEntity} to {@code null}. + * + * Note: Setting the path goal does NOT compute a path. + * + * @param pos the final movement goal position + */ public void setPathGoal(Vector3i pos) { - pathGoalPosition.set(pos); + goalPosition.set(pos); pathGoalEntity = null; resetPath(); } + /** + * Retrieve the final movement goal position. + * + * If the {@link #pathGoalEntity} is set, the goal position is derived from the entity's location. + * Otherwise, the goal position is {@link #goalPosition}. + * + * @return the current goal position + */ public Vector3i getPathGoal() { + //TODO: this is logic on component, doing a lookup on a different entity. + // this is definitely not the recommended way to handle components and dependencies between entities. if (pathGoalEntity != null && pathGoalEntity.getComponent(LocationComponent.class) != null) { Vector3f worldPosition = pathGoalEntity.getComponent(LocationComponent.class).getWorldPosition(new Vector3f()); Vector3i pos = Blocks.toBlockPos(worldPosition); - pathGoalPosition.set(pos); + goalPosition.set(pos); } - return pathGoalPosition; + return goalPosition; } + /** + * Clear the stored {@link #path} and reset the {@link #pathIndex} to 0. + */ public void resetPath() { path.clear(); pathIndex = 0; } + /** + * Increment the {@link #pathIndex} by one and set the next {@link #target}, if it exists. + * + * TODO: should this just include whether the path is finished check? + */ public void advancePath() { pathIndex += 1; if (pathIndex < path.size()) { @@ -73,10 +142,24 @@ public void advancePath() { } } + /** + * Whether the entity reached the end of the {@link #path}. + * + * Note: finishing the path does not necessarily mean that the entity reached the goal position! + */ public boolean isPathFinished() { return pathIndex >= path.size(); } + /** + * Store given path and reset {@link #pathIndex} pointer. + * Also set {@link #target} to the first waypoint if the given path is not empty. + * + * TODO: Why reset pathIndex, but not the overall pathGoal? Or at least ensure that the given path leads to pathGoal? + * However, pathGoal may be irrelevant if pathGoalEntity is set... this is confusing... + * + * @param path the new path the entity should move along. + */ public void setPath(List path) { resetPath(); this.path.addAll(path); @@ -98,8 +181,8 @@ public void copyFrom(MinionMoveComponent other) { target.set(other.target); targetTolerance = other.targetTolerance; pathGoalEntity = other.pathGoalEntity; - pathGoalPosition.set(other.pathGoalPosition); - pathGoalDistance = other.pathGoalDistance; + goalPosition.set(other.goalPosition); + goalTolerance = other.goalTolerance; path = other.path; pathIndex = other.pathIndex; //TODO change me when migrate JOML movementTypes.clear(); diff --git a/src/main/java/org/terasology/module/behaviors/debug/FlexibleMovementDebugRenderSystem.java b/src/main/java/org/terasology/module/behaviors/debug/FlexibleMovementDebugRenderSystem.java index e984189e..976857ea 100644 --- a/src/main/java/org/terasology/module/behaviors/debug/FlexibleMovementDebugRenderSystem.java +++ b/src/main/java/org/terasology/module/behaviors/debug/FlexibleMovementDebugRenderSystem.java @@ -30,6 +30,10 @@ public void initialise() { @Override public void renderOverlay() { + //TODO: Only render the path debug overlay if the debug mode is enabled. + // The RenderingDebugConfig holds more specific flags which debug modes are enabled, but that config is not extensible. + // How would we register new debug modes like this one for pathfinding? Should this be tied to debug mode in general, or + // should it be possible to toggle the debug rendering via console command, similar to RenderingDebugCommands? selectionRenderer.beginRenderOverlay(); for (EntityRef entity : entityManager.getEntitiesWith(MinionMoveComponent.class)) { MinionMoveComponent minionMoveComponent = entity.getComponent(MinionMoveComponent.class); diff --git a/src/main/java/org/terasology/module/behaviors/plugin/LeapingMovementPlugin.java b/src/main/java/org/terasology/module/behaviors/plugin/LeapingMovementPlugin.java index b95979a5..c348a045 100644 --- a/src/main/java/org/terasology/module/behaviors/plugin/LeapingMovementPlugin.java +++ b/src/main/java/org/terasology/module/behaviors/plugin/LeapingMovementPlugin.java @@ -1,4 +1,4 @@ -// Copyright 2021 The Terasology Foundation +// Copyright 2022 The Terasology Foundation // SPDX-License-Identifier: Apache-2.0 package org.terasology.module.behaviors.plugin; @@ -45,16 +45,17 @@ public CharacterMoveInputEvent move(EntityRef entity, Vector3fc dest, int sequen dest); } - Vector3f delta = getDelta(entity, dest); - float yaw = getYaw(delta); - long dt = getTime().getGameDeltaInMs(); - CharacterMovementComponent movement = entity.getComponent(CharacterMovementComponent.class); // The underlying WalkingPlugin assumes that entities are not affected by gravity. // To simulate this, we'll use the FLYING movement mode for all entities when moving them with this plugin. if (movement.mode != MovementMode.FLYING) { entity.send(new SetMovementModeEvent(MovementMode.FLYING)); } + + Vector3f delta = getDelta(entity, dest); + float yaw = getYaw(delta); + long dt = getTime().getGameDeltaInMs(); + return new CharacterMoveInputEvent(sequence, 0, yaw, delta, false, false, true, dt); } } diff --git a/src/main/java/org/terasology/module/behaviors/plugin/WalkingMovementPlugin.java b/src/main/java/org/terasology/module/behaviors/plugin/WalkingMovementPlugin.java index 7227d673..b3e27c3d 100644 --- a/src/main/java/org/terasology/module/behaviors/plugin/WalkingMovementPlugin.java +++ b/src/main/java/org/terasology/module/behaviors/plugin/WalkingMovementPlugin.java @@ -1,4 +1,4 @@ -// Copyright 2021 The Terasology Foundation +// Copyright 2022 The Terasology Foundation // SPDX-License-Identifier: Apache-2.0 package org.terasology.module.behaviors.plugin; @@ -18,7 +18,8 @@ import org.terasology.flexiblepathfinding.plugins.basic.WalkingPlugin; public class WalkingMovementPlugin extends MovementPlugin { - private static final Logger logger = LoggerFactory.getLogger(LeapingMovementPlugin.class); + + private static final Logger logger = LoggerFactory.getLogger(WalkingMovementPlugin.class); public WalkingMovementPlugin(WorldProvider world, Time time) { super(world, time); @@ -45,10 +46,6 @@ public CharacterMoveInputEvent move(EntityRef entity, Vector3fc dest, int sequen dest); } - Vector3f delta = getDelta(entity, dest); - float yaw = getYaw(delta); - long dt = getTime().getGameDeltaInMs(); - CharacterMovementComponent movement = entity.getComponent(CharacterMovementComponent.class); // The underlying WalkingPlugin assumes that entities are not affected by gravity. // To simulate this, we'll use the FLYING movement mode for all entities when moving them with this plugin. @@ -56,6 +53,10 @@ public CharacterMoveInputEvent move(EntityRef entity, Vector3fc dest, int sequen entity.send(new SetMovementModeEvent(MovementMode.FLYING)); } - return new CharacterMoveInputEvent(sequence, 0, yaw, delta, false, false, dt); + Vector3f delta = getDelta(entity, dest); + float yaw = getYaw(delta); + long dt = getTime().getGameDeltaInMs(); + + return new CharacterMoveInputEvent(sequence, 0, yaw, delta, false, false, false, dt); } } diff --git a/src/main/java/org/terasology/module/behaviors/systems/PluginSystem.java b/src/main/java/org/terasology/module/behaviors/systems/PluginSystem.java index 7a436d78..7caa163b 100644 --- a/src/main/java/org/terasology/module/behaviors/systems/PluginSystem.java +++ b/src/main/java/org/terasology/module/behaviors/systems/PluginSystem.java @@ -1,4 +1,4 @@ -// Copyright 2021 The Terasology Foundation +// Copyright 2022 The Terasology Foundation // SPDX-License-Identifier: Apache-2.0 package org.terasology.module.behaviors.systems;