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

Buoyancy demo nitpick #262

Closed
sbj42 opened this issue Oct 5, 2016 · 3 comments
Closed

Buoyancy demo nitpick #262

sbj42 opened this issue Oct 5, 2016 · 3 comments

Comments

@sbj42
Copy link

sbj42 commented Oct 5, 2016

In the buoyancy demo, if I put the "water" plane at a different Y position, the objects float at a different depth. Try setting plane.position[1] to -1 or 1, and see where the shapes come to rest.

I believe this is because of a bug on line 89:

var height = 0 - aabb.lowerBound[1];

This is meant to compute the height of the portion of the shape's bounding box that is "underwater". I think that 0 should actually be planePosition[1], so like this:

var height = planePosition[1] - aabb.lowerBound[1];
@schteppe
Copy link
Owner

schteppe commented Oct 5, 2016

It's not technically a bug, because the user cannot change the position of the plane in the demo?

In any case you're right and it should be using the correct bounds, if someone wants to reuse the code. Wanna submit a PR?

@sbj42
Copy link
Author

sbj42 commented Oct 5, 2016

You're right. In fact that's how I spotted this, I was adapting it for my own project.

Yes, I'll submit a pull request. I found some other things along the way that I think would be worth having in the demo, so I'll include those as well. For instance, The applied force for each shape can be multiplied by body.mass / body.shapes.length so that it works better for bodies with different masses and compound bodies.

@sbj42 sbj42 changed the title Bug in buoyancy demo Buoyancy demo nitpick Oct 5, 2016
@sbj42
Copy link
Author

sbj42 commented Oct 6, 2016

Pull request #263 submitted. I decided not to include the body.mass multiplier because that's a slippery slope leading to questions of body density and I didn't think that was necessarily important for the demo.

@sbj42 sbj42 closed this as completed Oct 6, 2016
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

No branches or pull requests

2 participants