[Scummvm-tracker] [ScummVM :: Bugs] #14610: SHERLOCK: ROSETATTOO: Crash when clicking during flashback scene at Cleopatra's Needle

[ScummVM :: Bugs]

#14610: SHERLOCK: ROSETATTOO: Crash when clicking during flashback scene at
Cleopatra's Needle
-------------------------------------------------+-------------------------
Reporter:  PushmePullyu                          |      Owner:  (none)
    Type:  defect                                |     Status:  new
Priority:  normal                                |  Component:  Engine:
                                                 |  Sherlock
 Version:                                        |   Keywords:
    Game:  Sherlock Holmes: Case of the Rose     |
  Tattoo                                         |
-------------------------------------------------+-------------------------
 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:
 1. Load this save:
 [https://bugs.scummvm.org/attachment/ticket/14580/rosetattoo.015]
 2. Talk to Needhem and select option 1: "You witnessed..."
 3. 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.
-- 
Ticket URL: <https://bugs.scummvm.org/ticket/14610>
ScummVM :: Bugs <https://bugs.scummvm.org>
ScummVM