docs: investigate and document Actions, Behaviors, and Components

src/main/java/org/terasology/behaviors/components/FindNearbyPlayersComponent.java

@@ -15,9 +15,11 @@
 public class FindNearbyPlayersComponent implements Component<FindNearbyPlayersComponent> {
     /* Search radius for finding nearby players */
     public float searchRadius = 10f;
+
     /* List of player entities nearby */
     public List<EntityRef> charactersWithinRange;
-    /* The player entity closest to the actor */
+
+    /* The player entity closest to the entity */
     public EntityRef closestCharacter;
 
     @Override

src/main/java/org/terasology/behaviors/system/FindNearbyPlayersSystem.java

@@ -26,31 +26,38 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
+/**
+ * Update all {@link FindNearbyPlayersComponent}s on active entities with information about alive players within range and which of them is closest to the entity.
+ * <br>
+ * The update frequency is once per game tick.
+ */
 @RegisterSystem(RegisterMode.AUTHORITY)
 public class FindNearbyPlayersSystem extends BaseComponentSystem implements UpdateSubscriberSystem {
 
-
     private static final Logger logger = LoggerFactory.getLogger(FindNearbyPlayersSystem.class);
 
     @In
     private EntityManager entityManager;
 
     @Override
     public void update(float delta) {
-        Iterable<EntityRef> clients = entityManager.getEntitiesWith(ClientComponent.class);
+        //TODO: Update with lower frequency.
+        //      Updating the nearby entities every 200ms should be enough from a gameplay perspective and we can safe some
+        //      computing cycles every tick that way.
+
         Map<Vector3f, EntityRef> clientLocationMap = new HashMap<>();
 
-        for (EntityRef client : clients) {
+        for (EntityRef client : entityManager.getEntitiesWith(ClientComponent.class)) {
             ClientComponent clientComponent = client.getComponent(ClientComponent.class);
             EntityRef character = clientComponent.character;
-            AliveCharacterComponent aliveCharacterComponent = character.getComponent(AliveCharacterComponent.class);
-            if (aliveCharacterComponent == null) {
+            if (!character.hasComponent(AliveCharacterComponent.class)) {
                 continue;
             }
             LocationComponent locationComponent = character.getComponent(LocationComponent.class);
             if (locationComponent == null) {
                 continue;
             }
+
             clientLocationMap.put(locationComponent.getWorldPosition(new Vector3f()), character);
         }
         Set<Vector3f> locationSet = clientLocationMap.keySet();
@@ -61,6 +68,8 @@ public void update(float delta) {
             float maxDistance = findNearbyPlayersComponent.searchRadius;
             float maxDistanceSquared = maxDistance * maxDistance;
 
+            //TODO: Use a (shortest) path distance to determine "nearest" player instead of beeline distance.
+            //      At least make it configurable via the FindNearbyPlayersComponent whether to use path or beeline.
             List<Vector3f> inRange = locationSet.stream()
                     .filter(loc -> loc.distanceSquared(actorPosition) <= maxDistanceSquared)
                     .sorted(Comparator.comparingDouble(v3f -> v3f.distanceSquared(actorPosition)))
@@ -74,6 +83,8 @@ public void update(float delta) {
 
             List<EntityRef> charactersWithinRange = inRange.stream().map(clientLocationMap::get).collect(Collectors.toList());
 
+            //TODO: If the order of entities in the list has a meaning (e.g., closest to farthest) we should not check for set equality here.
+            //      If the order is not important, we should just store the nearby entities as set in the component directly.
             if (!isEqual(charactersWithinRange, findNearbyPlayersComponent.charactersWithinRange)) {
                 findNearbyPlayersComponent.charactersWithinRange = charactersWithinRange;
                 findNearbyPlayersComponent.closestCharacter = charactersWithinRange.get(0);
@@ -82,6 +93,9 @@ public void update(float delta) {
         }
     }
 
+    /**
+     * Compares two lists of entities with set-equality, i.e., whether they contain the same entities (in any order).
+     */
     private boolean isEqual(List<EntityRef> one, List<EntityRef> two) {
         if ((one == null && two != null) || (one != null && two == null)) {
             return false;

src/main/java/org/terasology/minion/move/ContinuousMoveAlongPathNode.java

@@ -10,10 +10,12 @@
 import org.terasology.navgraph.WalkableBlock;
 
 /**
- * Call child node, as long as the actor has not reached the end of the path. Sets <b>MinionMoveComponent.target</b> to
- * next step in path.<br/> Old construct() from original {@link MoveAlongPathNode} moved to
- * {@link SetupContinuousMoveNode} to allow for path travelling to be interrupted. This enables a character to follow
- * a moving target.
+ * Call child node, as long as the actor has not reached the end of the path. 
+ * <br/>
+ * Sets {@link MinionMoveComponent#target} to next step in path.
+ * <br/> 
+ * Old {@code construct()} from original {@link MoveAlongPathNode} moved to {@link SetupContinuousMoveNode} to allow for path travelling to be interrupted.
+ * This enables a character to follow a moving target.
  * <br/>
  * <b>SUCCESS</b>: when actor has reached end of path.<br/>
  * <b>FAILURE</b>: if no path was found previously.<br/>
@@ -22,6 +24,10 @@
 @BehaviorAction(name = "move_along_path_continuous", isDecorator = true)
 public class ContinuousMoveAlongPathNode extends BaseAction {
 
+    //TODO: What's the difference between ContinuousMoveAlongPathNode and MoveAlongPathNode and do we need both?
+    //      The comments indicate that 
+    //          (SetupContinuousMoveNode + ContinuousMoveAlongPathNode) == MoveAlongPathNode + interrupt
+
     @Override
     public BehaviorState modify(Actor actor, BehaviorState result) {
         MinionMoveComponent moveComponent = actor.getComponent(MinionMoveComponent.class);

src/main/java/org/terasology/minion/move/MinionMoveComponent.java

@@ -10,6 +10,10 @@
 
 public class MinionMoveComponent implements Component<MinionMoveComponent> {
 
+    //TODO: Can/should we get rid of this movement type?
+    //      If an actor should move along a path, we'll construct a (more complex) behavior like `dynamicPathfindingFollow.behavior`
+    //      in which we set the target, compute the path, setup the actor before moving, and eventually move the along the path by
+    //      subsequently calling the basic `move_to` child action.
     public enum Type {
         @SerializedName("direct")
         DIRECT,
@@ -26,8 +30,11 @@ public enum Type {
     public int currentIndex;
 
     public transient WalkableBlock currentBlock;
+    /** Whether the actor encountered a {@link org.terasology.engine.logic.characters.events.HorizontalCollisionEvent HorizontalCollisionEvent} recently. */
     public transient boolean horizontalCollision;
+    /** Whether the actor movement is supposed to be jumping. */
     public transient boolean jumpMode;
+    /** Time frame in which the actor is supposed to enter {@link #jumpMode} (if greater 0). */
     public transient float jumpCooldown;
 
     @Override

src/main/java/org/terasology/minion/move/MoveToAction.java

@@ -17,56 +17,81 @@
 import static org.joml.Math.abs;
 
 /**
- * <b>Properties:</b> <b>distance</b><br/>
- * <br/> Moves the actor to the target defined by <b>MinionMoveComponent</b>.<br/> <br/>
- * <b>SUCCESS</b>: when distance between actor and target is below <b>distance</b>.<br/>
- * <b>FAILURE</b>: when there is no target.<br/>
- * <br/> Auto generated javadoc - modify README.markdown instead!
+ * Moves the actor to the target defined by {@link MinionMoveComponent#target}.
+ * <br/>
+ * Note: This action only moves the actor if its movement type is {@link MinionMoveComponent.Type.DIRECT}!
+ * <br/>
+ * The actor movement is based on the properties of {@link MinionMoveComponent#target}, in particular:
+ *    - target                  The target position to move towards.
+ *    - type                    The means how to determine the best path or action to reach the target (currently only DIRECT supported)
+ *    - horizontalCollision     Whether the actor recently encountered a horizontal collision, e.g., walking into a wall
+ *    - jumpCooldown            ??? set to 0.3 in case a horizontal collision is detected and reduced by Actor#getDelta() each evaluation. I think the entity will attempt to jump for 300ms after hitting an obstacle...
+ *    - jumpMode                Whether the actor should jump on next movement.
  */
 @BehaviorAction(name = "move_to")
 public class MoveToAction extends BaseAction {
     private static Logger logger = LoggerFactory.getLogger(MoveToAction.class);
+
+    /** To complete the "move" action the distance between the actor and the target needs to be below {@code distance}. */
     @Range(min = 0, max = 10)
     private float distance = 0.2f;
 
-
+    /**
+     * @return {@link BehaviorState.SUCCESS} when distance between actor and target is below {@code distance},
+     *         {@link BehaviorState.FAILURE} when there is no target,
+     *         {@link BehaviorState.RUNNING} while moving towards the target.
+     */
     @Override
     public BehaviorState modify(Actor actor, BehaviorState result) {
         BehaviorState state = BehaviorState.FAILURE;
+
         MinionMoveComponent moveComponent = actor.getComponent(MinionMoveComponent.class);
 
         if (moveComponent.target == null) {
             return BehaviorState.FAILURE;
         }
-        if (moveComponent.type == MinionMoveComponent.Type.DIRECT) {
 
+        //TODO: also handle other movement types?
+        if (moveComponent.type == MinionMoveComponent.Type.DIRECT) {
             boolean reachedTarget = processDirect(actor, moveComponent);
             state = reachedTarget ? BehaviorState.SUCCESS : BehaviorState.RUNNING;
-
         }
+        //TODO: either make the null check before accessing the component, or just throw it out completely
         if (moveComponent != null && moveComponent.target != null) {
+            // handle horizontal collisions, e.g., when the entity hits a wall
             if (moveComponent.horizontalCollision) {
                 moveComponent.horizontalCollision = false;
                 moveComponent.jumpCooldown = 0.3f;
             }
             moveComponent.jumpCooldown -= actor.getDelta();
             moveComponent.jumpMode = moveComponent.jumpCooldown > 0;
             actor.save(moveComponent);
-
         }
         return state;
     }
 
+    /**
+     * Move the actor's entity by sending a {@link CharacterMoveInputEvent} in the direction of the target defined by {@link MinionMoveComponent#target}.
+     * <br/>
+     * The actor has reached it's target if it is within {@code distance} along X and Z axis (horizontal), and within a distance of 2 along Y axis (vertical). 
+     *
+     * @param actor             The actor to be moved. Must have a {@link LocationComponent}.
+     * @param moveComponent     The movement configuration for the actor.
+     *
+     * @return whether the actor reached the target (before the movement o.O)
+     */
     private boolean processDirect(Actor actor, MinionMoveComponent moveComponent) {
+        boolean reachedTarget = false;
 
         LocationComponent locationComponent = actor.getComponent(LocationComponent.class);
-        boolean reachedTarget = false;
         Vector3f worldPos = locationComponent.getWorldPosition(new Vector3f());
         Vector3f targetDirection = moveComponent.target.sub(worldPos, new Vector3f());
+
         Vector3f drive = new Vector3f();
         float yaw = (float) Math.atan2(targetDirection.x, targetDirection.z);
         float requestedYaw = (float) (180f + Math.toDegrees(yaw));
 
+        //TODO: why not do an early return when the target is reached without sending a CharacterMoveInputEvent?
         if (abs(targetDirection.x) < distance && (abs(targetDirection.y) < 2f) && (abs(targetDirection.z) < distance)) {
             drive.set(0, 0, 0);
             reachedTarget = true;
@@ -80,7 +105,6 @@ private boolean processDirect(Actor actor, MinionMoveComponent moveComponent) {
             moveComponent.jumpMode, (long) (actor.getDelta() * 1000));
         actor.getEntity().send(wantedInput);
 
-
         return reachedTarget;
     }
 

src/main/java/org/terasology/minion/move/SetupContinuousMoveNode.java

@@ -22,6 +22,7 @@ public class SetupContinuousMoveNode extends BaseAction {
     public BehaviorState modify(Actor actor, BehaviorState result) {
 
         MinionMoveComponent moveComponent = actor.getComponent(MinionMoveComponent.class);
+        //TODO: log something in case expected components or properties are missing
         if (moveComponent != null && moveComponent.path != null && moveComponent.path != Path.INVALID) {
             moveComponent.currentIndex = 0;
             WalkableBlock block = moveComponent.path.get(moveComponent.currentIndex);