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

{cmake} FLOAT_PRECISION should be GODOT_FLOAT_PRECISION & added to the target #1145

Closed
wants to merge 1 commit into from

Conversation

asmaloney
Copy link
Contributor

@asmaloney asmaloney commented Jun 16, 2023

All top-level CMake options should be prefixed with GODOT_ so as not to "pollute the environment" when included in other projects.

Adding it to the target as PUBLIC will propagate it to any projects including godot-cpp.

@asmaloney asmaloney requested a review from a team as a code owner June 16, 2023 16:18
@asmaloney asmaloney changed the title {cmake} FLOAT_PRECISION should be GODOT_FLOAT_PRECISION {cmake} FLOAT_PRECISION should be GODOT_FLOAT_PRECISION & added to the target Jun 16, 2023
…e target

All top-level CMake options should be prefixed with GODOT_ so as not to "pollute the environment" when included in other projects.

Adding it to the target as PUBLIC will propagate it to any projects including godot-cpp.
@asmaloney
Copy link
Contributor Author

bump

@dsnopek dsnopek added the topic:buildsystem Related to the buildsystem or CI setup label Oct 18, 2023
@fire
Copy link
Member

fire commented Jun 30, 2024

@asmaloney Can you rebase the git pull request?

Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

The general flow looks good to me, but needs a rebase.

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

The rename part of this PR has been superseded by PR #1583.

Comment on lines +163 to +167
if("${GODOT_FLOAT_PRECISION}" STREQUAL "double")
target_compile_definitions( ${PROJECT_NAME} PUBLIC
REAL_T_IS_DOUBLE
)
endif()
Copy link
Member

@aaronfranke aaronfranke Sep 18, 2024

Choose a reason for hiding this comment

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

Is this needed anymore now that we have this code?

set(GODOT_PRECISION "single" CACHE STRING "Set the floating-point precision level (single|double)")
if ("${GODOT_PRECISION}" STREQUAL "double")
	add_definitions(-DREAL_T_IS_DOUBLE)
endif()

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you're right, this is no longer needed

@dsnopek
Copy link
Collaborator

dsnopek commented Oct 17, 2024

Thanks! However, it looks like this was fixed by another PR

@dsnopek dsnopek closed this Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archived cmake topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants