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 proper collision on non-rectangular shaped elevators. #1677

Open
wants to merge 1 commit into
base: 1.12
Choose a base branch
from

Conversation

p4plus2
Copy link

@p4plus2 p4plus2 commented Mar 16, 2018

This change breaks the bounding box into strips rather than a single bounding box. This gives a more expected behaviour to non-rectangular platforms. Due to the "big margin" check sometimes it is difficult to fall through a one block gap, perhaps it is worth making the margin a config value (how was the value 150 chosen anyway?).

Two notes:

  1. I am not sure how one would "upgrade" existing elevators which were moving on world load. One method could be a hack like:
if (tagCompound.hasKey("bminX")) {
    bounds.clear();
    int bminX = tagCompound.getInteger("bminX");
    int bminZ = tagCompound.getInteger("bminZ");
    int bmaxX = tagCompound.getInteger("bmaxX");
    int bmaxZ = tagCompound.getInteger("bmaxZ");
    bounds.add(new Bounds(bminX, bminZ, bmaxX, bmaxZ));
}
  1. I have never really done any code in java let alone a minecraft mod, so this approach may not be implemented optimally, this approach was done more or less by vaguely emulating the style of surrounding code where obvious. I am open to changing or correcting the implementation if there is a better way to do it.

@p4plus2
Copy link
Author

p4plus2 commented Mar 16, 2018

Noticed a small change that should also be made if (bounds == null) { should be if (bounds.isEmpty()) Strictly speaking it makes no difference and the check could probably be fully removed as far as I can tell, but proper error checking isn't a bad thing.

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.

1 participant