-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Block break progress improvement #6500
base: minor-next
Are you sure you want to change the base?
Changes from all commits
7e0f680
020b2d1
38b2fbb
9f9878c
54b1b94
7e5b903
0cadd69
dd7b333
36a3094
be17d67
ab5342c
7128f27
dc89118
72a69dc
b51b9e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -865,6 +865,29 @@ static function() : void{ | |
Timings::$playerChunkSend->stopTiming(); | ||
} | ||
|
||
/** | ||
* Checks if the player is currently on the ground. This is more accurate than {@link Player::isOnGround()} but slower. | ||
*/ | ||
public function isActuallyOnGround() : bool{ | ||
$bb = $this->boundingBox->expandedCopy(-0.001, 0.001, -0.001); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change doesn't look related to the PR. Why would this be needed for ground calculation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixes inaccurate progress at high x/z coordinates There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Many things break down at large coordinates. I doubt this is the proper fix for the problem. |
||
$maxY = (int) floor($this->location->y); | ||
$minY = $maxY - 1; | ||
$floorMinX = (int) floor($bb->minX); | ||
$floorMinZ = (int) floor($bb->minZ); | ||
$floorMaxX = (int) floor($bb->maxX); | ||
$floorMaxZ = (int) floor($bb->maxZ); | ||
for ($x = $floorMinX ; $x <= $floorMaxX ; $x++){ | ||
for ($y = $minY ; $y <= $maxY ; $y++){ | ||
for ($z = $floorMinZ ; $z <= $floorMaxZ ; $z++){ | ||
if ($this->getWorld()->getBlockAt($x, $y, $z)->collidesWithBB($bb)){ | ||
return true; | ||
} | ||
} | ||
} | ||
} | ||
return false; | ||
} | ||
|
||
private function recheckBroadcastPermissions() : void{ | ||
foreach([ | ||
DefaultPermissionNames::BROADCAST_ADMIN => Server::BROADCAST_CHANNEL_ADMINISTRATIVE, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like calculate or smth in the name can be better than this function combined with docs ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm not a fan of this naming either. Also, why do we actually need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need it because when using the normal is on ground method the break progress indicator is still too fast when jumping. What should it be renamed to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like that warrants investigation into regular onGround behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Legend has it that
PlayerAuthInputPacket
should allow making onGround checks more accurate, so this hack wouldn't be needed anymore.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the PR should use the built-in onGround for now. It doesn't make sense to add an extra function specifically for this.
Once the proper onGround is fixed per #6547 we won't need this workaround. I don't like adding stuff to the API that we know we're gonna delete anyway.