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

Mouse mapping support #741

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open

Conversation

lightmanLP
Copy link
Contributor

@lightmanLP lightmanLP commented Dec 11, 2024

Support for binding mouse keys

  • update dxgi mouse capture to make it compatible with sdl
  • fix fullscreen dxgi mouse capture behavior

@lightmanLP lightmanLP force-pushed the feat/mouse-mapping branch 2 times, most recently from df0a498 to 9a9c219 Compare December 18, 2024 05:19
@lightmanLP lightmanLP marked this pull request as ready for review December 18, 2024 06:21
Copy link
Collaborator

@briaguya-ai briaguya-ai left a comment

Choose a reason for hiding this comment

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

overall stuff is looking good! left a few comments (some you already had todos for)

i wasn't able to test this out locally (now that the resourcefilter stuff landed it seems the soh side of that will need to land before testing this is straightforward), but i was hoping to test out what the mapping experience - specifically what happens when this popup is visible

image

and someone clicks on (or misses when trying to click on) the cancel button

Comment on lines 80 to +85
bool ControlDeck::KeyboardGameInputBlocked() {
// block keyboard input when typing in imgui
return AllGameInputBlocked() || ImGui::GetIO().WantCaptureKeyboard;
ImGuiWindow* activeIDWindow = ImGui::GetCurrentContext()->ActiveIdWindow;
return AllGameInputBlocked() ||
(activeIDWindow != NULL &&
activeIDWindow->ID != Context::GetInstance()->GetWindow()->GetGui()->GetMainGameWindowID()) ||
ImGui::GetTopMostPopupModal() != NULL; // ImGui::GetIO().WantCaptureKeyboard, but ActiveId check altered
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

just curious - why does the keyboard game input blocking logic need to change for mouse support?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you hold mouse down, ActiveId changes from zero and locks keyboard inputs

Copy link
Collaborator

@briaguya-ai briaguya-ai Dec 18, 2024

Choose a reason for hiding this comment

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

so this is codifying "we want to block keyboard input when mouse is held down"? the previous logic here was "this is when we don't want to send keyboard input to the game" (if we're blocking all game input or if someone is typing in imgui)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ImGui blocks in-game input via WantCaptureKeyboard when you holding down mouse button

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My changes replicates WantCaptureKeyboard, but without this block

src/controller/controldeck/ControlDeck.cpp Outdated Show resolved Hide resolved
src/window/MouseMeta.h Outdated Show resolved Hide resolved
src/graphic/Fast3D/gfx_dxgi.cpp Outdated Show resolved Hide resolved
src/graphic/Fast3D/gfx_dxgi.cpp Show resolved Hide resolved
@lightmanLP
Copy link
Contributor Author

Cancel button click should work without any problems, you can test it here https://github.com/lightmanLP/2ship2harkinian/tree/mouse-mapping-test
P.S. slightly outdated because of release-version LUS

@briaguya-ai
Copy link
Collaborator

briaguya-ai commented Dec 18, 2024

Cancel button click should work without any problems, you can test it here https://github.com/lightmanLP/2ship2harkinian/tree/mouse-mapping-test P.S. slightly outdated because of release-version LUS

the cancel button does seem to work as expected, but i am still somewhat concerned about always capturing mouse when reading input for mappings

accidental mouse inputs are decently common, so having some safeguards would be nice

one example i noticed when testing this out was if someone accidentally double clicks on an existing mapping it'll replace the mapping

Screencast.From.2024-12-18.10-13-50.mp4

i'd like to hear what others think about this (is it a problem we should worry about? if it's a problem we should worry about, what would a good solution be?)

@lightmanLP
Copy link
Contributor Author

lightmanLP commented Dec 18, 2024

one example i noticed when testing this out was if someone accidentally double clicks on an existing mapping it'll replace the mapping

It might be fixed by spawning binding popup offsetted from cursor and checking for cursor to be in popup boundaries for mouse binding

@briaguya-ai
Copy link
Collaborator

It might be fixed by spawning binding popup offsetted from cursor and checking for cursor to be in popup boundaries for mouse binding

that seems reasonable to me!

it doesn't solve the "tried to click the cancel button and missed" problem but that's probably fine - if people start reporting that as a bug once this ships we can look into other solutions

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

Successfully merging this pull request may close these issues.

3 participants