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.

Sign up for GitHub

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

Jump to bottom

add keybinding menu subcategories #966

Merged
merged 8 commits into from
Aug 31, 2018
Merged

add keybinding menu subcategories #966

merged 8 commits into from
Aug 31, 2018

Conversation

commy2
Copy link
Contributor

@commy2 commy2 commented Aug 24, 2018 •
edited
Loading

When merged this pull request will:

Test:

for "_i" from 1 to 20 do { 
    private _subcat = ["Evens", "Odds"] select (_i % 2 == 1);
    [["Debug Mod", _subcat], format ["DebugKey_%1", _i], [format ["Key%1 Name",  _i], format ["Key%1 Tooltip",  _i], format ["Category 1",  _i]], { 
        systemChat str diag_frameNo; 
    }, {}, [15, [false, false, false]]] call CBA_fnc_addKeybind; 
};

https://i.imgur.com/RlZFL6y.png

@commy2 commy2 added this to the 3.8.1 milestone Aug 24, 2018
@commy2 commy2 removed the WIP label Aug 25, 2018
dedmen
dedmen approved these changes Aug 25, 2018
View reviewed changes
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.

Code review, no ingame test

@commy2
Copy link
Contributor Author

commy2 commented Aug 30, 2018

Merge?

PabstMirror
PabstMirror approved these changes Aug 31, 2018
View reviewed changes
Copy link
Contributor

@PabstMirror PabstMirror left a comment

Choose a reason for hiding this comment

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

looks good

@commy2 commy2 merged commit 2fef3de into master Aug 31, 2018
@commy2 commy2 deleted the keybindig-categories branch August 31, 2018 15:23
mharis001
mharis001 reviewed Aug 31, 2018
View reviewed changes
Copy link
Contributor

@mharis001 mharis001 left a comment

Choose a reason for hiding this comment

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

Code looks good, just some style changes I can recommend.

addons/keybinding/gui.hpp

class controls {
class Background: RscText {
colorBackground[] = {0.25,0.25,0.25,0.4};
Copy link
Contributor

Choose a reason for hiding this comment

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

A style change that I can suggest is colorBackground[] = {0.15,0.15,0.15,0.4}; and bumping the size up to POS_H(1):
cba_keybind_subcat

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would have to be changed for the settings as well, for consistency. It's carbon coby atm.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Should change both then.

addons/keybinding/gui.hpp
onButtonClick = QUOTE(_this call (uiNamespace getVariable 'FUNC(gui_editKey)'));
style = ST_LEFT;
colorBackground[] = {0,0,0,0};
colorBackgroundActive[] = {1,1,1,1};
Copy link
Contributor

Choose a reason for hiding this comment

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

When hovered, the text can only be read because of the shadow. Remove the shadow, colorFocused[] = {0.5, 0.5, 0.5, 0.5};, and have text color change to black on hover (like before and vanilla). AFAIK RscButton has no property to do that, so would have to be done through EHs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could work with https://community.bistudio.com/wiki/User_Interface_Event_Handlers#onMouseEnter

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that is what I was thinking.

addons/keybinding/fnc_gui_update.sqf

private _edit = _subcontrol controlsGroupCtrl IDC_KEY_EDIT;
_edit ctrlSetText _displayName;
_edit ctrlSetTooltip _tooltip;
Copy link
Contributor

Choose a reason for hiding this comment

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

Like before and vanilla, have the tooltip be shown for both the name and the keys.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

addons/keybinding/fnc_gui_update.sqf

if (_isDuplicated) then {
_ctrlKeyList lnbSetColor [[_index, 1], [1, 0, 0, 1]];
_edit ctrlSetTextColor [1,0,0,1];
Copy link
Contributor

Choose a reason for hiding this comment

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

A lot of red on screen, like before, have only the keys turn to red when duplicate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will go with whatever the base game does. Will check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, Vanilla only highlights the keys.

@commy2
Copy link
Contributor Author

commy2 commented Aug 31, 2018

lol, will still look at those suggestions

@mharis001
Copy link
Contributor

Rip, review right as its merged.

@commy2 commy2 mentioned this pull request Aug 31, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mharis001 mharis001 mharis001 left review comments

@PabstMirror PabstMirror PabstMirror approved these changes

@dedmen dedmen dedmen approved these changes

Assignees
No one assigned
Labels
Feature
Projects
None yet
Milestone
3.9
Development

Successfully merging this pull request may close these issues.

Request: Add Sub-Category for keybindings
4 participants
@commy2 @mharis001 @dedmen @PabstMirror