search

jcornaz / heron

[DISCONTINUED] An ergonomic physics API for bevy games
MIT License
292 stars 44 forks source link

Implement a HeightField collision shape #102

Closed elegaanz closed 3 years ago

elegaanz commented 3 years ago

Resolves #62

Some notes:

elegaanz commented 3 years ago

The debug example won't compile, is it expected? It can't find the re-exported rapier crate in heron_rapier apparently…

jcornaz commented 3 years ago

Thank you @elegaanz!

I'll look at it tomorrow :-)

The debug example won't compile, is it expected? It can't find the re-exported rapier crate in heron_rapier apparently…

No, that isn't expected. I indeed found today a small missconfiguration of the debug crate that could lead to this behavior if it is compiled alone (without the rest of the workspace). It is fixed in main.

But you don't have to worry about that. I tried the changes in your branches, and everything compiles fine. You can try cargo run test --workspace --no-default-features --features "2d, debug" if you want ;-)

elegaanz commented 3 years ago

I know it compiles and passes the test suite, but I wanted to see if the debug implementation actually draws what it should draw. 'll try to rebase on main to see if it helps.

jcornaz commented 3 years ago

Yes.

In the examples directory, there is a debug example that display the currently supported shapes.

You can add a height field in that example, and then look at the result with: cargo run --example debug --no-default-features --features "2d, debug"

jcornaz commented 3 years ago

I made some ajustements on your work and pushed into a branch heightfield. You can pull from it ;-)

I went with a Vec<Vec> to define the heights, both in 2D and 3D (and only take the first sub-vec in account in 2D). I couldn't get #[cfg(features)] to work as I wanted

That's fine for me. In fact, it may be the best approach.

I'm not sure about the debug implementation, because I don't know much about lyon. I'll probably add a heightfield to the debug example to see if it does what I expect.

I see it could look weird if the the first height isn't the lowest of the shape. I fixed it in my version of this branch.

The scale that rapier expects is actually more the dimensions of the field, so I had to do a little conversion.

Ah! Actually that makes sense. I have been fooled by the name rapier uses. But I realize now that asking the user to define the size of the field makes a bit more sense, and is probably more natural for the user.

Do you mind changing the scale: f32 field for a size: Vec2?

Also, it looks like rapier/parry always centers the heightfield. Is there anything I need to do to update the transform of a collision shape in rapier when it is updated in bevy?

I think that the position of the shape in the world should correspond the center of the field. That would be the most coherent with other placements of bevy components (like camera, sprite) and the other collision shapes (like sphere and cuboid).

If I understand well, that's what rapier does right?

If that's the case, you only need to fix the rendering of the debug shape to center it.

Tell me if you want more support ;-)

elegaanz commented 3 years ago

Thanks for your feedback, I just have one question: in 2D scale would be a f32, or should I use a Vec2 here too and ignore the second element?

jcornaz commented 3 years ago

Yes. You use Vec2 for both 2d/3d. In 2d you ignore the y part.

elegaanz commented 3 years ago

I finally found time to make the change, sorry for making you wait. It should be good now if you want to give it another review.

jcornaz commented 3 years ago

Thank you very much @elegaanz. Il'll have a look (and probably merge) during the week.

Do not worry about the time, if it was urgent the issue woudn't be marked 'up-for-grab' ;-)