KBM-only remapping #1797
KBM-only remapping #1797
Conversation
kalaposfos13
force-pushed
the
kbm-only
branch
from
cc784e4 to
6da09ae
Compare
|
This was my attempt at designing a GUI for this. Since there's so many functions (ability to add up to 3 inputs per output is the most painful one to design), I can't seem to make it take a smaller space. The intended use for this is that the dropdown boxes list down the possible inputs per output (since there can be up to three), and the user can press the corresponding input button, then input up to three keystrokes to fill in each combobox. Design wise this seems to be the only thing I can think of to support the things this program wants to do, but I am unable to code it this way. As such, I'm at least putting the UI file here, and maybe someone else can code in the input saving and such. (saved as txt file, rename to ui to use) |
|
@rainmakerv3 I think it's worth combining at least the keyboard, since some people can play on laptops or they don't have a controller, it should have been added a long time ago, and as for the graphical interface, we haven't made significant progress in this direction yet, let's leave it for later. |
|
Yes I agree. My thought is that this would be a good place to leave this for anyone who wants to use it. But like you, I also believe this is good enough to merge in its current state |
| BindingConnection toggle_connection = | ||
| BindingConnection(InputBinding(toggle_keys.key2), toggle_out, toggle_keys.key3); | ||
| connections.insert(connections.end(), toggle_connection); |
There was a problem hiding this comment.
| BindingConnection toggle_connection = | |
| BindingConnection(InputBinding(toggle_keys.key2), toggle_out, toggle_keys.key3); | |
| connections.insert(connections.end(), toggle_connection); | |
| connections.emplace_back(InputBinding(toggle_keys.key2), toggle_out, toggle_keys.key3); |
Less lines and constructs object in-place. Ditto for all other cases of insert() like this
| return GetMouseWheelEvent(e); | ||
| case SDL_EVENT_GAMEPAD_BUTTON_DOWN: | ||
| case SDL_EVENT_GAMEPAD_BUTTON_UP: | ||
| return (u32)e.gbutton.button + 0x10000000; // I believe this range is unused |
There was a problem hiding this comment.
| return (u32)e.gbutton.button + 0x10000000; // I believe this range is unused | |
| return (u32)e.gbutton.button + 0x10000000; // I believe this range is unused |
What is this magic value? Could we get a constexpr variable with a name instead?
There was a problem hiding this comment.
It's an offset so the SDL ranges for different types of inputs don't overlap (they all start at 0, and I have to differentiate between them somehow) Keyboard and mouse got the no offset range, as they were the first to be implemented, then for controller buttons, I just chose this as it didn't overlap with anything, and for axes I chose a large offset (0x80000000) so when sorting them they will be at the end of the list, and that is important for reasons. I think a cleaner way to do it would be to finally make this a struct and just give up on storing everything in a single u32 (id, input type, analog value for axes)
| // solution 1: add it to the keycode as a 0x0FF00000 (a bit hacky but works I guess?) | ||
| // I guess in software developement, there really is nothing more permanent than a temporary | ||
| // solution | ||
| value_mask = (u32)((e.gaxis.value / 256 + 128) << 20); // +-32000 to +-128 to 0-255 |
There was a problem hiding this comment.
Ditto for above, this code is hard to understand at a glance
src/sdl_window.cpp
Outdated
| using Libraries::Pad::OrbisPadButtonDataOffset; | ||
|
|
||
| // get the event's id, if it's keyup or keydown | ||
| bool input_down = event->type == SDL_EVENT_KEY_DOWN || |
There was a problem hiding this comment.
| bool input_down = event->type == SDL_EVENT_KEY_DOWN || | |
| const bool input_down = event->type == SDL_EVENT_KEY_DOWN || |
Ditto for other cases of const correctness
| } | ||
| } | ||
| // LOG_DEBUG(Input, "Input activated for the first time, adding it to the list"); | ||
| pressed_keys.insert(pressed_keys.end(), {value, false}); |
There was a problem hiding this comment.
This inserts the key into pressed_keys without checking is_pressed, is it okay?
| pressed_keys.insert(pressed_keys.end(), {value, false}); | |
| pressed_keys.emplace_back(value, false); |
| if (value == (u32)-1) { | ||
| return false; | ||
| } | ||
| if ((value & 0x80000000) != 0) { |
There was a problem hiding this comment.
This value seems has special meaning depending some bits. Please make a bitfield struct describing its layout and use that. Manual bit parsing of random parts can be hard to memorize
| if (!key1) | ||
| key1 = key_id; | ||
| else if (!key2) | ||
| key2 = key_id; | ||
| else if (!key3) | ||
| key3 = key_id; |
There was a problem hiding this comment.
This can be a lambda defined in the function to reduce code duplication
|
@raphaelthegreat I did the easy changes, and I'll do the rest tomorrow. So far I've only one comment on this: |
|
You can use projections in ranges to simplify it to |
This is a modified version of my remapping PR that only contains the keyboard and mouse part, as this is already complete and has proven to be mostly bug-free, as it has been extensively tested by people using @diegolix29's fork. The controller part is still WIP and is not ready for merging yet, so until that's ready too, I'll just put this out here following @rainmakerv3's advice. The code still has some controller remapping elements left in, but they aren't hooked up to the main input loop. If I left in anything that I shouldn't have, please let me know.