Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Listen for path calculation in FindPathToNode #18

Merged
merged 4 commits into from Aug 10, 2019
Merged

Listen for path calculation in FindPathToNode #18

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a5fc850
Listen for path calculation in FindPathToNode
AndyTechGuy Jul 9, 2019
cdce874
Add temporary fix for PathRenderSystem injection
AndyTechGuy Jul 21, 2019
933a75a
Pathfinding changes from mentor review
AndyTechGuy Jul 29, 2019
5fe037a
Reset old path in construct() of FindPathToNode
AndyTechGuy Aug 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 18 additions & 9 deletions src/main/java/org/terasology/minion/move/FindPathToNode.java
View file
Open in desktop
Expand Up @@ -15,7 +15,10 @@
*/
package org.terasology.minion.move;

import com.google.gson.annotations.Expose;
import com.google.common.util.concurrent.FutureCallback;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.SettableFuture;
import org.terasology.logic.behavior.BehaviorAction;
import org.terasology.logic.behavior.core.Actor;
import org.terasology.logic.behavior.core.BaseAction;
Expand All @@ -29,6 +32,7 @@
import org.terasology.registry.In;

import java.util.Arrays;
import java.util.List;

/**
* Requests a path to a target defined using the <b>MinionMoveComponent.target</b>.<br/>
Expand Down Expand Up @@ -68,21 +72,26 @@ public void construct(final Actor actor) {
moveComponent.path = Path.INVALID;
return;
}
pathfinderSystem.requestPath(
SettableFuture<List<Path>> pathFuture = pathfinderSystem.requestPath(
actor.getEntity(), currentBlock.getBlockPosition(),
Arrays.asList(workTarget.getBlockPosition()));
/*, new PathfinderSystem.PathReadyCallback() {
@Override
public void pathReady(int pathId, List<Path> path, WalkableBlock target, List<WalkableBlock> start) {

if (path == null) {
Futures.addCallback(pathFuture, new FutureCallback<List<Path>>() {
@Override
public void onSuccess(List<Path> paths) {
if (paths == null) {
moveComponent.path = Path.INVALID;
} else if (path.size() > 0) {
moveComponent.path = path.get(0);
} else if (paths.size() > 0) {
moveComponent.path = paths.get(0);
}
actor.save(moveComponent);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that this call is either thread-safe or always called on the main thread?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that it is? I changed the function to explicitly use a direct executor just to be save. This way, this call either runs directly in the action, or from pathfinding when the path is calculated, as found here: https://stackoverflow.com/questions/35211331/in-which-thread-futurecallback-will-be-called-for-a-listenablefuture-in-guava (this answer is for ListenableFuture instead of SettableFuture, but they are both implementations of Future so i'm assuming the executors work the same way)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay, I get it now. I wondered why you even put it in a Future for the same thread if you could just call it directly. But the pathfinderSystem seems to force you to use the Future approach. A bit odd, but ok.

}
});*/

@Override
public void onFailure(Throwable t) {
moveComponent.path = Path.INVALID;
}
}, MoreExecutors.directExecutor());
}

@Override
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/org/terasology/minion/move/MoveAlongPathNode.java
View file
Open in desktop
Expand Up @@ -26,6 +26,7 @@
import org.terasology.navgraph.WalkableBlock;
import org.terasology.pathfinding.componentSystem.PathRenderSystem;
import org.terasology.pathfinding.model.Path;
import org.terasology.registry.CoreRegistry;
import org.terasology.registry.In;

/**
Expand All @@ -40,11 +41,14 @@
public class MoveAlongPathNode extends BaseAction {
private static final Logger logger = LoggerFactory.getLogger(MoveAlongPathNode.class);

@In
// @In
private transient PathRenderSystem pathRenderSystem;

@Override
public void construct(Actor actor) {
// TODO: Temporary fix for injection malfunction in actions, ideally remove this in the future.
pathRenderSystem = CoreRegistry.get(PathRenderSystem.class);
AndyTechGuy marked this conversation as resolved.
Show resolved Hide resolved

MinionMoveComponent moveComponent = actor.getComponent(MinionMoveComponent.class);
if (moveComponent != null && moveComponent.path != null && moveComponent.path != Path.INVALID) {
pathRenderSystem.addPath(moveComponent.path);
Expand Down