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

MOHAWK: RIVEN: Add keymapper support (continued) #1999

Merged
merged 3 commits into from Jan 12, 2020

Conversation

@bluegr
Copy link
Member

bluegr commented Jan 12, 2020

This is based on PR #1990, and includes fixes for the comments made in that pull request.

It's been two weeks without any activity in the other PR, and the changes requested were pretty straightforward, so I decided to continue the work that @ccawley2011 started.

@bluegr bluegr requested a review from bgK Jan 12, 2020
@ccawley2011

This comment has been minimized.

Copy link
Member

ccawley2011 commented Jan 12, 2020

Thanks for sorting this out.

One thing I do want to point out, though, is that Skip and Pause are already handled in the global keymap, which is why I didn't add them in the original PR.

engines/mohawk/riven.cpp Outdated Show resolved Hide resolved
@bgK

This comment has been minimized.

Copy link
Member

bgK commented Jan 12, 2020

Thanks! I've noticed a small issue. Otherwise, looks good to me.

@bgK

This comment has been minimized.

Copy link
Member

bgK commented Jan 12, 2020

One thing I do want to point out, though, is that Skip and Pause are already handled in the global keymap.

The global keymap having shortcuts that apply only for some engines seems incorrect to me. Users see the key bindings in the remap dialog, notice they don't work and then complain about it :)

ccawley2011 and others added 3 commits Dec 30, 2019
@bluegr bluegr force-pushed the bluegr:riven_keymapper branch from ef8caaa to 71223f9 Jan 12, 2020
@bluegr

This comment has been minimized.

Copy link
Member Author

bluegr commented Jan 12, 2020

Since all the issues have been addressed now, this can be merged. Thanks for the feedback!

@bluegr bluegr merged commit 12145ac into scummvm:master Jan 12, 2020
1 check was pending
1 check was pending
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.