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

Use real_t in physics code #45519

Merged
merged 1 commit into from
Jan 29, 2021
Merged

Conversation

aaronfranke
Copy link
Member

This was originally part of #21922, however that PR is taking forever to be reviewed, so I took the physics parts of it and put it into this PR. @AndreaCatania has already given #21922 an approval, so hopefully this PR won't take too long to get in (of course, more eyes are welcome, so I've requested a review from the entire Physics team).

Copy link
Contributor

@pouleyKetchoupp pouleyKetchoupp left a comment

Choose a reason for hiding this comment

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

The changes look good.

Just a couple things I'm wondering:

  • There are still references to PACKED_FLOAT32_ARRAY in ConvexPolygonShape2DSW and ConcavePolygonShape2DSW data, I guess they can be changed as well (it's just error messages it seems).
  • It might be outside of this PR's scope, but float is still used extensively in scene nodes, including physics 2D and 3D. Should it be changed too to make sure things stay consistent when using double precision?

@aaronfranke
Copy link
Member Author

aaronfranke commented Jan 28, 2021

@pouleyKetchoupp I will likely copy more stuff out of #21922 later to more PRs if it's something that has a good chance of being merged.

Copy link
Contributor

@Xrayez Xrayez left a comment

Choose a reason for hiding this comment

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

I'd merge and see what happens!

@akien-mga akien-mga merged commit 46de553 into godotengine:master Jan 29, 2021
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants