Fix emu screen trashing legacy input path input #2612
Conversation
|
Confirmed: fixes #2609. |
|
Verified @thedax's findings. |
|
This still doesn't fix F3/pause/other keys not working. It also has a compile error. Also, with this, at least controllers work, but not properly. If I hold down "x", the game acts like I just tapped it. Obviously, this makes it very hard to control some games. It does work properly with the keyboard, however. -[Unknown] |
|
@unknownbrackets Those sound like bug but are you sure they are introduced by this pull request? I can try to fix those if you give me more detail. What platform and what was the compile error? |
|
I don't think it's good that you call __CtrlButtonUp(-1); every frame. It'd be better to diff the "keyqueues" (more like lists of held down keys) and send __CtrlButtonUp and Down accordingly. This input merge has turned into a bit of a mess (I get tens of emails complaining of broken controls) ... but we'll sort it out. The general idea is sound and will be useful. |
|
Thank you for taking the brunt of the email blast. I had hoped we would get all these issues out of the way by testing my dev branch but no one showed any interest until it got merged. Diffing the key queue across should be possible and sounds needed. On windows the normal input state is set every frame but it sounds like some special keys are only set on up & down events. |
Yes, both definitely worked in the build right before your pull, fbe8a19. The compile error I got was that KeyMap::IsMapped() was not defined.
Right. It does sample, but if there is time in which the pressed buttons are wrong, the emu thread could theoretically sample during this time. If it did, it would get wrong buttons and the game would think that you released those keys and pressed them again. Especially with a small window of time, this would just cause hard to reproduce glitches. That sort of issue should exist on all platforms that clear the state using e.g. -[Unknown] |
|
Thank you @unknownbrackets I think those commits should address both issues. Could some one on windows test it for me? |
|
F3 still doesn't do anything, but yes, that fixes the buttons not working when held down. I don't really even understand the purpose of the queue. Is it to add latency to the updates? I'm not sure that's really good for rhythm games. Really, the game can sample much more frequently than once per frame so it'd be better to avoid having more buffers and latency, and instead update using events. I'm obviously missing some reason it needs 4 layers of abstraction for. -[Unknown] |
|
Thank you for testing it. I will try to see how windows is handling F3. The queue allows us to support an arbitrary number of keys and a bounded yet arbitrary number of active keys. Another possible solution we discussed was to use bit field. I am not sure if you saw the discussion: #1925 The queue should not add latency. It has an added bonus in providing ordering info to the UI. This makes text input possible and allows the mapping screen to know which key was pressed most recent. Eventing is an interesting idea and sub-frame would be nice. The reason I went with the queue is to provide an abstraction because SDL and android have their input handling inside native. Since I think ector wants to keep native separat they cannot call into anything ppsspp specific. Windows input is handled within the main repo so if someone is interested eventing should be possible to implement. |
|
More simply, the queue isn't a queue, it's just a list of buttons held down on the current frame. Makes it possible to support more inputs than can fit in a bitfield. But it might make more sense to not collect all the inputs once per frame, and as you say, instead just send keyup and keydown events all the way through the mapping system. This should give us slightly lower latency. |
|
Traced the F3 issue to KeyboardDevice.cpp around line 142. Escape has a special case but it looks like F3 also needs to be special cased and routed through the legacy path. So it should be something like: if (key_pad_map[i] == VK_F3) Since this file is only in the windows build I don't feel safe editing it. |
|
A game can set a sampling frequency using It can be as low as 5555 microseconds, which means updating 3 times per frame or so. Any game requesting a frequency other than 16666 is probably currently broken. I'm not saying this was changed by this or any recent pulls. It can select any frequency between 5555 and 20000 microseconds. All of this is handled within sceCtrl. It just needs to know the current value at all times to provide the game with the correct information. So, you're saying that the "legacy" input path needs to be used? Does that mean it is not legacy? -[Unknown] |
|
I'm okay with the legacy path living a few more commits as long as it can later be phased out. Right now it's important to get things working again, I'll merge when you got F3 working again. |
|
@unknownbrackets Both legacy (Yes I know it is a pretentious name) and modern key input are processed whenever EmuScreen::update() gets called. With that in mind pressedLastFrame would be better named pressedLastUpdate @hrydgard Thank you, I should have that done in three minutes. |
Update() can be called at subframe intervals.
|
Just noting that F3 is only the default, as is the case with escape. If I set it to ~, it still doesn't work. Adding an if for F3 isn't going to solve things for people who have configured their keys. Does that mean the legacy input path needs to be taken for all keys? If it's going to be fixed in a short-term way, I should also mention that the backspace and pause keys will need similar treatment, as they are also defaults. -[Unknown] |
|
No it means I need to get started on supporting virtual keys in modern path. That would allow remapping of keys like turbo, back, pause, plus keys for the analog stick |
|
I'll merge to get most of the stuff working again, awaiting a proper fix for mapping other things than the real psp buttons. |
Fix emu screen trashing legacy input path input
You'll move my silly mistake once you see it.