-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
BACKENDS: Add double-click time feature to support OS-configurable double-click intervals, implement it for Windows. #4460
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
Conversation
|
It's been a while since I've played with the EventRecorder, so apologies if I'm missing a detail on this. It looks like this forces OSystem implementations to cache doubleclick time since EventRecorder is storing it in one place. I would expect OSystem::getDoubleClickTime() to query the platform directly and leave it up to engines if they wants to cache that or not. If EventRecorder could treat getDoubleClickTime() as an event that it logs the responses of and responds with them during playback, I think you can remove _haveDoubleClickTime, processHaveDoubleClickTime(), and even the kFeatureDoubleClickTime. If it weren't for EventRecorder, I think this would just be "getDoubleClickTime() queries the platform, returns 0 if not implemented." I don't know how feasible it is to make EventRecorder treat this query as an event, but I would prefer the complexity to be EventRecorder's burden and keep OSystem/backends simpler. On the Windows side, I recall GetDoubleClickTime() being around in the Win9x days, so no compatibility issues there. =) |
|
I am a bit lost here. What is the code supposed to do? After studying it, I see the following:
What is the intended usage of this feature? |
On some OS (Windows and macOS for sure) users can set the double-click interval in the operating system's mouse settings. It's meant as an accessibility feature but it is also just good practice to ask the OS what the double-click time is to make it consistent with what the user is used to. Even if the user doesn't change it, the default isn't even the same on all OS (e.g. Windows is 500ms, macOS is 300ms). So, this adds a way for engines to get the OS double-click time, and there are a few that could use it (I'm going to add it to mTropolis, where MTI can use it). The time is also stored in event recordings so that when playing back recordings, the double-click time is treated as the same as it was on the recording machine to prevent any inconsistencies. |
|
I think adding a way for engines that use double-click events to query the Operating System double-click time is a good idea. If the The changes for the event recorder look a bit intrusive though. Since the event recorder adds its setting to the |
|
I appreciate the idea of this change. Respecting the OS double-click speed would greatly help regarding accessibility in case the user has special requirements regarding the double-click speed. |
|
Note that when we discussed this on discord, two ideas were raised:
Both should hopefully mean that the OS setting is used to detect double-clicks, but one has the logic in the backends and the other in each engine that needs it. I can see some advantages to either approach. For example having double-clicks detection in the backend and identified in |
I was going to try the latter route but it ran into numerous problems. The main issue is that if click count is expected to be in Event, then all backends need to support it. SDL1 doesn't produce click counts, and some platforms convert non-mouse inputs into clicks. Intercepting converted mouse events and updating their click count was turning out to be very complicated, especially since converted clicks and actual clicks can coexist on the same platform. At that point, there would need to be a way to determine double-click independent of the SDL event queue anyway, so it doesn't really even avoid the problem. Just querying the time and having the engine figure it out from click events is much simpler. Anyway, I'll make some updates to this to address feedback. I've been trying to figure out if such a thing exists on Linux and it sounds like it can be retrieved from the XSETTINGS "Net/DoubleClickTime" key, maybe? https://www.freedesktop.org/wiki/Specifications/XSettingsRegistry/ |
1e77821 to
6898b50
Compare
|
Apparently, there is a misunderstanding, Particularly from people that say "nice change". Let me elaborate a bit. I see that this PR is merely exposing the value via the OSystem API. There is no demonstrated usage. This could be fine, as we are careful with changing the OSystem API, however, I'd like to understand what will happen to it later. Doubleclicks. The general approach to double-clicks is the following (apologies for explaining it here, just for the record):
Ugh, that is complicated. Now the question is, where to detect those? We have two options:
If we offload this burden of tracking the clicks on every engine, I personally find it suboptimal because you will have to reimplement and test that complicated logic every time. I suggest implementing this functionality witin the event manager, so it is completely transparent to the engines. The engines will then receive Also, if some backend supports double clicks, we may offload this to them, because they will do all this grunt work for us and just generate events. Of course, some engines have game scripts detecting double clicks. To address this, I would implement double clicks as a turnable feature. E.g. you tell the EventMan to turn on the double clicks, and it will, in turn, tell it to the baclend Now, the question is. Would it be possible to implement this as part of this PR, or is this too much to ask? However, if we just merge it without implementing the usage, I am afraid we may end up with just a dead code. |
|
A few more thoughts on this:
I'm an odd case, but I would end up using this as-is due to how complex mouse handling is in Ultima 8. It even has a case where if left and right mouse are clicked before the timeout, it produces a jump. |
Muppet Treasure Island has some things that depend on double-clicks, right now it's using a hard-coded 500ms time, and I'd like to get it from the OS instead, so that's the first usage, but there are some other engines with double-click detection too that could also use it.
I think we need to support both scenarios because there are possibly quirks in how engines need handle it. e.g. what happens if the user clicks more than twice in the double-click window? In mTropolis, there is a "clickcount" var that increases until the timer from the first click expires, so it can go higher than 2. But, it's possible that a game may instead want to reset the timer as soon as a double-click is detected.
I think the SDL approach of putting a click count in the mouse event is the best option.
I want to add it to the mTropolis engine already, so it won't be dead code. I can add some EventManager handling to this if you want, but I think it'd be better to do it separately because I already tried doing that and it more complicated than expected because of remapped events (e.g. gamepad inputs converted to mouse events). |
|
@elasota Do you have in mind eventually implementing the EventManager change? Because if we just merge it without the established future plans, I'm afraid it could sit just as an unused code complication that is easy to break without a proper test case. Speaking of the testcase, once implemented, this functionality also must be covered in the testbed engine. |
|
@elasota ping? |
|
I can add it to testbed some time this week. I guess since it doesn't look like there are actually any other event managers than DefaultEventManager that it should be feasible to do multi-click detection there, but as mentioned previously, it needs some further investigation re: how to handle non-mouse-click events converted into mouse click events. |
…uble-click intervals, implement it for Windows.
…th event recorder simpler.
22207f4 to
98f2998
Compare
98f2998 to
5fb11cd
Compare
|
Thank you! |
Definitely not sure the changes for the event recorder are in good places, open to suggestions.
This can be implemented on macOS as well with doubleClickInterval, see https://developer.apple.com/documentation/appkit/nsevent/1528384-doubleclickinterval