SHERLOCK: Mouse clicks may be ignored

[antoniou79]

Tested on Windows 10 x64, and Android 12 (smartphone) with ScummVM 2.8.1 (2.8.1.1 on Android) and also ScummVM 2.9.0git from recent master HEAD.

This is an issue with both "The Lost Files of Sherlock Holmes" games: "The Case of the Serrated Scalpel" and "The Case of the Rose Tattoo". It can occasionally be observed in the demo we have on the ScummVM website for the Serrated Scalpel.

This does not happen consistently but it happens frequent enough to be annoying (I've only tested with early parts of the games, maybe later on it becomes more of an issue, since these games feature mini games and various user interface modes).

On Android this tends to happen more frequently (maybe due to the special way we have in place to differentiate taps from long touches or double taps etc).

How to reproduce:
Start a new game, reach proper gameplay (after the intro cutscenes) and start interacting with the interface and the screen hotspots. *Sometimes* the game will ignore the users clicks even though user has control (ie. it's not during a cutscene or during a script that disabled user controls).

From a look through the engine code, I think I've spotted the culprit but fixing it will require someone very familiar with the engine -- hopefully not a complete overhaul of the input handling system.

The culprit seems to *mainly* be this method's "Events::delay()" implementation:
​https://github.com/scummvm/scummvm/blob/d0336f568e3101dc480363451a76257dc2d3d4ce/engines/sherlock/events.cpp#L333
The delay method implements a blocking delay. It distinguishes between a short delay and a longer delay, but in both cases it calls the method Events::pollEvents(). For the longer delays (10 ms or more), it breaks the delay in 10ms segments and iterates them while looping for the total duration with each iteration calling pollEvents().

This method Events::delay() seems to be called in numerous places in the engine code, either directly or via the "Events::wait(int numFrames)" method.

The method Events::pollEvents():

• Handles screen updates via checkForNextFrameCounter() (this is also an issue when attempting to fix the situation)
• Consumes the next pending event
• Updates the mouse position

Calling pollEvents() frequently results in smooth mouse movement (that the original games *do not have*). But since in this case pollEvents is called while in a blocking wait, it can consume click events without the engine ever properly handling them. (The keyboard events are pushed in a special queue (_pendingKeys) so presumably those are properly handled eventually; I am only focusing on missing mouse clicks in this report).

During my debugging the repeated calls to pollEvents during executing a blocking "wait" would consume the mouse down and up events before the blocking wait would conclude, thus the rest of the code remained unsuspecting that a click event occured.

Since for the early scenes of the games the events.wait() ScalpelScene::doBgAnim() and TattooScene::doBgAnim() seemed to be mainly "eating up" the mouse clicks, I've attempted a partial fix by replacing those waits with a similar method that would not call pollEvents(). But, this also required manually placing pollEvents() in other parts of the engine code because it expected pollEvents() to have been run after a doBgAnim() call, and doing a checkForNextFrameCounter() during the wait to maintain how the engine updated the screen. And as an expected side-effect, the mouse movement also became sluggish. However, this did make the engine not miss those clicks (tested in Windows and Android).

This would not be a complete fix, since there's other places where wait() and delay() are used and would need to be fixed. (For example in "Serrated Scalpel" this would need to be done at least for the Journal, Setup Menu, maybe the map, darts game(?) and perhaps elsewhere). I suspect that direct calls for delay() won't need "fixing" because practically the issue comes from the longer delays that are cause by wait() (since wait has a number of frames arguments).