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.

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

Pathfinding improvements #42

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

Conversation

agent-q1
Copy link
Contributor

Bug fixes

This PR lists a couple of improvements to the getBlock() function in Pathfinding which is intended to return the WalkableBlock at a specific position.

Bug 1

The Gooeys in MR are never actually able to calculate the path to target (Behavior returns path.INVALID) because the system is unable to locate the WalkableBlock where the gooey are present. I speculate the reason is that the heights of the Gooeys are really low and since in the original implementation of the getBlock(), it only checks all the blocks below the current pos (upto a depth of 4), it starts off underground and is never able to find a WalkableBlock. That is why, I have added the pos+=2 to ensure that it starts off a little higher.

Bug 2

Imagine a Gooey standing at the edge of a block that is at a height 2 (the ground is at height 0). If the gooey walked to the edge, then even if most of the gooey is actually not on the edge block at height 2, the gooey would still be at a height 2. Hence if we don't find a block at the current pos, it becomes important to check the surroundings.

In MetalRenegades, the height of the gooey is 1. Most of the time, the
NavGraphSystem would not be able to find a suitable WalkableBlock
(because it would be looking underground).
Since entities have a span, the exact block on which they are standing
cannot be ascertained directly by just checking the block. Even the
surrounding blocks need to be checked.
@@ -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.

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

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants