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

Add Capsule2d primitive #11585

Merged
merged 7 commits into from
Jan 29, 2024
Merged

Add Capsule2d primitive #11585

merged 7 commits into from
Jan 29, 2024

Conversation

Jondolf
Copy link
Contributor

@Jondolf Jondolf commented Jan 28, 2024

Objective

Currently, the Capsule primitive is technically dimension-agnostic in that it implements both Primitive2d and Primitive3d. This seems good on paper, but it can often be useful to have separate 2D and 3D versions of primitives.

For example, one might want a two-dimensional capsule mesh. We can't really implement both 2D and 3D meshing for the same type using the upcoming Meshable trait (see #11431). We also currently don't implement Bounded2d for Capsule, see #11336 (comment).

Having 2D and 3D separate at a type level is more explicit, and also more consistent with the existing primitives, as there are no other types that implement both Primitive2d and Primitive3d at the same time.

Solution

Rename Capsule to Capsule3d and add Capsule2d. Capsule2d implements Bounded2d.

For now, I went for Capsule2d for the sake of consistency and clarity. Mathematically the more accurate term would be Stadium or Pill (see Wikipedia), but those might be less obvious to game devs. For reference, Godot has CapsuleShape2D. I can rename it if others think the geometrically correct name is better though.


Changelog

  • Renamed Capsule to Capsule3d
  • Added Capsule2d with Bounded2d implemented

@Jondolf Jondolf added C-Feature A new feature, making something new possible A-Math Fundamental domain-agnostic mathematical operations labels Jan 28, 2024
@alice-i-cecile alice-i-cecile added the C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide label Jan 28, 2024
Copy link
Contributor

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

@@ -382,6 +382,7 @@ pub struct Rectangle {
/// Half of the width and height of the rectangle
pub half_size: Vec2,
}
impl Primitive2d for Rectangle {}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I randomly spotted this missing, lol

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jan 28, 2024

Much prefer Capsule2D: the more geometrically correct terms are much less intuitive. If users complain we can add doc aliases.

Edit: oh you're way ahead of me. Figures :p You have a great eye for details like this.

@alice-i-cecile alice-i-cecile added this to the 0.13 milestone Jan 28, 2024
Copy link
Contributor

@NiseVoid NiseVoid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👀

@matiqo15 matiqo15 added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jan 28, 2024
@alice-i-cecile
Copy link
Member

@Jondolf a couple of issues following the merge; let me know when you have it fixed up.

@Jondolf
Copy link
Contributor Author

Jondolf commented Jan 29, 2024

Should be fixed @alice-i-cecile

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 29, 2024
Merged via the queue into bevyengine:main with commit a9f061e Jan 29, 2024
22 checks passed
@Jondolf Jondolf deleted the capsule-2d branch January 29, 2024 18:29
github-merge-queue bot pushed a commit that referenced this pull request Feb 3, 2024
# Objective

The `Capsule2d` primitive was added in #11585. It should support meshing
like the other 2D primitives.

## Solution

Implement meshing for `Capsule2d`.

It doesn't currently support "rings" like Bevy's `Capsule` shape (not
`Capsule3d`), but it does support resolution to control the number of
vertices used for one hemicircle. The total vertex count is two times
the resolution; if we allowed setting the full vertex count, odd numbers
would lead to uneven vertex counts for the top and bottom hemicircles
and produce potentially unwanted results.

The capsule looks like this (with UV visualization and wireframe) using
resolutions of 16, 8, and 3:

![Resolution
16](https://github.com/bevyengine/bevy/assets/57632562/feae22de-bdc5-438a-861f-848284b67a52)
![Resolution
8](https://github.com/bevyengine/bevy/assets/57632562/e95aab8e-793f-45ac-8a74-8be39f7626dd)
![Resolution of
3](https://github.com/bevyengine/bevy/assets/57632562/bcf01d23-1d8b-4cdb-966a-c9022a07c287)

The `2d_shapes` example now includes the capsule, so we also get one
more color of the rainbow 🌈

![New 2D shapes
example](https://github.com/bevyengine/bevy/assets/57632562/1c45b5f5-d26a-4e8c-8e8a-e106ab14d46e)
@Jondolf Jondolf mentioned this pull request Feb 4, 2024
46 tasks
tjamaan pushed a commit to tjamaan/bevy that referenced this pull request Feb 6, 2024
# Objective

Currently, the `Capsule` primitive is technically dimension-agnostic in
that it implements both `Primitive2d` and `Primitive3d`. This seems good
on paper, but it can often be useful to have separate 2D and 3D versions
of primitives.

For example, one might want a two-dimensional capsule mesh. We can't
really implement both 2D and 3D meshing for the same type using the
upcoming `Meshable` trait (see bevyengine#11431). We also currently don't
implement `Bounded2d` for `Capsule`, see
bevyengine#11336 (comment).

Having 2D and 3D separate at a type level is more explicit, and also
more consistent with the existing primitives, as there are no other
types that implement both `Primitive2d` and `Primitive3d` at the same
time.

## Solution

Rename `Capsule` to `Capsule3d` and add `Capsule2d`. `Capsule2d`
implements `Bounded2d`.

For now, I went for `Capsule2d` for the sake of consistency and clarity.
Mathematically the more accurate term would be `Stadium` or `Pill` (see
[Wikipedia](https://en.wikipedia.org/wiki/Stadium_(geometry))), but
those might be less obvious to game devs. For reference, Godot has
[`CapsuleShape2D`](https://docs.godotengine.org/en/stable/classes/class_capsuleshape2d.html).
I can rename it if others think the geometrically correct name is better
though.

---

## Changelog

- Renamed `Capsule` to `Capsule3d`
- Added `Capsule2d` with `Bounded2d` implemented

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
tjamaan pushed a commit to tjamaan/bevy that referenced this pull request Feb 6, 2024
# Objective

The `Capsule2d` primitive was added in bevyengine#11585. It should support meshing
like the other 2D primitives.

## Solution

Implement meshing for `Capsule2d`.

It doesn't currently support "rings" like Bevy's `Capsule` shape (not
`Capsule3d`), but it does support resolution to control the number of
vertices used for one hemicircle. The total vertex count is two times
the resolution; if we allowed setting the full vertex count, odd numbers
would lead to uneven vertex counts for the top and bottom hemicircles
and produce potentially unwanted results.

The capsule looks like this (with UV visualization and wireframe) using
resolutions of 16, 8, and 3:

![Resolution
16](https://github.com/bevyengine/bevy/assets/57632562/feae22de-bdc5-438a-861f-848284b67a52)
![Resolution
8](https://github.com/bevyengine/bevy/assets/57632562/e95aab8e-793f-45ac-8a74-8be39f7626dd)
![Resolution of
3](https://github.com/bevyengine/bevy/assets/57632562/bcf01d23-1d8b-4cdb-966a-c9022a07c287)

The `2d_shapes` example now includes the capsule, so we also get one
more color of the rainbow 🌈

![New 2D shapes
example](https://github.com/bevyengine/bevy/assets/57632562/1c45b5f5-d26a-4e8c-8e8a-e106ab14d46e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Math Fundamental domain-agnostic mathematical operations C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide C-Feature A new feature, making something new possible S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants