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

addKeybind - Handle empty strings to indicated no code #688

Merged
merged 2 commits into from
May 28, 2017

Conversation

PabstMirror
Copy link
Contributor

_downCode - Code for down event, empty string for no code. <CODE>

ACRE uses this:
["ACRE2", "CycleRadio", (localize LSTRING(CycleRadio)), { [1] call FUNC(cycleRadios) }, "", [58, [true, false, true]]] call cba_fnc_addKeybind;

@PabstMirror PabstMirror added this to the 3.4 milestone May 23, 2017
@PabstMirror PabstMirror requested a review from commy2 May 23, 2017 02:37
@jonpas
Copy link
Member

jonpas commented May 23, 2017

Shouldn't this automatically be done by params? As it only accepts code {} as input, if it's not code it will use the default value, which is code {} anyways.

@@ -88,6 +88,10 @@ if (_defaultKeybind isEqualTypeParams [0, false, false, false]) then {
_defaultKeybind = [_defaultKey, [_defaultShift, _defaultControl, _defaultAlt]];
};

// Handle empty strings to indicated no code
if (_downCode isEqualType "") then {_downCode = {};};
if (_upCode isEqualType "") then {_upCode = {};};
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not go the extra mile and do:

+if (_downCode isEqualType "") then {
    _downCode = compile _downCode;
};
 +if (_upCode isEqualType "") then {
    _upCode = compile _upCode;
};

Would make more sense to me.

Copy link
Contributor Author

@PabstMirror PabstMirror May 23, 2017

Choose a reason for hiding this comment

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

The new method is consistent with the old way which only checked if it was a string

    if(IS_CODE(_downCode)) then {
        [_hashDown, "keydown"] call cba_fnc_removeKeyHandler;

Compiling could cause problems if they pass something like "#nothing".
I figured it's deprecated anyway as there is no reason to not pass {}

Copy link
Contributor

Choose a reason for hiding this comment

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

I know what you mean, but then this isn't limited to STRING either, but any type.
And "#nothing" is a bad example, because that actually doesn't throw an error when compiling or calling the result, because # is treated like #LINE 1 and other instructions that survive preprocessing even though it does nothing.

@commy2
Copy link
Contributor

commy2 commented May 23, 2017

@jonpas True, but it also throws an error. From what I understand, STRING used to work as replacement for CODE just like how you can use STRING inside addEventHandler.

@jonpas
Copy link
Member

jonpas commented May 23, 2017

Ah, you are correct indeed, totally forgot about the error.

@@ -88,6 +88,10 @@ if (_defaultKeybind isEqualTypeParams [0, false, false, false]) then {
_defaultKeybind = [_defaultKey, [_defaultShift, _defaultControl, _defaultAlt]];
};

// Handle empty strings to indicated no code
Copy link
Contributor

@Killswitch00 Killswitch00 May 24, 2017

Choose a reason for hiding this comment

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

The lines below results in any string, empty or not, to be regarded as an empty code block The comment suggests otherwise.

@commy2 commy2 merged commit 2aaafa7 into master May 28, 2017
@commy2 commy2 deleted the addKeybindEmptyStrings branch May 28, 2017 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants