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

Pathfinding improvements #42

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Pathfinding improvements #42

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e475d05
Added bug fix
agent-q1 Jun 30, 2020
1e49e1e
feat: Added some fixes to getBlock()
agent-q1 Jul 20, 2020
fe40e57
Merge branch 'develop' of https://github.com/Terasology/Pathfinding i…
agent-q1 Jul 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions src/main/java/org/terasology/navgraph/NavGraphSystem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import java.util.HashMap;
import java.util.Map;

import static org.joml.Math.round;

@RegisterSystem
@Share(value = NavGraphSystem.class)
public class NavGraphSystem extends BaseComponentSystem implements UpdateSubscriberSystem, WorldChangeListener {
Expand Down Expand Up @@ -106,14 +108,33 @@ public WalkableBlock getBlock(EntityRef minion) {
}

public WalkableBlock getBlock(Vector3f pos) {
Vector3i blockPos = new Vector3i(pos.x + 0.25f, pos.y, pos.z + 0.25f);
Vector3i blockPos = new Vector3i(round(pos.x), round(pos.y), round(pos.z));
Copy link

Choose a reason for hiding this comment

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

This change looks correct. The engine internally uses RoundingMode.HALF_UP to convert V3f to V3i.

blockPos.y += 2; //Added in case the height of minion is really low
WalkableBlock block = getBlock(blockPos);
if (block == null) {
while (blockPos.y >= (int) pos.y - 4 && block == null) {
blockPos.y--;
block = getBlock(blockPos);
if (block == null) {
block = getBlock(blockPos);
}

// Checking Neighbours as minion could be hanging on the edge of a block

for (int i = 0; i < 8; i++) {
int dx = NavGraphChunk.DIRECTIONS[i][0];
int dz = NavGraphChunk.DIRECTIONS[i][1];
Vector3i directionVector = new Vector3i(dx, 0, dz);
directionVector.add(blockPos);
block = getBlock(directionVector);
Copy link

@kaen kaen Jul 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This 100% of the time overwrites what's assigned in line 117. I think that means two things:

  1. 117 could simply be removed.
  2. This will never return the WalkableBlock directly under the NPC, only one offset by the first of the eight directions to match it.

If I'm wrong there just correct me.

I think this method should prefer the block directly underneath if it can find one. I also think it should only search the surrounding block closest to the entity. Consider an NPC standing on the very corner A of a 1x1 pillar. If the opposite corner C happens to be checked first then this method will return C, which in the worst case could have a very different navgraph result.

As written I think this section adds more bugs than it fixes. The following psuedo code might do better.

while (same condition as above)
  result = getBlock(blockPos);
  if result != null
    break

  offset = entityPos - blockPos
  offset.y = 0
  offset = Math.round(offset)
  offsetPos = blockPos + offset

  result = getBlock(offsetPos)

if (block != null) {
break;
}
}


}
}


return block;
}

Expand All @@ -131,13 +152,15 @@ public NavGraphChunk updateChunk(Vector3i chunkPos) {
}
NavGraphChunk navGraphChunk = heightMaps.remove(chunkPos);
if (navGraphChunk != null) {
navGraphChunk.disconnectNeighborMaps(getNeighbor(chunkPos, -1, 0), getNeighbor(chunkPos, 0, -1), getNeighbor(chunkPos, 1, 0), getNeighbor(chunkPos, 0, 1));
navGraphChunk.disconnectNeighborMaps(getNeighbor(chunkPos, -1, 0), getNeighbor(chunkPos, 0, -1),
getNeighbor(chunkPos, 1, 0), getNeighbor(chunkPos, 0, 1));
navGraphChunk.cells = null;
}
navGraphChunk = new NavGraphChunk(world, chunkPos);
navGraphChunk.update();
heightMaps.put(chunkPos, navGraphChunk);
navGraphChunk.connectNeighborMaps(getNeighbor(chunkPos, -1, 0), getNeighbor(chunkPos, 0, -1), getNeighbor(chunkPos, 1, 0), getNeighbor(chunkPos, 0, 1));
navGraphChunk.connectNeighborMaps(getNeighbor(chunkPos, -1, 0), getNeighbor(chunkPos, 0, -1),
getNeighbor(chunkPos, 1, 0), getNeighbor(chunkPos, 0, 1));
return navGraphChunk;
}

Expand Down