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

Allow block to specify their type for AI pathfinding #3546

Merged
merged 4 commits into from
Jan 12, 2017

Conversation

diesieben07
Copy link
Contributor

Allow blocks to specify their PathNodeType for AI pathfinding. This allows e.g. mod-added fences to behave like vanilla fences in respect to Mob AI.

+ * Set the PathNodeType for this block. null for vanilla behavior.
+ * @return the PathNodeType
+ */
+ @Nullable public net.minecraft.pathfinding.PathNodeType getAiPathNodeType()
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Annotation should be on a new line;
  2. At least this should be state-aware; consider exposing the block positon.

@@ -8,11 +8,13 @@
}
}
}
@@ -430,6 +431,7 @@
@@ -430,6 +431,9 @@
IBlockState iblockstate = p_189553_1_.func_180495_p(blockpos);
Block block = iblockstate.func_177230_c();
Material material = iblockstate.func_185904_a();
+ if(block.isBurning(p_189553_1_, blockpos)) return PathNodeType.DAMAGE_FIRE;
Copy link
Contributor

Choose a reason for hiding this comment

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

The new method should handle this check. Other opinions welcomed.

@mezz
Copy link
Contributor

mezz commented Dec 18, 2016

Looks like a good idea.

I think liach's requests make sense here.

@diesieben07
Copy link
Contributor Author

Updated as suggested.

+ * Set the PathNodeType for this block. null for vanilla behavior.
+ * @return the PathNodeType
+ */
+ @Nullable public net.minecraft.pathfinding.PathNodeType getAiPathNodeType(IBlockState state, IBlockAccess world, BlockPos pos)
Copy link
Contributor

Choose a reason for hiding this comment

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

All other methods in Forge project is like

@Nullable
public net.minecraft.pathfinding.PathNodeType getAiPathNodeType(IBlockState state, IBlockAccess world, BlockPos pos)

instead of putting annotation and the method head in the same line.

Copy link
Member

Choose a reason for hiding this comment

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

I assume diesieben just wanted to minimize the patch size by putting it all in the same line. A similar thing can be found here in commit 150566d.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but it doesn't really make sense here, the Block patch is a huge blob all all already anyways. I just forgot to fix this in my update.

@liach
Copy link
Contributor

liach commented Dec 18, 2016

I suggest making a test mod to show that this change works as intended. You should screenshot a picture like this. It is a picture from Minecraft Wiki that demonstrates the Zombies' AI.

@@ -1359,12 +1359,21 @@
+ return false;
+ }
+
+ /**
+ * Set the PathNodeType for this block. null for vanilla behavior.
Copy link
Contributor

@bs2609 bs2609 Dec 18, 2016

Choose a reason for hiding this comment

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

Suggestion for the comment:
Gets the PathNodeType for this block. Return null for vanilla behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

Returns

@diesieben07
Copy link
Contributor Author

Added a test mod, see attached image (zombie thinks the test block is an open door and tries to walk through it).

2016-12-18_23 42 58

@bs2609
Copy link
Contributor

bs2609 commented Dec 21, 2016

The Block patch needs regenerating (due to #3547)

- integrate existing patch for burning blocks
- move annotation to it's own line
- improve javadoc
@diesieben07
Copy link
Contributor Author

Rebased onto current 1.11.x.

@mezz
Copy link
Contributor

mezz commented Dec 22, 2016

Tested and it works as expected.

@mezz mezz added 1.11 Feature This request implements a new feature. labels Dec 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.11 Feature This request implements a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants