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

Add support for named Button hotkeys #13706

Merged
merged 4 commits into from Aug 25, 2017

Conversation

Projects
None yet
5 participants
@pchote
Member

pchote commented Jul 23, 2017

This restarts progress towards moving our hotkey definitions into mod code. A NamedHotkey class is introduced to enable ButtonWidget Key's to refer to a named hotkey. All hardcoded button hotkeys are switched over to this new setup.

Followup PRs will remove the remaining hardcoded hotkeys and then finally replace the hardcoded list in KeySettings with a dynamic dictionary using mod-defined defaults/descriptions.

@pchote pchote added this to the Next + 1 milestone Jul 23, 2017

Show outdated Hide outdated OpenRA.Game/Input/NamedHotkey.cs Outdated
@rob-v

rob-v approved these changes Jul 23, 2017

👍

Show outdated Hide outdated OpenRA.Game/Input/NamedHotkey.cs Outdated
@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Jul 23, 2017

Member

Updated with a lint test that covers this first part. @rob-v you were a bit quick, so may want to reevaluate/reconfirm your 👍.

Member

pchote commented Jul 23, 2017

Updated with a lint test that covers this first part. @rob-v you were a bit quick, so may want to reevaluate/reconfirm your 👍.

@rob-v

This comment has been minimized.

Show comment
Hide comment
@rob-v

rob-v Jul 23, 2017

Contributor

lgtm 👍

Contributor

rob-v commented Jul 23, 2017

lgtm 👍

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Jul 23, 2017

Member

Generalized the *Widget linting to automatically detect the fields to be checked.

Member

pchote commented Jul 23, 2017

Generalized the *Widget linting to automatically detect the fields to be checked.

@pchote pchote requested review from reaperrr, abcdefg30 and Phrohdoh Aug 5, 2017

@Phrohdoh

I left some inline comments but nothing required.

Show outdated Hide outdated OpenRA.Game/Input/NamedHotkey.cs Outdated
X: 4
Width: 34
Height: 35
Background:
Key: escape Shift

This comment has been minimized.

@Phrohdoh

Phrohdoh Aug 5, 2017

Member

Would you add another commit that changes escape to Escape for consistency?

@Phrohdoh

Phrohdoh Aug 5, 2017

Member

Would you add another commit that changes escape to Escape for consistency?

This comment has been minimized.

@pchote

pchote Aug 13, 2017

Member

This is asking for a major change that isn't really related to this PR: capitalizing all of these (not just escape, but space, tab, the F-keys, etc) is over 100 changed lines that i'm not interested in having to test.

@pchote

pchote Aug 13, 2017

Member

This is asking for a major change that isn't really related to this PR: capitalizing all of these (not just escape, but space, tab, the F-keys, etc) is over 100 changed lines that i'm not interested in having to test.

This comment has been minimized.

@Phrohdoh

Phrohdoh Aug 13, 2017

Member

That's fair.

@Phrohdoh

Phrohdoh Aug 13, 2017

Member

That's fair.

Show outdated Hide outdated OpenRA.Mods.Common/Lint/CheckChromeHotkeys.cs Outdated
@abcdefg30

Once @Phrohdoh's nits are resolved.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Aug 13, 2017

Member

Rebased and fixed nits.

Member

pchote commented Aug 13, 2017

Rebased and fixed nits.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Aug 13, 2017

Contributor

Needs another rebase.

Contributor

reaperrr commented Aug 13, 2017

Needs another rebase.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Aug 15, 2017

Member

Rebased again.

Member

pchote commented Aug 15, 2017

Rebased again.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Aug 25, 2017

Contributor

I didn't test this extensively, but a quick test on the asset browser and build palette showed now regressions and this already had +2 anyway.

Contributor

reaperrr commented Aug 25, 2017

I didn't test this extensively, but a quick test on the asset browser and build palette showed now regressions and this already had +2 anyway.

@reaperrr reaperrr merged commit a29360f into OpenRA:bleed Aug 25, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@pchote pchote deleted the pchote:remove-hardcoded-hotkeys-part-1 branch Oct 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment