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.

Sign up for GitHub

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

Jump to bottom

Cleanup: Reflect on bevy_math's curve structs #13189

Closed
mweatherley opened this issue May 2, 2024 · 0 comments · Fixed by #13348
Closed

Cleanup: Reflect on bevy_math's curve structs #13189

mweatherley opened this issue May 2, 2024 · 0 comments · Fixed by #13348
Labels
A-Math Fundamental domain-agnostic mathematical operations C-Usability A simple quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Uncontroversial This work is generally agreed upon

Comments

@mweatherley
Copy link
Contributor

What problem does this solve or what need does it fill?

Our cubic curves and NURBS are presently lacking Reflect implementations in bevy_reflect. This is especially important for CubicCurve and RationalCurve, which might actually be passed around in user data.

Additional context

For end-users to derive Reflect on structs that contain math data types, those math types need some amount of reflection, which is something that we have to supply.
@mweatherley mweatherley added C-Enhancement A new feature S-Needs-Triage This issue needs to be labelled labels May 2, 2024
@alice-i-cecile alice-i-cecile added D-Trivial Nice and easy! A great choice to get started with Bevy C-Usability A simple quality-of-life change that makes Bevy easier to use A-Math Fundamental domain-agnostic mathematical operations S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Uncontroversial This work is generally agreed upon and removed C-Enhancement A new feature S-Needs-Triage This issue needs to be labelled labels May 2, 2024
@aristaeus aristaeus mentioned this issue May 13, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue May 16, 2024
@aristaeus
Add reflect impls for bevy_math curve structs (#13348)
be03ba1 
# Objective

Fixes #13189

## Solution

To add the reflect impls I needed to make all the struct fields pub. I
don't think there's any harm for these types, but just a note for
review.

---------

Co-authored-by: Ben Harper <ben@tukom.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Math Fundamental domain-agnostic mathematical operations C-Usability A simple quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Uncontroversial This work is generally agreed upon
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add reflect impls for bevy_math curve structs aristaeus/bevy
2 participants
@mweatherley @alice-i-cecile