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

Cannot call Body.setDensity on a static body #640

Open
jmfd opened this issue Jul 26, 2018 · 5 comments
Open

Cannot call Body.setDensity on a static body #640

jmfd opened this issue Jul 26, 2018 · 5 comments
Labels

Comments

@jmfd
Copy link
Contributor

jmfd commented Jul 26, 2018

Calling Body.setDensity on a static body will yield invalid results.

This is a regression from Issue #378 (change f7d1877).

See attached example; the box bouncing off the ground will disappear when the ground's density is changed.

setDensityBug.zip

I can easily workaround by testing if it is static first.

@liabru
Copy link
Owner

liabru commented Jul 30, 2018

The functions setDensity, setMass, setInertia shouldn't be used static bodies because they are considered to always have infinite mass. Do you mean that the functions should guard against this? Might be a good idea?

@jmfd
Copy link
Contributor Author

jmfd commented Jul 30, 2018

Yes - while this is no problem for Hype, I filed because I thought from an API perspective it would probably be better to at least not break physics bodies due to NaNs cropping up from setting this value. Seemed like a regression, as Hype was actually supplying Infinity to density already in static cases and that broke the same way.

To take it further, it would be nice if the supplied values could be preserved; this way setStatic could be simply switched on/off and the body wouldn't need to be resupplied with the density. It seems more like an implementation detail for collision calculations that a static body has Infinity density. (But that's just my 2¢ as I already am working around it!)

@liabru liabru added the improve label Jul 31, 2018
@liabru
Copy link
Owner

liabru commented Jul 31, 2018

Yeah I think adding guards is a good idea then.

To take it further, it would be nice if the supplied values could be preserved;

Agreed, it should be possible to implement that using the existing body.parts[i]._original cache approach that's set inside Body.setStatic.

@jmfd
Copy link
Contributor Author

jmfd commented Aug 1, 2018

I'm not sure how much of a factor it is for what you are proposing, but I am swapping out the body.parts (via Body.setParts()) when there are path changes:

beach

This is an upcoming feature in Hype; if there's a better way to change the vertices feel free to let me know. I need to keep the top-level body object (vs just create a whole new one) because there's also a new API for users to get at it and they may keep it around.

@jmfd
Copy link
Contributor Author

jmfd commented Aug 2, 2018

Agreed, it should be possible to implement that using the existing body.parts[i]._original cache approach that's set inside Body.setStatic.

Interesting; I got a report of a bug today that ended up having me look into this - please see #641.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants