Skip to content

Conversation

@BenjaminBrienen
Copy link
Contributor

Objective

Solution

  • Use bevy_math::ops instead of std floating point operations.

Testing

  • Did you test these changes? If so, how?
    Unit tests and cargo run -p ci -- test

  • How can other people (reviewers) test your changes? Is there anything specific they need to know?
    Execute cargo run -p ci -- test on Windows.

  • If relevant, what platforms did you test these changes on, and are there any important ones you can't test?
    Windows

Migration Guide

  • Not a breaking change
  • Projects should use bevy math where applicable

@BenjaminBrienen
Copy link
Contributor Author

This is incomplete because the test still fails :)

Copy link
Contributor

@IQuick143 IQuick143 left a comment

Choose a reason for hiding this comment

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

The -ln needs fixing, I left some other comments, but those aren't all that blocking.

Someone else please take a thorough look, cause there's a lot of code touched so I may have missed smth.

@IQuick143 IQuick143 added C-Code-Quality A section of code that is hard to understand or change A-Math Fundamental domain-agnostic mathematical operations A-Cross-Cutting Impacts the entire engine D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Sep 16, 2024
BenjaminBrienen and others added 6 commits September 16, 2024 21:42
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
@BenjaminBrienen
Copy link
Contributor Author

This is in a good state for now, but I'm waiting for conclusive feedback on exactly how explicit we should make the calls to bevy_math trig functions, especially in documentation and examples.

@BenjaminBrienen
Copy link
Contributor Author

If anyone would like to suggest a different name for bevy_math::ops before it is made public, now is a good time. To me, ops kind of implies the traits like Add and Mul (for bevy, maybe instead for matrices and vector operations).

@alice-i-cecile
Copy link
Member

ops is fine IMO :)

@alice-i-cecile
Copy link
Member

This is in a good state for now, but I'm waiting for conclusive feedback on exactly how explicit we should make the calls to bevy_math trig functions, especially in documentation and examples.

Stick it in the prelude, but use only the method names please.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Sep 16, 2024
Merged via the queue into bevyengine:main with commit 29508f0 Sep 16, 2024
@BenjaminBrienen BenjaminBrienen deleted the fix-floating-point-math branch September 17, 2024 00:15
@IQuick143
Copy link
Contributor

Oh wow, this ended up being quite an undertaking, props for the work.

@BenjaminBrienen BenjaminBrienen self-assigned this Nov 10, 2024
@BenjaminBrienen BenjaminBrienen removed 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 Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Cross-Cutting Impacts the entire engine A-Math Fundamental domain-agnostic mathematical operations C-Code-Quality A section of code that is hard to understand or change C-Machine-Specific This bug is isolated to specific hardware or driver configurations D-Straightforward Simple bug fixes and API improvements, docs, test and examples

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bevy_color's operations are not stable across all hardware due to use of trigonometric float methods

5 participants