Skip to content
This repository has been archived by the owner on Nov 29, 2022. It is now read-only.

HeightField Collision shape #62

Closed
jcornaz opened this issue Mar 8, 2021 · 5 comments · Fixed by #102
Closed

HeightField Collision shape #62

jcornaz opened this issue Mar 8, 2021 · 5 comments · Fixed by #102
Assignees
Labels
enhancement New feature or improvement up-for-grabs Good for newcomers

Comments

@jcornaz
Copy link
Owner

jcornaz commented Mar 8, 2021

No description provided.

@elegaanz
Copy link
Contributor

elegaanz commented May 21, 2021

Hi, I would like to help implement that, would a PR be accepted? (edit: I just saw the "up-for-grabs" label) If yes, do you have any tip or things I need to know about the code base or this issue before I get started?

@jcornaz
Copy link
Owner Author

jcornaz commented May 21, 2021

Hi, thanks @elegaanz!

Adding a new collision shape shouldn't be too hard. Add the new variant to Body (in core crate) and let the compiler tells you what else you need to change.

However.... I am currently making some big changes (see: #31 (comment)). So you might want to wait 2-3 days, to avoid unnecessary conflicts. (I plan to merge theses changes this weekend)

@jcornaz
Copy link
Owner Author

jcornaz commented May 21, 2021

Ah, and if you need help, you can either open a draft PR and/or contact me on discord (@jomag)

@elegaanz
Copy link
Contributor

I started something, just to get a bit more familiar with the code, even if I need to redo it in a few days, and I have a question: in 2D heightfields are defined by a single-dimension array, but for 3D you need a matrix/two dimensional array. Is something like that a good idea?

enum Body {
    // ...
    HeightField {
        #[cfg(feature = "2d")]
        heigths: Vec<f32>,

        #[cfg(feature = "3d")]
        heigths: Vec<Vec<f32>>,
}

Or is there a better way?

@jcornaz
Copy link
Owner Author

jcornaz commented May 22, 2021

I merged the changes I was talking about. So you can take the latest version of main and work from there.

Yes, you can use #[cfg(feature = ...)]. It looks like the best option in this case.

The Vec<Vec<f32>> is acceptable too. I'm don't know a nice dynamically sized matrix type that could replace this.

You'll also needs a scale member that tells the distance separating each height value of the vector/matrix.

In 2d it's a f32, and in 3d it's a Vec2. But, we may simplify it by forcing the user to use the same scale for both axis in 3d. This is anyway the most common, and as a matter of fact, I've never seen a height map with a different scale in x than in y. That means the scale could always be a f32 in both 2d and 3d.

enum CollisionShape {
    // ...

    #[cfg(not(all(feature = "2d", feature = "3d")))]
    HeightField {

        scale: f32,

        #[cfg(feature = "2d")]
        heigths: Vec<f32>,

        #[cfg(feature = "3d")]
        heigths: Vec<Vec<f32>>,
   }
}

P.S. The enum is now named "CollisionShape" ;-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or improvement up-for-grabs Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants