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

Cone collision shape #64

Closed
jcornaz opened this issue Mar 8, 2021 · 4 comments
Closed

Cone collision shape #64

jcornaz opened this issue Mar 8, 2021 · 4 comments
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.

@jcornaz jcornaz added enhancement New feature or improvement up-for-grabs Good for newcomers labels Mar 8, 2021
@nicopap
Copy link
Contributor

nicopap commented Nov 24, 2021

I'm interested in implementing this. But how would this work with 2d? The first implementation that comes to mind is having extra enum variants to CollisionShape marked with #[cfg(dim3)] But I think this might not be very elegant.

enum CollisionShape {
  Sphere { radius: f32 },
  Capsule { half_segment: f32, radius: f32 },
  // etc.
  #[cfg(dim3)]
  Cone { height: f32, radius: f32 },
}

I don't even know if it's even possible to do this (it compiles fine, but it seems to just mark the field as non-existent). If it works there is still the question to know how the API will look like. Is this satisfactory? It's easy to make a point against it. I may be lacking imagination but I don't see the alternative.

(edit1)

Potential alternative

Have it not marked as #[cfg(dim3)], have a dim2 implementation that translates to a simple triangle. (I don't like this solution)

@jcornaz
Copy link
Owner Author

jcornaz commented Nov 24, 2021

Hi, Thanks for looking into it.

Indeed cone make sense only in 3d. There is no such thing as a 2d code. You can expose the cone variant behind a #[cfg(dim3)] as in your first suggestion.

You can also use this opportunity to add a #[non_exhaustive] on the enum itself force user's pattern matching to always include a else (_) branch. That something I should've done since the begining, I just didn't know it was possible.

there is still the question to know how the API will look like.

I'm not sure to understand what questions remain to be answered.

Following you'r suggestion usage would be like:

    commands
        .spawn_bundle(PbrBundle::default()) // <-- Render mesh
        .insert(RigidBody::Dynamic) // <-- Make it rigid body
        
        // The cone collision shape
        .insert(CollisionShape::Cone { heigh: 5.0, radius: 3.0 })

@nicopap
Copy link
Contributor

nicopap commented Nov 27, 2021

At least for the debug_3d crate, there is a good reason to keep the CollisionShape enum exhaustive: It will emit a compiler error when adding a new shape and forgetting to add the debug render routine for it.

I understand why in term of API stability it is good to use #[non_exhaustive]. It makes sense to not have a major version bump every time a new shape is added. But we should be aware of the downsides.

nicopap added a commit to nicopap/heron that referenced this issue Nov 27, 2021
nicopap added a commit to nicopap/heron that referenced this issue Nov 27, 2021
@jcornaz
Copy link
Owner Author

jcornaz commented Nov 28, 2021

Solved by #158

@jcornaz jcornaz closed this as completed Nov 28, 2021
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

No branches or pull requests

2 participants