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

Fixed players smaller than 1 block not being able to walk under overhang #2605

Merged
merged 1 commit into from
Mar 22, 2016
Merged

Conversation

Thutmose
Copy link
Contributor

With the vanilla code, it is impossible to make a player collide less than 1 block tall, as it will always check the block above for headroom.

This PR adds in a check to see if the player height is less than 1 block, and if so, does not check the block above for headroom.

@Thutmose
Copy link
Contributor Author

Without this patch, if a mod changes the size of a player to below 1 block tall, the player cannot walk into a space which is only 1 block tall, as it will always check the the block directly above the block containing the player.

This patch allows mods which change the player's size to then let players walk into areas which are 1x1x1.

@simon816
Copy link
Contributor

I think the ideal solution would be to check n blocks above where n is the ceiling of the player's height (applicable to all living entities too).

@Thutmose
Copy link
Contributor Author

That shouldn't be necessary, the normal collision code (which is run anyway) would cover those cases, as it checks that anyway.

@Thutmose
Copy link
Contributor Author

Here are some images to explain why this patch is needed:

image

The hitbox is small enough to fix, but the player cannot walk through

image

@LexManos
Copy link
Member

Actually the PROPER patch would be to remove that second part all together. and potentially allow for some way to detect partially filled blocks.
The Math.round also needs to be converted to a (int)Math.ceil call.
Kinda curious about how @iChun did morph as this is his code. And I know he made it work with partially filled blocks.

@Thutmose
Copy link
Contributor Author

On the partially filled blocks thing, I was trying to keep it as minimal as possible.

I will test and see what happens if that method is removed entirely (ie always returns true)

@LexManos
Copy link
Member

The METHOD doesn't need to be removed, the second block check does because the caller method itterates through all blocks.
Also, this may be worth patching in Entity itself as it ONLY checks 1 block.

@Thutmose
Copy link
Contributor Author

the call for the method is this: this.pushOutOfBlocks(this.posX - (double)this.width * 0.35D, this.getEntityBoundingBox().minY + 0.5D, this.posZ + (double)this.width * 0.35D);

for each combination of signs for the (double)this.width * 0.35D

If I replace
return !this.worldObj.getBlockState(pos).getBlock().isNormalCube() && (this.height < 1 || !this.worldObj.getBlockState(pos.up()).getBlock().isNormalCube());
with
return !this.worldObj.getBlockState(pos).getBlock().isNormalCube();

Then it will no longer properly push the player out of a block if it just goes over the head, like in this setup:

image

@simon816 was somewhat correct in saying that ideally it should check up more, given that the initial call is only from the foot location

Edit: The one in Entity only seems to be called by items and xporbs for some reason.

@Thutmose
Copy link
Contributor Author

Once this section is patched, collision works fine in partial blocks,

image

and

image

That particular method is only used for pushing the player out of a block if they manage to get into it in the first place

Should I push the patch with the Math.round turned to (int)Math.ciel?

@LexManos
Copy link
Member

Ah I see the issue.
the isHeadspaceFree needs a ! in the if IsOpenBlock call. Once that's done, removing the second block check will work fine.

@Thutmose
Copy link
Contributor Author

Yep, that seems to do it, re-generating patches with that change.

@iChun
Copy link
Contributor

iChun commented Mar 21, 2016

I never had any problems with players being smaller than one block as long as the eye height of the player was appropriate...

@iChun
Copy link
Contributor

iChun commented Mar 21, 2016

Unless this is a new issue in 1.9, I've not had any issues in MC 1.7.10 or 1.8.

@Thutmose
Copy link
Contributor Author

did you try in 1.8.9?

I have the player's eye height adjusted correctly (I assume it is, it moves the view point appropriately, the f3+b box shows correctly, and can place blocks directly above myself, and it doesn't suffocate)

It just won't let the player walk into an area with no headroom above the block the player is in.

Here is what I have been using:

https://github.com/Thutmose/Pokecube/blob/master/Pokecube%20Addons/src/main/java/pokecube/pokeplayer/Proxy.java#L163-L189

I guess I should probably use an AT to make setSize(float width, float height) in Entity public.

Edit: as you can see in the above images, it does collide properly, and the eye height seems to be in the correct spot.

@iChun
Copy link
Contributor

iChun commented Mar 21, 2016

No, I've not tested in 1.8.9. I only started development on 1.8.9 a few weeks back and had nothing to do with eyeHeight.

How does elytra in 1.9 handle having the player smaller than a block then? :/

@Thutmose
Copy link
Contributor Author

I haven't tried my player as pokemob for 1.9 yet, I was getting it written on 1.8.9 first due to the less bugs.

I was going to see about porting it once I got the basics working.

@iChun
Copy link
Contributor

iChun commented Mar 21, 2016

I was gonna say to hold off on this PR till it's been tested in 1.9, then I realised that master is still on 1.8.9. Hmm.

@LexManos
Copy link
Member

Ya, nobody seems to change the player's size so nobody has run into this before. However it's a issue between 1.8.9 and 1.9, so it's worth patching both.
Simple fix tho.

@Thutmose
Copy link
Contributor Author

@iChun I checked 1.7.10, and it doesn't check the block above, which would explain why you never came across this problem there.

I don't have a 1.8 environment setup to check, but it might also be the same there too.

@Thutmose
Copy link
Contributor Author

So is there anything else I need to do for this?

@LexManos
Copy link
Member

Squash, and one for 1.9 would be appreciated {So I can pull both and fix both branches}.

@Thutmose
Copy link
Contributor Author

I Think I squashed it? I will do one for 1.9.

@LexManos
Copy link
Member

You squashed but now it has a useless commit message. Remember, this is what creates the changelog. A "Rebase and recommit" line isn't informative.

@Thutmose
Copy link
Contributor Author

That should be better description?

@Thutmose
Copy link
Contributor Author

#2618 was made with the same changes but for 1.9 branch.

The patch file is slightly different due to isNormalCube() being moved to the blockstate instead of in the block

LexManos added a commit that referenced this pull request Mar 22, 2016
Fixed players smaller than 1 block not being able to walk under overhang
@LexManos LexManos merged commit b317d5d into MinecraftForge:master Mar 22, 2016
@Thutmose Thutmose deleted the playersize branch March 22, 2016 01:15
@LexManos
Copy link
Member

God dammet was this event tested? Went and tested it in game and had ALL kinda of issues. Like being sucked into the side of walls, and being pushed in the wrong directions.
f14f77d

IChun do me a favor, do 1.8.9's version.

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.

4 participants