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

Fixes related to teleport Y coordinates #184

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dcoshea
Copy link

@dcoshea dcoshea commented Apr 17, 2020

This PR includes two commits for separate issues related to the selection of the Y coordinate that the player is teleported to. Following are some brief descriptions of the commits, see the commit messages for more details.

The first commit probably fixes two reported issues, at least partially. There was a problem with teleporting to be on top of the highest block if it wasn't at the player's current Y coordinate which would result in the player being teleported to spawn instead if there were no air gaps. I think this fixes issue #178 and probably issue #115 too. On its own this is a trivial commit.

The second commit isn't associated with any reported issues, but while working on the above I noticed that the teleporting code wouldn't deal well with blocks at (assuming the standard world height of 256) Y = 254 or 255 because it insisted on checking if the blocks into which the player would be teleported (Y and Y + 1) were breathable. My fix assumes that the void above the top of the world is breathable. This certainly seems to be the case in vanilla Minecraft.

Since the second commit is more complicated and doesn't address an existing issue, I've kept it as a separate commit even though it touches the same areas of the code as the first commit and in fact replaces some of the changes made by that commit. This way, if you don't want the change from the second commit or just don't want to have to review it, just the first commit can be pulled.

I tested the first commit by verifying it made the problem I saw as described in issue #178 go away, and tested the second by building a sort of staircase ascending to Y = 255 and verifying that I was teleported up it all the way to the top (in a situation where the teleport moved me not only backwards but also sideways due to a circular border).

…#178)

The fix for issue Brettflan#57 added some limits to the values searched when looking for
a safe Y coordinate for the teleport destination, including calling
getHighestBlockYAt() to determine the Y coordinate of the highest non-empty
block.  In non-nether cases with normal terrain that doesn't approach the
world's maximum Y values, highestBlockBoundary will be set to 1 greater than the
return value of getHighestBlockYAt(), i.e. the Y coordinate of the empty block
above the highest non-empty block.

In the non-nether, non-flying case, limTop was previously set to this value, and
the safe Y coordinate search would proceed up to *but not including* this limit,
so the maximum Y coordinate searched to check if it was empty would be the
highest *non*-empty y coordinate.  However, the player's current Y coordinate is
checked first even if it is equal to limTop (unless it is somehow not greater
than limBot).  As a result, if the only safe block for the player to be
teleported on top of was getHighestBlockYAt(), they would only be teleported
there if it wouldn't result in a change in their Y coordinate, otherwise they
would be teleported to spawn.

This fix expands the search to include highestBlockBoundary.

highestBlockBoundary cannot be greater than world.getMaxHeight() - 2, so the
search for a Y coordinate will now include that value rather than excluding it.
isSafeSpot() only adds 1 to that value, giving world.getMaxHeight() - 1, which
is still within the valid Y coordinate range.

This might also fix issue Brettflan#115.
For worlds other than the nether (which is a special case due to the
bedrock ceiling) the maximum Y coordinate to which a player would be
teleported was previously world.getMaxHeight() - 2.  This enabled
isSafeSpot() to check the blocks at Y and Y + 1 to ensure that they
were breathable.  However, this meant that if there were blocks all
the way up to 1 or 2 less than world.getMaxHeight(), the player would
never be teleported on top of them, and instead would be teleported to
an air gap somewhere below them, or to spawn if no such gap was found.

This commit enables the player to be teleported to Y coordinates up to
world.getMaxHeight() so that they can be teleported on top of the
highest possible blocks (which would be at world.getMaxHeight() - 1).
As part of this, isSafeSpot() is updated to treat the void at Y >=
world.getMaxHeight() as breathable.
@andrewkm
Copy link

This PR would be great. :)

@dcoshea
Copy link
Author

dcoshea commented May 19, 2020

Subsequent commit 4551e27 says "This plugin is no longer being supported", so I guess this pull request won't be accepted, but from a quick look at the network graph I see that fork https://github.com/TheCompieter/WorldBorder seems to include my commits.

@andrewkm
Copy link

@dcoshea
Looks like using the latest compile from TheCompieter still breaks the following two things:
https://ecocitycraft.com/forum/threads/drowning-at-the-edge-of-ecc-with-no-water-near-me.198543/
https://ecocitycraft.com/forum/threads/getting-stuck-under-platform-at-edge-of-ecc.198553/

Tons of videos inside :)

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