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

DS: Virtual keyboard support #3312

Merged
merged 3 commits into from Nov 1, 2021
Merged

Conversation

@ccawley2011
Copy link
Member

@ccawley2011 ccawley2011 commented Aug 26, 2021

This is opened as a PR because it affects common code. Fixes Trac #12341.

Copy link
Member

@sev- sev- left a comment

I do not understand why you're enforcing VK on all platforms now. The rest looks fine to me.

Loading

@@ -330,13 +330,11 @@ Common::Keymap *DefaultEventManager::getGlobalKeymap() {
act->setEvent(EVENT_MAINMENU);
globalKeymap->addAction(act);

#ifdef ENABLE_VKEYBD
Copy link
Member

@sev- sev- Aug 27, 2021

Choose a reason for hiding this comment

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

Why is this suddenly removed?

Loading

@@ -384,7 +384,6 @@ HardwareInput Keymapper::findHardwareInput(const Event &event) {
void Keymapper::hardcodedEventMapping(Event ev) {
// TODO: Either add support for long presses to the keymapper
// or move this elsewhere as an event observer + source
#ifdef ENABLE_VKEYBD
Copy link
Member

@sev- sev- Aug 27, 2021

Choose a reason for hiding this comment

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

Same question. E.g. we show the Virtual Keyboard only when it is explicitly defined for the port

Loading

common/events.h Outdated
@@ -89,9 +89,7 @@ enum EventType {
EVENT_CUSTOM_ENGINE_ACTION_START = 20,
EVENT_CUSTOM_ENGINE_ACTION_END = 21,

#ifdef ENABLE_VKEYBD
Copy link
Member

@sev- sev- Aug 27, 2021

Choose a reason for hiding this comment

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

And here

Loading

@ccawley2011
Copy link
Member Author

@ccawley2011 ccawley2011 commented Aug 27, 2021

The DS port provides a custom virtual keyboard, since it's more flexible and uses less RAM. The defines are removed so that the virtual keyboard action is provided by the keymapper. Ideally, the code for the DS keyboard's event observer should be handled in the default event manager so that the other backends with custom virtual keyboards can benefit from it, however this might have to be done after the master branch is unfrozen.

Loading

@ccawley2011
Copy link
Member Author

@ccawley2011 ccawley2011 commented Aug 27, 2021

Ideally, the code for the DS keyboard's event observer should be handled in the default event manager so that the other backends with custom virtual keyboards can benefit from it, however this might have to be done after the master branch is unfrozen.

I've opened PR #3316 with these changes. It shouldn't be as much of a problem as I had first thought, but I'll wait until that PR is merged before updating this one.

Loading

@sev-
Copy link
Member

@sev- sev- commented Nov 1, 2021

#3316 has been merged. Please, review this one now/

Loading

@ccawley2011 ccawley2011 merged commit e3b72b6 into scummvm:master Nov 1, 2021
8 checks passed
Loading
@sev-
Copy link
Member

@sev- sev- commented Nov 10, 2021

Will you also backport it to branch-2-5?

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants