Opened 15 months ago
Closed 12 months ago
#14610 closed defect (fixed)
SHERLOCK: ROSETATTOO: Crash when clicking during flashback scene at Cleopatra's Needle
Reported by: | PushmePullyu | Owned by: | PushmePullyu |
---|---|---|---|
Priority: | normal | Component: | Engine: Sherlock |
Version: | Keywords: | ||
Cc: | PushmePullyu | Game: | Sherlock Holmes: Case of the Rose Tattoo |
Description
Tested with master 645230091f29ddcb73474cd820581e5f739afa15 on Linux x86_64 using the English CD version of Rose Tattoo.
The flashback scene with Needhem at Cleopatra's Needle uses the following talk files via talkTo():
from SherlockEngine::sceneLoop(): Need18a from SherlockEngine::sceneLoop() Tattoo::TattooScene::doBgAnim() BaseObject::checkObject(): movi68s // This disables the mouse via TattooTalk::cmdMouseOnOff(), see below Boom68d Kick68d Prus68d Fade68d // This enables the mouse via TattooTalk::cmdMouseOnOff(), see below from SherlockEngine::sceneLoop(): Fade68d // after changing scene, with _scriptMoreFlag set to 1 and _scriptSaveIndex set to 380 from SherlockEngine::sceneLoop() Tattoo::TattooScene::doBgAnim() Sprite::checkSprite(): Strt18s.tlk // This should now be called with _scriptMoreFlag unset, but see below
Starting a walk action by left clicking on a valid target position right before the first talkTo() call for "Fade68d" will hit this code:
engines/sherlock/talk.cpp:176 in talkTo():
if (people[HOLMES]._walkCount || (!people[HOLMES]._walkTo.empty() && (IS_SERRATED_SCALPEL || people._allowWalkAbort))) { // Only interrupt if trying to do an action, and not just if player is walking around the scene if (people._allowWalkAbort) { abortFlag = true; // an arrow zone might have been clicked before the interrupt, cancel the scene transition ui._exitZone = -1; } people[HOLMES].gotoStand(); }
In addition to stopping the walk action via gotoStand(), this also sets abortFlag (_allowWalkAbort is always true in Rose Tattoo), which in turn causes _talkToAbort to be set:
engines/sherlock/talk.cpp:462 in talkTo():
_talkToAbort = abortFlag;
The next talkTo() call for "Fade68d" will now return early due to _talkToAbort, and the _scriptMoreFlag intended for this call will remain set.
When calling talkTo() for "Strt18s.tlk", this flag causes doScript() to incorrectly use the index in _scriptSaveIndex, which leads to interpreting out-of-bounds data as a script, possibly crashing the game. The earlier talkTo() calls are not affected because doBgAnim() resets _talkToAbortFlag.
The original game does not exhibit the problem: it completely disables player control and hides the cursor in "movi68s" via opcode 0xAE (TattooTalk::cmdMouseOnOff()) and undoes this in "Fade68d". In ScummVM however, the opcode only hides the cursor (which is immediately shown again).
To reproduce:
- Load this save: https://bugs.scummvm.org/attachment/ticket/14580/rosetattoo.015
- Talk to Needhem and select option 1: "You witnessed..."
- Near the end of the flashback scene, when the two men are leaving: keep spam-clicking somewhere Sherlock could walk to
If you are unlucky in causing a crash, setting a breakpoint in Sherlock::Talk::talkTo(Common::String) with the condition
filename._str[0] == 'S' && _scriptMoreFlag
will show if the bug was triggered.
Change History (3)
comment:1 by , 15 months ago
Owner: | set to |
---|---|
Resolution: | → pending |
Status: | new → pending |
comment:2 by , 14 months ago
Forgot to link the DOS save (so original behavior can be verified): https://bugs.scummvm.org/attachment/ticket/14580/HOLMES2.SAV
comment:3 by , 12 months ago
Resolution: | pending → fixed |
---|---|
Status: | pending → closed |
PR for this is here: https://github.com/scummvm/scummvm/pull/5318