Incompatibility with Essential Commands (breaks /rtp)
pahanakun opened this issue · 6 comments
When Essential Commands runs alongside ServerCore, using the random teleport command always teleports to a block height of y=2
Disabling ServerCore returns the /rtp function to normal, random teleport places you on top of the highest block.
ServerCore: 1.2.6-1.18.1
Minecraft: 1.18.1
Essential Commands: 0.19.0-mc1.18.1
I have posted this issue on the github page for Essential Commands as well.
Thanks for reporting this issue!
It seems Essentials Commands uses raycasting to find valid Y levels for its RTP's in unloaded chunks.
ServerCore blocks raycasting in unloaded chunks by default because vanilla doesn't use it for anything that requires chunkloading, and can cause large lagspikes.
There is honestly no good reason why essentials commands should use this either, as its quite inefficient for what they're trying to accomplish.
This should be resolved in the dev builds now.
@Wesley1808 I appreciate the quick patch for mod compat!
There is honestly no good reason why essentials commands should use this either, as its quite inefficient for what they're trying to accomplish.
I'm curious, what would you recommend instead? This is by far the slowest command of the EC bunch, and I'd love to improve it.
I'm curious, what would you recommend instead? This is by far the slowest command of the EC bunch, and I'd love to improve it.
Here's an example for a part of a random teleport algorithm I wrote myself once.
It iterates through a column of blocks vertically downwards, rather than raycasting, as raycasting tends to do a lot of extra expensive calculations we don't need.
The calculateMaxY()
method may seem confusing, what it really does is iterate through the chunk sections to reduce the amount of times we need to iterate through air blocks. Its faster to check 5 times if a chunk section contains no blocks (mc caches the block count per section), compared to calling chunk.getBlockState()
80 times.
this.maxY = the Y coordinate it starts searching at. For example, the nether has it at 90 so it doesn't land on the roof.
/**
* Gets the Y level of the surface of given position.
*
* @param chunk: The chunk to check in
* @return Integer: the Y level the surface is at.
*/
private int getY(Chunk chunk) {
final int maxY = this.calculateMaxY(chunk);
final int bottomY = chunk.getBottomY();
if (maxY <= bottomY) {
return Integer.MIN_VALUE;
}
final BlockPos.Mutable mutable = new BlockPos.Mutable(this.x, maxY, this.z);
boolean isAir = chunk.getBlockState(mutable).isAir(); // Block at head level
boolean isAir2; // Block at feet level
boolean isAir3; // Block below feet
mutable.move(Direction.DOWN);
for (isAir2 = chunk.getBlockState(mutable).isAir(); mutable.getY() > bottomY; isAir2 = isAir3) {
mutable.move(Direction.DOWN);
isAir3 = chunk.getBlockState(mutable).isAir();
if (!isAir3 && isAir2 && isAir) {
return mutable.getY() + 1;
}
isAir = isAir2;
}
return Integer.MIN_VALUE;
}
/**
* Allows us to skip a lot of unnecessary calls to chunk.getBlockState();
*/
private int calculateMaxY(Chunk chunk) {
final ChunkSection[] sections = chunk.getSectionArray();
final int maxSectionIndex = Math.min(sections.length - 1, this.maxY >> 4);
for (int index = maxSectionIndex; index >= 0; --index) {
if (!sections[index].isEmpty()) {
return Math.min(index << 4 + 15, this.maxY);
}
}
return Integer.MIN_VALUE;
}
/**
* Checks if the position is safe to teleport to.
*
* @param chunk: The chunk to check in
* @return Boolean: if the position is safe.
*/
private boolean isSafe(Chunk chunk, int y) {
if (y <= chunk.getBottomY()) return false;
var blockPos = new BlockPos(this.x, y - 1, this.z);
var material = chunk.getBlockState(blockPos).getMaterial();
return blockPos.getY() < this.maxY && !material.isLiquid() && material != Material.FIRE;
}
Below here are optional, but easy checks which you can do before loading the chunk you need for getting the Y position.
For example, if we know we are going to land outside the border, or in an ocean biome, or any other biome likely to result in a failure, we skip loading the chunk and move on to the next position.
/**
* Quick checks to skip a lot of expensive chunk loading.
*/
private boolean isValid(BlockPos pos) {
if (this.world.getWorldBorder().contains(pos)) {
return this.isBiomeValid(this.world.getBiome(pos));
}
return false;
}
private boolean isBiomeValid(Biome biome) {
final RegistryKey<Biome> key = this.world.getRegistryManager().get(Registry.BIOME_KEY).getKey(biome).orElse(null);
final Biome.Category category = biome.getCategory();
return key != null
&& category != OCEAN
&& category != RIVER
&& category != BEACH
&& category != SWAMP
&& category != UNDERGROUND
&& category != NONE
&& key != THE_END
&& key != SMALL_END_ISLANDS;
}
One of the remaining problems would be on how to most efficiently load the chunk. Theres 2 ways of doing this that I know of:
-
world.getChunk()
Blocks the main thread until it is loaded, then use it. Easiest way, but by far the most noticeable for players aswell. -
world.getChunkManager().addTicket(ChunkTicketType ticketType, ChunkPos pos, 1, player.getId())
Doesn't block the main thread (or barely), but it means you have to "wait" asynchronously and check from time to time if the chunk is fully loaded. Examples on how to check if a chunk is fully loaded can also be found in this repository underChunkManager.java
.
If you choose this option you'll want to make sure you stop checking if a chunk is loaded when said chunk ticket runs out.
Example of a ChunkTicketType that keeps the chunk loaded for 170 ticks aka 8.5 seconds:
ChunkTicketType<Integer> PRE_LOCATE = ChunkTicketType.create("pre_locate", Integer::compareTo, 170);
Wow, lots of great ideas here. Thanks for taking the time to write up such a detailed response. I'll put this newfound knowledge to good use.
I do have a question about one of the things that you mentioned: In ChunkTicketType
, what is this expiryTicks
value relative to? The tick when the ticket is made? When the chunk load starts? When it finishes? (I'm sure my lack of experience with all things "chunks" is showing.)
ExpiryTicks is the amount of ticks a chunk ticket will stay active for.
Thats why I said, if you want to check for loaded chunks, don't bother checking if the ticket expired ;)