Add initial support for mouse and gamepad input by maaxxaam · Pull Request #29 · OpenAWE-Project/OpenAWE

maaxxaam · 2023-03-28T13:42:19Z

Adds support for capturing and adding bindings to mouse movement and button clicks. Also adds an option to lock mouse inside game's window. Includes a modified ControlledFreeCamera that uses mouse movement for camera rotation.

Related to #13

maaxxaam · 2023-03-29T15:18:01Z

Noticed something weird: I cannot use my laptop's trackpad together with the keyboard - looks like mouse movement events get suppresed. External mouse works okay though.

Edit: after some research, it appears that this behaviour comes from OS settings and has nothing to do with the engine itself.

Use vertical mouse scroll for Y movement to test 1D mouse bindings

Nostritius

Here is my first review for this one. Apart from the things, already pointed out, I would like to see one Controlled freecamera instead of two separate, ideally in a modular way, so you have methods like "setMouseInputDisabled".
I have not yet tested, the functionality of the camera in practice, I will probably follow up with comments when I have done that.

Nostritius · 2023-04-02T09:26:45Z

src/events/event.h


 typedef std::variant<
 		AxisEvent<float>,
+		AxisEvent<double>,


Did you use this one somewhere? If not, it should be deleted

src/events/eventman.h

src/platform/window.h

src/platform/window.cpp

src/platform/window.h

src/game.cpp

src/platform/window.cpp

That removes getMouseLastPosiiton() as a public method since its used only within Window class now. Also renames lock/unlock mouse functions for clarity.

InputManager is used to expand upon GLFW callbacks, adding button hold state and some stub code for future gamepad support

maaxxaam · 2023-04-05T17:09:36Z

Commit a520a7e changes the keyboard layout:
WASD, R/F and T/G have not changed. Camera rotation uses Arrow keys (both horizontal and vertical rotation), and camera rotation sensitivity can be changed separately now with Y/H. Q allows switching between arrow keys and mouse movement for camera rotation control.

src/platform/window.cpp

src/platform/inputman.h

src/events/gamepad.h

src/platform/gamepadconversion.cpp

src/events/eventman.cpp

src/platform/gamepadconversion.h

Nostritius · 2023-04-15T08:19:34Z

src/platform/gamepadconversion.cpp

+Events::Gamepad2DAxis convertGamepadStick(int stick) {
+	switch (stick) {
+        case 0: return Events::Gamepad2DAxis::kGamepadAxisLeft;
+        case 1: return Events::Gamepad2DAxis::kGamepadAxisRight;


Could you use the glfw nacros in these functions?

GLFW does not distinguish gamepad sticks and triggers by default - it treats them all as 1D axes, so library macros are weird to use here (like GLFW_GAMEPAD_AXIS_LEFT_X for the whole left stick?). I can create an enum for them, though, and switch to using macros for triggers axes.

src/platform/inputman.h

src/platform/platform.cpp

Nostritius · 2023-04-15T14:19:12Z

src/game.cpp

+	EventMan.setActionCallback({ kLockMouse }, [&](Events::Event event){_window->hideMouseCursor();});
+	EventMan.setActionCallback({ kUnlockMouse }, [&](Events::Event event){_window->showMouseCursor();});
+	EventMan.addBinding(kLockMouse, Events::kMouseLeft);
+	EventMan.addBinding(kUnlockMouse, Events::kKeyEscape);


If you want to keep the lock/unlock mouse mechanic on the layer of the game class, it would probably be simpler to jus put this into the callback. And please define a unified key combination to lock the camera, one which is probably never used in the game, for example alt + l.

- Formatting and typos - Add useRawMouseMotion() - Use GLFW macros for gamepad axes

Nostritius

I tested the new freecamera today and I like it so far. I think it would be beneficial to have a README hint, which keys control the camera how.
As I have written in the last Review , I'm not really happy with adding a new event type where we have to do all the injecting by ourselves which adds a lot of code. So I would prefer a solution which would be able to work without this hold event. As I have written, would the utilizing of the update function help?

Nostritius · 2023-05-01T12:37:46Z

src/platform/window.cpp

 	glfwSetFramebufferSizeCallback(_window, &Window::callbackFramebufferSize);
+	GamepadMan.setGamepadButtonCallback(_window, &Window::callbackGamepadButton);
+	GamepadMan.setGamepadTriggerCallback(_window, &Window::callbackGamepadTrigger);
+	GamepadMan.setGamepadStickCallback(_window, &Window::callbackGamepadStick);


The Gamepad code should stay independent of the window, since the gamepads are in no way associated with it. The gamepad callbacks should be initialized in the game class directly over the GamepadMan singleton.

src/platform/window.cpp

Nostritius · 2023-05-05T16:55:43Z

src/events/event.h

 */
 enum KeyState {
 	kPress,
+	kHold,


I see your point especially with the mouse stuff. Would it be an option if you rather implement the update(float delta) method?

FreeCamera now has flags for whether to clear direction/rotation after update(delta) or not. This allows to handle both keyboard and mouse/gamepad events without KeyManager. Also includes some tweaks according to the PR's feedback.

Removes hold state from both event.h and GamepadManager, since keyboard keys don't have hold state now.

CI failed because, apparently, fmt cannot format an Enum (see prev. two commits). Don't know why it started to fail now (code's been there for a while), but a simple conversion to an int should hopefully suffice.

Nostritius

The code crashes at my laptop when executed in the gamepad manager. furthermore I have some additional comments,

Nostritius · 2023-05-29T11:49:40Z

src/platform/gamepadman.h

+	void pollGamepadEvents();
+	void setGamepadButtonCallback(GLFWwindow *window, InputGamepadButtonCallback function);
+	void setGamepadTriggerCallback(GLFWwindow *window, InputGamepadTriggerCallback function);
+	void setGamepadStickCallback(GLFWwindow *window, InputGamepadStickCallback function);


The gamepad callbacks do not need a reference to the GLFWwindow

Nostritius · 2023-05-29T16:41:42Z

src/platform/window.cpp

+	w->_mouseButtonCallback(button, action, mods);
+}
+
+void Window::callbackGamepadButton(GLFWwindow *window, int button, int action) {


I think you misunderstood me here, the gamepad callbacks should be moved into the GamepadMan and the Window should have absolutely nothing to do with the gamepads.

src/platform/window.cpp

src/platform/gamepadman.cpp

Also adds debug messages for gamepad connection

Since both movement and camera rotation should have worked with them from the beginning in this case

Nostritius

I have left three comments for the code. One thing I have to add, it would be nice if the locking of the mouse cursor is off by default, just to prevent someone inexperienced being caught in the window.

src/events/event.h

Nostritius · 2023-07-01T20:21:08Z

src/events/eventman.cpp

+	}
+}
+
+void EventManager::injectMouse1DAxisInput(Events::Mouse1DAxis axis, float position, float delta) {


From what I see in the current state, you are not using the 1D Axis inputs. Do we need them in the future? If not, I would prefer to remove them for simplifying the whole interface

Actually, they are used for separating mouse scroll inputs (that are 2D by default) into horizontal and vertical components. Here is a snippet for callback assignment from game.cpp:

_window->setMouseScrollCallback([&](glm::vec2 absolute, glm::vec2 delta){ EventMan.injectMouse1DAxisInput(Events::kMouseScrollHorizontal, absolute.x, delta.x); EventMan.injectMouse1DAxisInput(Events::kMouseScrollVertical, absolute.y, delta.y); });

Nostritius · 2023-07-01T20:34:04Z

README.md

+- [TG] to increase/decrease movement speed
+# Camera
+- [←→↑↓] to look left/right/up/down
+- [YH] to increase/decrease camera speed


I assume you use a QWERTY keyboard? Since I am using a german QWERTZ keyboard this combination is a little bit counter intuitive for me. Maybe you have an idea how to handle this better, if not we can leave it this way for now.

Yes, when I'm referring to keys here, I imply QWERTY layout. Also not sure yet of what can be done here for now.

Also makes mouse lock disabled by default

Nostritius · 2023-07-09T17:09:02Z

Merged, Thanks :)

Nostritius · 2023-07-09T17:12:05Z

If you are interested, I have recently added support for moving Alan with very basic animations. If you want you could try to implement his Orbital Cam.

maaxxaam added 3 commits March 26, 2023 15:10

Add support for mouse buttons

8657141

Add support for mouse movement

970c06d

Add mouse controlled free cam for testing

fdc2a7d

maaxxaam force-pushed the mouse-handling branch from f7ac940 to fdc2a7d Compare March 28, 2023 13:48

maaxxaam marked this pull request as ready for review March 28, 2023 13:49

Swap X and Y axis to make code less confusing

60ad3c2

maaxxaam added 2 commits March 29, 2023 18:22

Add support for mouse scroll wheel

df956d7

Split movement and rotation handling for mouse controlled free cam

57191ef

maaxxaam force-pushed the mouse-handling branch from 86bccab to 57191ef Compare March 29, 2023 15:27

maaxxaam added 2 commits March 30, 2023 02:24

Allow using 1D components of a 2D axis input

b1f4316

Rebind Y movement for mouse controlled free cam

4c9c278

Use vertical mouse scroll for Y movement to test 1D mouse bindings

maaxxaam force-pushed the mouse-handling branch from 16b768a to 4c9c278 Compare March 30, 2023 15:51

Nostritius requested changes Apr 2, 2023

View reviewed changes

maaxxaam added 4 commits April 4, 2023 14:36

Fix formatting and callback checks

ce1cd12

Make 2D axis callback vector-based

dedd9ed

That removes getMouseLastPosiiton() as a public method since its used only within Window class now. Also renames lock/unlock mouse functions for clarity.

Add InputManager class

3a54d32

InputManager is used to expand upon GLFW callbacks, adding button hold state and some stub code for future gamepad support

Merge ControlledFreeCamera and MouseControlledFreeCamera

a520a7e

maaxxaam requested a review from Nostritius April 5, 2023 17:13

maaxxaam force-pushed the mouse-handling branch from 1923b70 to 5a309bc Compare April 8, 2023 00:47

maaxxaam changed the title ~~Add initial support for mouse input~~ Add initial support for mouse and gamepad input Apr 8, 2023

maaxxaam force-pushed the mouse-handling branch from 5a309bc to 4d2594f Compare April 8, 2023 13:43

Add gamepad support

90237f8

Nostritius requested changes Apr 15, 2023

View reviewed changes

maaxxaam added 3 commits April 16, 2023 18:55

Small changes according to the feedback

0b77647

- Formatting and typos - Add useRawMouseMotion() - Use GLFW macros for gamepad axes

Split InputManager into KeyManager and GamepadManager

3a8b6f5

Add modifiers for key events

94d1d75

maaxxaam force-pushed the mouse-handling branch from 4d2594f to 94d1d75 Compare April 16, 2023 17:57

Nostritius requested changes May 5, 2023

View reviewed changes

maaxxaam added 3 commits May 28, 2023 22:43

Remove KeyManager in favor of FreeCamera's update()

4425b24

FreeCamera now has flags for whether to clear direction/rotation after update(delta) or not. This allows to handle both keyboard and mouse/gamepad events without KeyManager. Also includes some tweaks according to the PR's feedback.

Remove hold state

6c85c42

Removes hold state from both event.h and GamepadManager, since keyboard keys don't have hold state now.

Fix fmt assertion failing for exception

9c93a1b

CI failed because, apparently, fmt cannot format an Enum (see prev. two commits). Don't know why it started to fail now (code's been there for a while), but a simple conversion to an int should hopefully suffice.

maaxxaam force-pushed the mouse-handling branch from 0b405a7 to 9c93a1b Compare May 28, 2023 20:53

maaxxaam requested a review from Nostritius May 28, 2023 22:13

Nostritius requested changes May 29, 2023

View reviewed changes

maaxxaam added 3 commits June 23, 2023 13:03

Simplify GamepadManager to disconnect it from Window class

d54ba63

Add delta calculation for gamepad events

21b5eb3

Also adds debug messages for gamepad connection

Update README to hint at camera controls

7c606d8

maaxxaam requested a review from Nostritius June 23, 2023 11:28

Make axis events for gamepad use absolute values

1da4d10

Since both movement and camera rotation should have worked with them from the beginning in this case

maaxxaam force-pushed the mouse-handling branch from 39979c0 to 1da4d10 Compare June 23, 2023 11:42

Nostritius requested changes Jul 1, 2023

View reviewed changes

Allow adding bindings to key combinations

f8af9de

Also makes mouse lock disabled by default

maaxxaam requested a review from Nostritius July 3, 2023 12:37

Nostritius merged commit 469a23b into OpenAWE-Project:master Jul 9, 2023

maaxxaam deleted the mouse-handling branch July 9, 2023 21:32

Nostritius mentioned this pull request Jul 25, 2023

Key Config #35

Open

Conversation

maaxxaam commented Mar 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maaxxaam commented Mar 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nostritius left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maaxxaam commented Apr 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nostritius left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nostritius left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Nostritius left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nostritius commented Jul 9, 2023

Uh oh!

Nostritius commented Jul 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

maaxxaam commented Mar 28, 2023 •

edited

Loading

maaxxaam commented Mar 29, 2023 •

edited

Loading

Nostritius left a comment •

edited

Loading

maaxxaam commented Apr 5, 2023 •

edited

Loading