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

Quickmount - Default enable quick mount (empty keybind) and Move to CBA Settings #6613

Merged
merged 2 commits into from
Oct 13, 2018

Conversation

jonpas
Copy link
Member

@jonpas jonpas commented Oct 4, 2018

When merged this pull request will:

Hotkey is unset by default, no reason to have a setting in that case as described in #6516.

@jameslkingsley review please.

@jonpas jonpas added the kind/enhancement Release Notes: **IMPROVED:** label Oct 4, 2018
@jonpas jonpas added this to the 3.13.0 milestone Oct 4, 2018
Copy link
Contributor

@dedmen dedmen left a comment

Choose a reason for hiding this comment

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

I'm not that other guy. But I still approve of this.

@Dystopian
Copy link
Contributor

What if some server admin would like to disable this feature? I think jameslkingsley meant just enabling by default.

@jonpas
Copy link
Member Author

jonpas commented Oct 4, 2018

But he also said that's a hacky fix. :D

@jameslkingsley
Copy link
Contributor

Looks good to me 👍

@Dystopian
Copy link
Contributor

I just think Quick Mount can be not appropriate for some milsim PvP games because it's cheating a little. Setting would be helpful in these cases.
BTW where is that jonpas who asked me to add setting for some features I implemented? ;)

@jonpas
Copy link
Member Author

jonpas commented Oct 4, 2018

I don't know, not my feature, I'll do what others think is best.

@PabstMirror
Copy link
Contributor

Could add a new setting called GVAR(allowed) that is global and default to true

@jonpas jonpas changed the title Remove Quick mount enable setting (enabled via keybind) Default enable Quick mount (empty keybind) and Move to CBA Settings Oct 5, 2018
@dedmen
Copy link
Contributor

dedmen commented Oct 5, 2018

Shouldn't the config entries be removed after the setting was moved to CBA?

@jonpas
Copy link
Member Author

jonpas commented Oct 5, 2018

movedToSQF = 1;
I don't remember, but this has something to do with something.

@PabstMirror
Copy link
Contributor

ace_common preinit runs first
if someone has a modified ace_settings in either mission config or configFile, the settings converted would think it was just an unconverted setting and try to add it first

[
QGVAR(enabled),
"CHECKBOX",
localize ELSTRING(common,Enabled),
Copy link
Contributor

Choose a reason for hiding this comment

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

[localize ELSTRING(common,Enabled), localize LSTRING(KeybindDescription)],
not sure if description removed intentionally,
it wasn't a perfect match, but I thought it gave some context to what the module did

QGVAR(enabled),
"CHECKBOX",
localize ELSTRING(common,Enabled),
format ["ACE %1", localize LSTRING(Category)],
Copy link
Contributor

Choose a reason for hiding this comment

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

LLSTRING and LELSTRING could be used here and anywhere

Copy link
Contributor

Choose a reason for hiding this comment

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

Could simply delete the localize , because the settings system can translate the strings on its own.

Copy link
Contributor

Choose a reason for hiding this comment

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

btw just checked, subcategory is not automatically localized

Copy link
Contributor

Choose a reason for hiding this comment

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

Issue report then.

Copy link
Contributor

Choose a reason for hiding this comment

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

@PabstMirror PabstMirror changed the title Default enable Quick mount (empty keybind) and Move to CBA Settings Quickmount - Default enable quick mount (empty keybind) and Move to CBA Settings Oct 13, 2018
@PabstMirror PabstMirror merged commit 9972f53 into master Oct 13, 2018
@PabstMirror PabstMirror deleted the hotkeyOnlyQuickMount branch October 13, 2018 03:14
@PabstMirror PabstMirror modified the milestones: 3.13.0, 3.12.4 Nov 9, 2018
BaerMitUmlaut pushed a commit that referenced this pull request Aug 5, 2019
…6613)

Quickmount - Default enable quick mount (empty keybind) and Move to CBA Settings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants