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: Fix two bugs when Indy and his father escape from the zeppelin (FM-TOWNS) #3931

Conversation

dwatteau
Copy link
Contributor

@dwatteau dwatteau commented Jun 1, 2022

Following up PR #3930… Still not critical for the 2.6.0 release.

I still don't have an FM-TOWNS Indy3, so I try to emulate and work around some old FM-TOWNS bugs by tweaking and dissecting my PC VGA version, eh. Real FM-TOWNS testers very welcome!

Again, this comes from ATMachine's very detailed comparisons:
https://web.archive.org/web/20110926050144/http://home.comcast.net/~ervind/ij3fmtowns.html
https://web.archive.org/web/20111015233333/http://home.comcast.net/~ervind/ij3misc.html

ij3towns68.png

Wrong text color when Indy and his father escape the zeppelin

Indy's text is always yellow in this game, except for this scene, when he finds his father hiding in the biplane. They both same the same "undefined" color, which is green, here. It's a bit confusing, you don't really know who's talking because of this.

This is what happens in the PC VGA version:

[...]
[0018] (33) SetPalColor(14,10)
[001E] (14) print(255,[Color(10),Text("Dad!  How'd you get here before me?")]); # <== Indy
[0047] (AE) WaitForMessage();
[0048] (13) ActorOps(1,[Costume(0)]);
[004D] (33) SetPalColor(10,10)
[0053] (14) print(255,[Text("Don't ask!" + wait() + "I didn't know you could fly a plane, Junior!")]); # <== Henry Sr.
[008F] (AE) WaitForMessage();
[0090] (33) SetPalColor(14,10)
[0096] (14) print(255,[Text("Fly, yes.  Land, no.")]); # <== Indy
[00AE] (AE) WaitForMessage();
[00AF] (14) print(255,[Text("Hold on!")]); # <== Indy
[...]

The text is not attached to any actor, and they didn't set an explicit color for the last three sentences, although they're really meant to be said by different characters.

But it still works, in the PC VGA version! Maybe because of the palette-shifting effects going on (mainly for the propeller). There are many SetPalColor() calls in that room. We know that the FM-TOWNS version disabled the rotating effect on the propeller which was made with palette-shitfing, though. So I guess this is somewhat related to the incorrect colors for the text in this version.

So the first commit just forces a color for each line in this script (as I had already done for a Mandible line in Loom). The problem is that we don't have anything telling us if a line is meant to be Indy's or his father's (there's no actor number).

But we see that Henry Sr's line is the only one using the wait() instruction in this script. And he's the only one who would say "Junior". So I look for this to detect Henry Sr's line. Everything else is Indy's. This should work for translated versions, too.

Verb leftovers during the same cutscene

In the screenshot above, you can see that the Walk to, Talk and Travel verbs are still visible on the screen, although the inventory and verb interface should be off since we're in a cutscene. But they failed to call cutscene([1]) (they just do cutscene()).

I can't say why this problem appears in the FM-TOWNS version but not in the PC VGA version. I also don't know if this problem appears with the original FM-TOWNS interpreter. That would be good to know. EDIT: happens under UNZ too.

And yeah, there's a third problem in that scene: the propeller is not animated, but if the FM-TOWNS porters gave up on this while having access to the full source, then I'm not going to try this until eBayers stop selling this version at gold prices :)

How to test

  1. Have an FM-TOWNS Indy 3 and be brave enough to confront and escape the zeppelin
  2. (testing the Japanese FM-TOWNS version would be nice too)
  3. You can cheat by running room 80 but it'd be better to properly exit the zeppelin from the maze, so that the room is completely initialized as it should.

Indy's text should be yellow. Henry Sr's text should be green. The verbs shouldn't be visible while you're flying in the biplane. Everything should go back to normal once you hit the floor.

Please don't merge until an FM-TOWNS tester confirms that this is OK (EDIT: done below).

@einstein95
Copy link
Contributor

@einstein95 einstein95 commented Jun 1, 2022

I've had a look into these bugs in the EN and JP FM-Towns versions, and it's really surprising.

Wrong text color when Indy and his father escape the zeppelin

  • Is present in EN
  • Is present in JP

JP has the SetPalColor calls like DOS VGA but still has the bug. Script 201 is identical except for the text being in Japanese. EN does not have these calls.
This PR fixes the bug in both versions.

Verb leftovers during the same cutscene

  • Is present in EN
  • Is present in JP

This PR fixes the bug in both versions.

Propeller using glitched palette

  • Is present in EN
  • Is present in JP

I was surprised to not see this bug in the JP version. The issue arises from the EN version, instead of calling startScript(200,[],F) like the VGA version (which is identical between the VGA and JP versions), instead doing:

[0008] (33) SetPalColor(7,10)
[000E] (33) SetPalColor(7,13)
[0014] (33) SetPalColor(7,5)
[001A] (33) SetPalColor(7,2)
[0020] (33) SetPalColor(7,14)

This seems like an early hack to try and disable the palette cycling, and instead messing the whole thing up.

@dwatteau
Copy link
Contributor Author

@dwatteau dwatteau commented Jun 1, 2022

Thank you very much for your tests!

Happy to see that looking for the wait() instruction makes the workaround work for the Japanese version too :)

The propeller behaving differently between the two versions on the same CD is very interesting! It was probably a very late test/change… That reminds me of the original bug that PR #1343 fixed, where it looked like the FM-TOWNS porters forgot to remove a temporary test. (By the way, for that Monkey2 FM-TOWNS issue, I wonder if the Japanese version was impacted too… if it wasn't, that change may actually cause an issue.)

If it's "just" a matter of fixing some scripts on the fly in that room by looking at what the Japanese version did, then the propeller could probably be fixed as well (I just hope that the original background resource is the same between the two versions). But if several scripts are involved, I'd rather own this version for an easier analysis, instead of trying to guess how the FM-TOWNS version messed things up, just by extrapolating things from my PC VGA resources :) Trying to fix too many things with an invisible hammer is a bit risky.

I'll watch out Japanese auction websites in case a non-millionaire version of Indy3 FM-TOWNS appears at some point.

@dwatteau dwatteau marked this pull request as draft Jun 2, 2022
…zeppelin (FM-TOWNS)

Most lines in this cutscene miss their color parameter, which makes
both actors speak with the same color, which is confusing since one of
them suddenly appeared, and you can barely see them.

(For some reason, the PC VGA version also misses the color parameters,
but it works there, maybe because there's some palette-shifting going
on, which has been mostly removed in the FM-TOWNS version?).

We can fix this by always giving an explicit color to Indy's and his
father's lines. But the lines are not attached to any actor, and it's
hard to determine who's who, especially if we want to support
translations too. Henry Sr. is the only one using a wait() instruction
in his sentence, and he's the only one saying "Junior", so we try to
detect that.
@dwatteau dwatteau force-pushed the fix/indy3-fmtowns-buggy-biplane-when-exiting-zeppelin branch from c8ced3d to ee1a289 Compare Jun 2, 2022
@dwatteau
Copy link
Contributor Author

@dwatteau dwatteau commented Jun 3, 2022

I think I may have found a better fix for the verbs being still on-screen on FM-TOWNS while leaving the zeppelin.

VAR_CUTSCENE_START_SCRIPT is 22 in this game, and it does this:

[0000] (2C) CursorSoftOff();
[0002] (2C) UserputSoftOff();
[0004] (48) if (Local[0] == 1) {
[000B] (AB)   saveVerbs(1,125,2);
[0010] (**) }
[0010] (60) freezeScripts(127);
[0012] (A0) stopObjectCode();
END

And room 80 starts with a cutscene (and only contains a cutscene) ant it does so this way:

[0000] (40) cutscene([]);

Changing it so that it runs cutscene[1]) instead means that saveVerbs() will be called to properly clean up the verbs, I believe. @einstein95: does it still work for you with this new approach? It seems like a better fix. Thanks.

@eriktorbjorn: I see that you've made a commit in a similar area for the Macintosh version: commit 276381d. Does my fix in commit ee1a289 (if you enable it for that platform and remove the drawVerb() workaround) also work on Macintosh?

I wonder if this problem in room 80 also happens with the original FM-TOWNS and Macintosh interpreters. If the visual problem also happens in an original interpreter, it's maybe cleaner to fix the cutscene initialization in that room? But if the original interpreters manage to draw the background over the verbs, then it's maybe better to apply the workaround in drawVerb() as you did.

EDIT: I'm told this scene is OK in the original Macintosh interpreter so commit 276381d is probably the best approach.

@einstein95
Copy link
Contributor

@einstein95 einstein95 commented Jun 3, 2022

does it still work for you with this new approach? It seems like a better fix. Thanks.

Yup, still works

Room 80 only contains a cutscene, where Indy and his father escape
from the zeppelin with the biplane. But it is started with
`cutscene()` instead of `cutscene([1])` which also disables the verb
interface.

The FM-TOWNS version doesn't like this and some verb leftovers were
mixed with the graphics. Adding the missing `[1]` parameter should fix
the issue.

Confirmed to also happen with the original interpreter under UNZ.
@dwatteau dwatteau force-pushed the fix/indy3-fmtowns-buggy-biplane-when-exiting-zeppelin branch from ee1a289 to 2b37d0d Compare Jun 21, 2022
@dwatteau dwatteau marked this pull request as ready for review Jun 21, 2022
@dwatteau
Copy link
Contributor Author

@dwatteau dwatteau commented Jun 21, 2022

I've had some confirmation that the visual problems above also happened with the original interpreter under UNZ, so I think my workaround is the proper approach. For the Macintosh version, I'm told that the original didn't have the problem with the verbs, so commit 276381d indeed looks correct as-is.

Removing the draft status on this PR, then.

@sev-
Copy link
Member

@sev- sev- commented Jul 2, 2022

Thank you!

@sev- sev- merged commit d945157 into scummvm:master Jul 2, 2022
8 checks passed
@dwatteau dwatteau deleted the fix/indy3-fmtowns-buggy-biplane-when-exiting-zeppelin branch Jul 3, 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
3 participants