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

implement crawling #936

Merged
merged 8 commits into from
Nov 17, 2024
Merged

implement crawling #936

merged 8 commits into from
Nov 17, 2024

Conversation

FDUTCH
Copy link
Contributor

@FDUTCH FDUTCH commented Nov 16, 2024

No description provided.

Copy link
Member

@TwistedAsylumMC TwistedAsylumMC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely looks good! Just a few little comments

@@ -282,3 +282,26 @@ func (box BBox) ZOffset(nearby BBox, deltaZ float64) float64 {
}
return deltaZ
}

// Corners returns slice of all hitbox corners
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a full stop at the end of this doc and the other 2 in this file.

Comment on lines 290 to 296
xyZ := mgl64.Vec3{min[0], min[1], max[2]}
xYz := mgl64.Vec3{min[0], max[1], min[2]}
xYZ := mgl64.Vec3{min[0], max[1], max[2]}
XYz := mgl64.Vec3{max[0], max[1], min[2]}
XyZ := mgl64.Vec3{max[0], min[1], max[2]}
Xyz := mgl64.Vec3{max[0], min[1], min[2]}
return []mgl64.Vec3{min, max, xyZ, xYz, xYZ, XYz, XyZ, Xyz}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd remove these variables and do something like this:

return []mgl64.Vec3{
  {min[0], min[1], max[2]},
  {min[0], max[1], min[2]},
  // etc.
}```

Comment on lines 1050 to 1060
var isAir bool
for _, corner := range p.Type().BBox(p).Translate(p.Position()).Corners() {
_, isAir = p.World().Block(cube.PosFromVec3(corner).Add(cube.Pos{0, 1, 0})).(block.Air)
if !isAir {
break
}
}

if !isAir && !p.crawling.CompareAndSwap(false, true) {
return
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be slightly simplified to

for _, corner := range p.Type().BBox(p).Translate(p.Position()).Corners() {
	_, isAir = p.World().Block(cube.PosFromVec3(corner).Add(cube.Pos{0, 1, 0})).(block.Air)
	if !isAir {
		if !p.crawling.CompareAndSwap(false, true) {
			return
		}
		break
	}
}

Comment on lines 2656 to 2632
default:
return 1.62
}
return 1.62
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No point changing this to a default cause, keep how it was before

Comment on lines 2618 to 2649
// EyeHeight returns the eye height of the player: 1.62, or 0.52 if the player is swimming.
// EyeHeight returns the eye height of the player.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could keep the last part of the doc and say "0.52 if the player is crawling or swimming, 1.26 when sneaking or 1.62 when standing."

return BBox{min: box.min.Mul(val), max: box.max.Mul(val)}
}

// Volume calculates Volume of the BBox.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Little nitpick, second "Volume" makes more sense to be "the volume"

@@ -2617,8 +2646,11 @@ func (p *Player) OnGround() bool {

// EyeHeight returns the eye height of the player: 1.62, or 0.52 if the player is swimming.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice if you could update this doc as previously mentioned to describe all states

@TwistedAsylumMC TwistedAsylumMC merged commit 32946c9 into df-mc:master Nov 17, 2024
1 check passed
@TwistedAsylumMC
Copy link
Member

Thanks!

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