Skip to content

Add mouse button mapping community patch #3078

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

Merged
merged 4 commits into from
Nov 1, 2023
Merged

Conversation

kcgen
Copy link
Member

@kcgen kcgen commented Nov 1, 2023

Description

Adds the mouse buttons to the mapper using @legoscia's community patch. Thanks @legoscia!

This let's users map host-side keyboard and joystick events to create DOS-side mouse-button click, hold, and release events.

It's handy for users on mono-click touch-pads or using mono-button mice , like the ChesterMouse:

This is the last of the features to be integrated from DOSBox ECE's patch set.

Notes to reviewers

Suggest reviewing commit-by-commit.

The first commit replaces our literal integer mouse buttons with a named enum as a prerequisite, because I didn't want to use the patch's magic literals. @FeralChild64 : hoping you can review this commit, specifically.

As for the patch itself: it's just a couple tiny additions to the mapper and follows the existing mapper structure perfectly.

Related issues

Fixes #3073

Manual testing

Tested the baseline PS/2 and serial mouse types.

Tested mapping host keyboard events to mouse buttons.

Tested in games and windows 3.1.

Tested:

  1. short click and release (with a keyboard button)
  2. long click and release (with a keyboard button)
  3. click, hold, and release
  4. click, hold, drag (moving the mouse), and release

Checklist

I have:

  • followed the project's contributing guidelines and code of conduct.
  • performed a self-review of my code.
  • commented on the particularly hard-to-understand areas of my code.
  • split my work into well-defined, bisectable commits, and I named my commits well.
  • applied the appropriate labels (bug, enhancement, refactoring, documentation, etc.)
  • checked that all my commits can be built.
  • confirmed that my code does not cause performance regressions (e.g., by running the Quake benchmark).
  • added unit tests where applicable to prove the correctness of my code and to avoid future regressions.
  • made corresponding changes to the documentation or the website according to the documentation guidelines.
  • locally verified my website or documentation changes.

@kcgen kcgen added the enhancement New feature or enhancement of existing features label Nov 1, 2023
@kcgen kcgen self-assigned this Nov 1, 2023
This matches the keyboard handling which also uses an enum names
for each button.

This a pre-requisite to integrate the mouse button mapper patch,
which uses magic integers instead of named buttons.
@kcgen kcgen force-pushed the kc/mappable-mouse-buttons-1 branch from 5adb49f to 4817e52 Compare November 1, 2023 17:00
Copy link
Collaborator

@FeralChild64 FeralChild64 left a comment

Choose a reason for hiding this comment

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

I have no remarks. Change looks straightforward an low risk.

@kcgen kcgen mentioned this pull request Nov 1, 2023
10 tasks
@Grounded0
Copy link
Collaborator

Grounded0 commented Nov 1, 2023

Pinging @Burrito78 as our quality assurance person.

kcgen and others added 3 commits November 1, 2023 13:10
Authors notes:

While trying to play The Settlers under DOSBox, I discovered that this
game requires something the manual calls a "special click": click the
left mouse button while holding down the right mouse button.  It turns
out that it's impossible to perform this on a MacBook trackpad.

Thus I came up with this patch, which lets you map keys to mouse
buttons.  Now I can use a key on the keyboard as the right mouse button,
and use the trackpad for the left mouse button.

Imported-from: https://sourceforge.net/p/dosbox/patches/264/
- Sets final and override qualifiers on the
  event class

- Uses a private and constant button
  identifier, as it can only be set at
  construction time.

- Deletes the default constructor

- Uses the MouseButtonId enum names instead of
  raw integers
@kcgen kcgen force-pushed the kc/mappable-mouse-buttons-1 branch from 4817e52 to 6efcb95 Compare November 1, 2023 20:10
Copy link
Collaborator

@weirddan455 weirddan455 left a comment

Choose a reason for hiding this comment

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

Thanks for clearing up the many mouse variable. I know it was kind of a nit-picky thing but it reads clearly now. This all looks good to me 👍

@kcgen kcgen merged commit d1db576 into main Nov 1, 2023
@Burrito78
Copy link
Collaborator

Pinging @Burrito78 as our quality assurance person.

Too late, but will check this out in main tomorrow!

@johnnovak
Copy link
Member

johnnovak commented Nov 1, 2023

Noice, @kcgen bravely poking the mapper-bear 🐻 ⚔️ 😆 Happy for others to make small changes to it... as long as it's not me 😎

This seems to be a handy set of features, maybe not even the WinUAE mapper implements such things (my gold standard mapper implementation), so one more thing for the Mapper V2.0 feature list 🚀

@kcgen
Copy link
Member Author

kcgen commented Nov 1, 2023

maybe not even the WinUAE mapper implements such things

The trick is to use a massive C++ object-oriented incoheritance tree combined with unmanaged pointers.

@H-K-47
Copy link

H-K-47 commented Nov 6, 2023

Sorry, how do you Map the scrollwheel? I want to map the Scrollwheel Up / Down to correspond the the Pgup/Down keyevent - that means if i scroll up it should emulate the pgup key event.

@kcgen
Copy link
Member Author

kcgen commented Nov 6, 2023

Sorry, how do you Map the scrollwheel? I want to map the Scrollwheel Up / Down to correspond the the Pgup/Down keyevent - that means if i scroll up it should emulate the pgup key event.

It's not possible; host-side mouse events aren't mappable to DOS-side keyboard events.

(note the wording in this PR's description: "host-side keyboard and joystick events to create DOS-side mouse-button click, hold, and release events.")

@Torinde
Copy link
Contributor

Torinde commented Nov 7, 2023

Sorry, how do you Map the scrollwheel? I want to map the Scrollwheel Up / Down to correspond the the Pgup/Down keyevent - that means if i scroll up it should emulate the pgup key event.

I hope that'll be supported as part work done for #3075 or #1953

@johnnovak
Copy link
Member

Sorry, how do you Map the scrollwheel? I want to map the Scrollwheel Up / Down to correspond the the Pgup/Down keyevent - that means if i scroll up it should emulate the pgup key event.

I hope that'll be supported as part work done for #3075 or #1953

Yes, it will be supported.

@kcgen kcgen deleted the kc/mappable-mouse-buttons-1 branch November 7, 2023 16:41
@johnnovak johnnovak added the input handling Issues related to handling any input (keyboard, mouse, joystick & game controllers) label Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or enhancement of existing features input handling Issues related to handling any input (keyboard, mouse, joystick & game controllers)
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Allow mouse buttons to be mapped to keyboard keys or controller buttons
9 participants