SCUMM: Missing sentence when Rapp gives the map piece

[dwatteau]

(This was discovered by Threepwang.)

How to reproduce:

1. Start any version of Monkey Island 2 with the 1978 boot-code (any platform, any language, the 1991 release or the Ultimate Edition… as far as I can say).
2. Open Rapp's coffin, and apply Ash-2-Life on his ashes.
3. Choose dialog option 2 (saying that the gas is now off).
4. Rapp gives you his part of the map.
5. When receiving it, Guybrush should say something, but no text is displayed. If you play the Ultimate Edition, you'll hear the associated vocal sample, though, but the text itself is still missing.

This also happens with the original interpreter.

(Note: which sentence Guybrush is supposed to say depends of the number of map pieces you already own. All of these sentences work as intended when you're not triggering them with Rapp.)

I think it's a bug in the original script, as in Bug #2215 for Indy4 (where commit 549eb83986a works around it in ScummVM).

The associated script is LFLF_0019/ROOM/LSCR_0203:

...

(14)     print(7,[Text(sound(0x3B5222F, 0xA) + "Now I can rest in the folds of the earth^")]);
(AE)     WaitForMessage();
(14)     print(7,[Text(sound(0x3B6A5B6, 0xA) + "^like a Steamin' Weenie in a soft, fresh bun.")]);
(AE)     WaitForMessage();
(14)     print(7,[Text(sound(0x3B85B7C, 0xA) + "Ahhh^")]);
(AE)     WaitForMessage();
(80)     breakHere();
(A8)     if (VAR_OVERRIDE) {
(05)       drawObject(199);
(29)       setOwnerOf(1139,0);
(29)       setOwnerOf(1139,15);
(29)       setOwnerOf(1138,0);
(29)       setOwnerOf(1138,15);
(29)       setOwnerOf(1142,0);
(29)       setOwnerOf(1142,15);
(**)     }
(58)     endOverride();
(14)     print(255,[Text(" ")]);
(10)     VAR_RESULT = getObjectOwner(200);
(88)     if (VAR_RESULT != VAR_EGO) {
(25)       pickupObject(200,0);
(1A)       Bit[366] = 1;
(**)     }
(0A)     startScript(109,[200]); # <==
(**)   }
(**) }
(58) endOverride();
(14) print(255,[Text(" ")]);
(0A) startScript(204,[Bit[586]]);
(80) breakHere();

Script 109 is the one starting Guybrush's sentences upon obtaining a part of the map.

Some calls to WaitForMessage() or breakHere() are maybe missing there, but I'm not sure as I don't really understand how this works.

[BLooperZ]

Seems that
print(255, [Text(" ")]);

is delibrately there to erase whatever text is on screen right now.

the called script before does not have WaitForMessage when it return (pretgy much consistent with all scripts)

can probably be patched by either replacing the empty print with noop
or placing WaitForMessage() before/in-place

[eriktorbjorn]

WaitForMessage() would probably delay the animation of Rapp turning back to ashes, so not printing the space is probably better. I don't dare patching the script when it's loaded, because I don't know how that would affect localized versions of the game. (I think the strings are hard-coded in the scripts.) So you'd probably have to detect at runtime when the print happens, and ignore it. A bit tricky since it happens a couple of times during that script.

Something like this seems to work, but could use more testing:

diff --git a/engines/scumm/string.cpp b/engines/scumm/string.cpp
index d3a53dfc42..74dfe27c36 100644
--- a/engines/scumm/string.cpp
+++ b/engines/scumm/string.cpp
@@ -55,6 +55,22 @@ namespace Scumm {
 void ScummEngine::printString(int m, const byte *msg) {
        switch (m) {
        case 0:
+               // WORKAROUND bug #12734: The script tries to clear the currently
+               // displayed message after Rapp gives you the map, but that means
+               // you'll never see Guybrush's reaction to finding a map piece.
+               //
+               // It's a bit hard to pin down the exact case, since it happens
+               // at a few different points during the script. We limit it to
+               // when the player has the map piece.
+               //
+               // We have to do it here, because we don't want to delay the
+               // animation of Rapp turning back to Ashes.
+		if (_game.id == GID_MONKEY2 && _roomResource == 19 &&
+			vm.slot[_currentScript].number == 203 &&
+			_actorToPrintStrFor == 255 && strcmp((const char *)msg, " ") == 0 &&
+			getOwner(200) == VAR(VAR_EGO) && VAR(VAR_HAVE_MSG)) {
+			return;
+		}
+

[eriktorbjorn]

I've submitted this as ​https://github.com/scummvm/scummvm/pull/3197

I don't know how much time I will have for ScummVM in the next few weeks, and I don't want to risk forgetting all about the issue. Hopefully there will be some feedback. Maybe even ideas for better ways of doing it.

[eriktorbjorn]

It was pointed out that I forgot to make the fix specific to Monkey Island 2. Oops! I've fixed that in the pull request.

[dwatteau]

Thank you very much!

I had done some local tests with some hand-patching of the original LFLF_0019/ROOM/LSCR_0203 script.

With BLooperZ's first suggestion, I changed:

print(255,[Text(" ")]);

to this:

WaitForMessage();
print(255,[Text(" ")]);

and now the text is printed. But indeed, if you do it this way, Rapp "waits" for your lines before doing his final goodbye animation, and it's a bit strange, especially if Rapp's map is the last one, because Guybrush speaks even more, then.

So I tried doing it a bit differently.

Instead of doing this in the original script:

(0A)     startScript(109,[200]);
(**)   }
(**) }
(58) endOverride();
(14) print(255,[Text(" ")]);
(0A) startScript(204,[Bit[586]]);
(80) breakHere();
(68) VAR_RESULT = isScriptRunning(204);
(28) unless (!VAR_RESULT) goto XXXX;
(48) if (VAR_VERB_SCRIPT == 14) {
(1A)   VAR_VERB_SCRIPT = 201;
(0A)   startScript(16,[2]);
(**) }
(2C) CursorShow();
(2C) UserputOn();
(80) breakHere();
(A0) stopObjectCode();

I did this:

(**)     (nothing)              # <== not starting script 109 here anymore (1)
(**)   }
(**) }
(58) endOverride();
(14) print(255,[Text(" ")]);
(0A) startScript(204,[Bit[586]]);
(80) breakHere();
(68) VAR_RESULT = isScriptRunning(204);
(28) unless (!VAR_RESULT) goto XXXX;
(A8) if (Bit[366]) {            # <== only start script 109 there, if the map was given (2)
(0A)   startScript(109,[200]);  # <== (2)
(AE)   WaitForMessage();        # <== (2)
(**) }                          # <== (2)
(48) if (VAR_VERB_SCRIPT == 14) {
(1A)   VAR_VERB_SCRIPT = 201;
(0A)   startScript(16,[2]);
(**) }
(2C) CursorShow();
(2C) UserputOn();
(80) breakHere();
(A0) stopObjectCode();

i.e. I start Guybrush's lines once Rapp is done animating.

The problem with my "solution" is that it's a bit more invasive so it's probably harder to work around in ScummVM, and it's potentially more risky. Your suggestion is also probably closest to the original intent.

I'll test your PR as soon as possible. The case I'm most interested is when Rapp's map is the last one you get. Guybrush has a couple more lines in that case, so I'd like to make sure that it doesn't cause any problem.

Thanks!

[dwatteau]

Well, actually it was quick for me to test that case with your PR.

It works well with my French version, even when it is the last map piece, and it doesn't seem strange in the way it is rendered.

I can't judge the PR in itself, but from a "playability" viewpoint it seems all good to me. Thanks again!

[dwatteau]

PR 3197 fixes this and has been merged, so this can be closed.

Thank you!