Skip to content
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

SCUMM: Replicate the original clock tower behavior (COMI) #4267

Conversation

dwatteau
Copy link
Contributor

@dwatteau dwatteau commented Sep 12, 2022

EDIT: This PR was improved a bit later on. Please see below.

(original PR; obsolete)

In Monkey Island 3, if Guybrush reads the clock in Puerto Pollo, he'll imitate a speaking clock. ScummVM disables this for the German release though, since the result was poor for that language, and because it was disabled in that original release (see Trac#1493).

I see that the official French release did the same thing, back then (tested with my French CDs in a Windows 98 VM, and on Windows 10 running the original interpreter and then using DREAMM).

Indeed, the result isn't really good in French, either. For example, Guybrush will say "sept-oh-un et trente-un secondes" while our speaking clock would say "dix-neuf heures, une minute, trente-et-une secondes".

(Dedicated to the French speaking clock, which was closed just a few weeks ago.)

How to test

  1. Have the French version of COMI (I believe it's on Steam and GOG).
  2. Start the game with boot param 226.
  3. Go to Puerto Pollo (= the first city), look at the clock tower.
  4. Guybrush should now say "Whoa! Il se fait tard. Je dois m'en aller." (French for "Look at the time, gotta scoot!") instead of reading the clock. This should be the same behavior as the original French interpreter.

@dwatteau dwatteau force-pushed the fix/scumm-comi-disable-speaking-clock-french-version branch from c16b242 to d469b3b Compare Sep 12, 2022
@AndywinXp
Copy link
Contributor

AndywinXp commented Sep 12, 2022

Now I'm curious about how the original interpreter did it. Since the interpreter is multilanguage (except for CJK languages) I think we should check how they did it (and for which languages), so to correctly match this and possibly other exceptions.

@AndywinXp
Copy link
Contributor

AndywinXp commented Sep 12, 2022

...oh. Substituting the standard exe with the CJK one yields the "look at the time, gotta scoot!" line. That's a good place to start I guess.

@AndywinXp
Copy link
Contributor

AndywinXp commented Sep 12, 2022

Okay, so here's how it's done on the original (and personally I think it might be prettier this way, what's your opinion on this?).

In what would be our ScummEngine_v7::actorTalk(const byte *msg):

// (this is done at the very beginning of the function...)
if (_vm->_language != Common::EN_ENG && _vm->_language != <other languages which allow the full clock lines> ) {
    // Check if we are receiving exactly this string tag and substitute it with the "gotta scoot" message.
    // Please note, we are comparing the *non translated* message, so exactly our "msg" input parameter.
    if (!strncmp(msg, "/CKGT326/", 9) {
        msg = "/VDSO325/Whoa! Look at the time.  Gotta Scoot.";
    }

    // After the substitution above, the following check should fail, and actorTalk should resume normally.
    // Reject every line which begins with the CKGT tag (which stands for "ClocK Guybrush Threepwood").
    if (!strncmp(msg, "/CKGT", 5)) {
        return;
    }
}

// Execute actorTalk normally...

The languages that allow the full clock lines that I know for certain are english and italian, but there might be more. Can someone check spanish on the original interpreter?

EDIT: Of course all of this would be wrapped in a big if (_vm->_game.id == GID_CMI) check 😄

@dwatteau
Copy link
Contributor Author

dwatteau commented Sep 12, 2022

I'm not very at ease with SCUMM > v6, but I'm fine with whatever you think is better. The original workaround was added in 2004, so there's maybe something to be improved, yeah.

It also looks like there was a Brazilian Portuguese release (from BraSoft) with English audio but Portuguese text.

@AndywinXp
Copy link
Contributor

AndywinXp commented Sep 12, 2022

I'm not very at ease with SCUMM > v6, but I'm fine with whatever you think is better. The original workaround was added in 2004, so there's maybe something to be improved, yeah.

It also looks like there was a Brazilian Portuguese release (from BraSoft) with English audio but Portuguese text.

Thanks for the heads up! So we're currently missing:

  • Spanish
  • Portuguese

Am I missing anything else? English and Italian allow the lines while all the other remaining languages trigger the interpreter hack. The Russian Akella conversion also allow the lines since it uses the eng interpreter IIRC.

@AndywinXp
Copy link
Contributor

AndywinXp commented Sep 13, 2022

Update, it has been confirmed that both spanish and portuguese intepreters have the hack, so the only allowed languages are english, italian and russian.

@dwatteau dwatteau force-pushed the fix/scumm-comi-disable-speaking-clock-french-version branch from d469b3b to c013f8d Compare Sep 13, 2022
@dwatteau dwatteau changed the title SCUMM: Disable the speaking clock reaction from Guybrush in French COMI SCUMM: Replicate the original clock tower behavior (COMI) Sep 13, 2022
@dwatteau
Copy link
Contributor Author

dwatteau commented Sep 13, 2022

Thank you very much @AndywinXp. I've pushed the more comprehensive solution that you've suggested above, then. Does it look good to you?

(Tested against my French and English COMI).

@AndywinXp
Copy link
Contributor

AndywinXp commented Sep 13, 2022

This works as expected and looks tidy, thanks!

EDIT: I just have one minor observation to make: we shouldn't limit the behavior to a room, as the clock is reachable from more than one room.

@dwatteau
Copy link
Contributor Author

dwatteau commented Sep 13, 2022

EDIT: I just have one minor observation to make: we shouldn't limit the behavior to a room, as the clock is reachable from more than one room.

Oops you're right, I forgot that.

I was doing that (and I could complete the room list) as a small optimization so that we don't run strncmp() on every string during the game, but maybe it's just useless for a v8 game where many bigger things are constantly happening?

@AndywinXp
Copy link
Contributor

AndywinXp commented Sep 15, 2022

I was doing that (and I could complete the room list) as a small optimization so that we don't run strncmp() on every string during the game, but maybe it's just useless for a v8 game where many bigger things are constantly happening?

I personally think that the performance overhead should be minimal in this case, correct me if I'm wrong but strcmp should have a O(n) complexity, with n being the number of characters compared. We are comparing strings for no more than 9 characters, so we should be good without restricting the check to a room.

@dwatteau
Copy link
Contributor Author

dwatteau commented Sep 15, 2022

I personally think that the performance overhead should be minimal in this case, correct me if I'm wrong but strcmp should

OK thanks, it makes sense. Moreover, actorTalk isn't called hundred of times per second so… and if the original interpreter did it this way too, I'm going to simplify my check then. Thank you

@dwatteau dwatteau force-pushed the fix/scumm-comi-disable-speaking-clock-french-version branch from c013f8d to 4e34b41 Compare Sep 15, 2022
@AndywinXp
Copy link
Contributor

AndywinXp commented Sep 15, 2022

Thanks 🙏 I'm gonna ask just one more favour (please don't kill me 😄), could you please squash the two commits into the last one? (Having a commit referencing the old workaround might be confusing)

I'll gladly pull the trigger and merge the PR immediately after that!

Guybrush can read the clock of Puerto Pollo, but this was only enabled
in the English, Italian and (fan-made) Russian releases, probably
because the result was poor for the other languages.

The German, French, Spanish, Brazilian Portuguese and CJK releases
disabled this feature, but the check was done in the interpreter, not in
the scripts.  This replicates this original behavior.

Findings and ScummEngine_v7::actorTalk() change by AndywinXp.
@dwatteau dwatteau force-pushed the fix/scumm-comi-disable-speaking-clock-french-version branch from 4e34b41 to 97c0002 Compare Sep 15, 2022
@dwatteau
Copy link
Contributor Author

dwatteau commented Sep 15, 2022

Sure! Done.

And thanks in advance for your merge :)

@AndywinXp AndywinXp merged commit b41d3f4 into scummvm:master Sep 15, 2022
8 checks passed
@AndywinXp
Copy link
Contributor

AndywinXp commented Sep 15, 2022

Great job, thanks!

@dwatteau dwatteau deleted the fix/scumm-comi-disable-speaking-clock-french-version branch Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants