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

Only set quicktune keybinds in debug builds #12779

Merged
merged 1 commit into from Oct 6, 2022

Conversation

rollerozxa
Copy link
Member

This PR disables the default quicktune keybinds in non-debug builds, as the quicktune functionality is only useful for engine development in debug builds anyways and the keybinds interfere with other keybinds mapped onto the numpad. (i.e. I have keybinds on my numpad to start and stop recording with OBS, which also triggers quicktune)

To do

This PR is a Ready for Review.

How to test

See that quicktune keybinds exist in debug and not in release...

@SmallJoker
Copy link
Member

I think it would make more sense to unbind the keys altogether for debug&release but document them in settingtypes.txt.

@sfan5
Copy link
Member

sfan5 commented Sep 20, 2022

I don't run debug builds for development so maybe do this for all non-release builds?
Doesn't matter that much though.

@Desour
Copy link
Member

Desour commented Sep 20, 2022

I don't run debug builds for development so maybe do this for all non-release builds?

Quicktune only works in debug builds, so this PR is consistent:

#ifndef NDEBUG
#define QUICKTUNE(type_, var, min_, max_, name){\
QuicktuneValue qv;\
qv.type = type_;\
qv.value_##type_.current = var;\
qv.value_##type_.min = min_;\
qv.value_##type_.max = max_;\
updateQuicktuneValue(name, qv);\
var = qv.value_##type_.current;\
}
#else // NDEBUG
#define QUICKTUNE(type, var, min_, max_, name){}
#endif

I agree it would be nice to enable quicktune independently of debug builds.

Copy link
Contributor

@TurkeyMcMac TurkeyMcMac left a comment

Choose a reason for hiding this comment

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

This way is fine for now, IMO.

@rubenwardy rubenwardy merged commit be5c675 into minetest:master Oct 6, 2022
appgurueu pushed a commit to appgurueu/minetest that referenced this pull request May 2, 2023
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.

None yet

6 participants