Skip to content

NUKE CMake dependent options#1624

Merged
slipher merged 1 commit intoDaemonEngine:masterfrom
slipher:nuke-dependent
Apr 9, 2025
Merged

NUKE CMake dependent options#1624
slipher merged 1 commit intoDaemonEngine:masterfrom
slipher:nuke-dependent

Conversation

@slipher
Copy link
Copy Markdown
Member

@slipher slipher commented Mar 26, 2025

cmake_dependent_option, as implemented in the GUI/TUI, is a pointless misfeature. It could be useful if it, say, showed the option as grayed out and then enabled itself as soon as the dependee changed. But what it actually does is hide the dependent option completely, and not show it until you reconfigure. So it's just a nuisance when you want to use the option: you have to configure multiple times. The goal of hiding options that are not relevant is hopeless anyway since there are always zillions of options that don't affect the current build.

We had just 2 cmake_depend_option's, USE_SMP and USE_SLIM_LTO.

Fixes Unvanquished/Unvanquished#1921.

cmake_dependent_option, as implemented in the GUI/TUI, is a pointless
misfeature. It could be useful if it, say, showed the option as grayed
out and then enabled itself as soon as the dependee changed. But what it
actually does is hide the dependent option completely, and not show it
until you reconfigure. So it's just a nuisance when you want to use the
option: you have to configure multiple times. The goal of hiding options
that are not relevant is hopeless anyway since there are always zillions
of options that don't affect the current build.

We had just 2 cmake_depend_option's, USE_SMP and USE_SLIM_LTO.

Fixes Unvanquished/Unvanquished#1921.
@illwieckz
Copy link
Copy Markdown
Member

I also seen CMake warnings about them, like they are not done properly or so.

I agree with the principle of removing the option dependency. We don't need that.

Copy link
Copy Markdown
Member

@illwieckz illwieckz left a comment

Choose a reason for hiding this comment

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

LGTM.

@slipher slipher merged commit a182d3c into DaemonEngine:master Apr 9, 2025
9 checks passed
@slipher slipher deleted the nuke-dependent branch April 9, 2025 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CMake warning

2 participants