Opened 3 years ago

Closed 3 years ago

#13354 closed defect (fixed)

MI1: Guybrush can hold his breath for longer than 10 minutes.

Reported by: ffyte Owned by: AndywinXp
Priority: normal Component: Engine: SCUMM
Version: Keywords:
Cc: Game: Monkey Island 1

Description

When Fester throws you off the dock, you should be able to hold your breath for only 10 minutes.

Testing with the floppy VGA version I get the following:
SCUMMVM 2.5.1: 10min 30s
FREEDOS: 10min 13s
DOSBOX-Staging 10min 13s
SCUMMVM version that is used by speedrunners of the game: 10min 13s

Others have tested Monkey Island 2 with similar results.

Steps to reproduce:
Play the game up to the trial with the idol of many hands, once Fester throws you into the bay, wait until time has passed.
or:
scummvm.exe -d0 -b 332 monkey-vga

Timing was done from when the screen with guybrush appears to when he drowns.

Change History (11)

comment:1 by eriktorbjorn, 3 years ago

I tried one approach at making the timing possibly more precise. That brought the time down from about 10:34 to 10:04. So that's... better? I haven't committed it, but my changes (only one so far) can be found here:

https://github.com/eriktorbjorn/scummvm/commits/scumm-timing

comment:2 by eriktorbjorn, 3 years ago

If I understand the script that handles drowning (room-42-205.dmp.txt), it first waits 480 seconds (8 minutes) before printing "Gee, I don't know how much longer I can hold my breath."

It then waits 60 seconds before changing the actor palette.
It then waits another 30 seconds before changing the actor palette again.
It then waits another 15 seconds before changing the actor palette again.
It then waits another 15 seconds before starting the final animation, where guybrush changes color several times before rising up.

(The conversation you overhear on the dock is handled by a different script, so that doesn't interfere with the main sequence of events.)

So by that standard, it should take exactly 10 minutes before the final animation begins. That's what I see with the aforementioned change, so I guess it's at least theoretically correct.

I haven't compared it to DOSBox, though.

comment:3 by ffyte, 3 years ago

We use modifications which are from this fork (and branch):
https://github.com/scummvm/scummvm/compare/master...UrQuan1:scumm-timing

The original author of these changes hasn't been around for a few years, but they seem to work fine for scumm games (and make them run faster and closer to native in general).

comment:4 by eriktorbjorn, 3 years ago

Owner: set to eriktorbjorn
Resolution: fixed
Status: newclosed

I've committed the changes from my pull request, which seem to be enough to fix this bug. See https://www.youtube.com/watch?v=9kOSkhxUCEs for a comparison between DOSBox and ScummVM.

Could you please verify, once there is a new daily build?

I can't judge the other changes from UrQuan1's repository. Fortunately someone else, more qualified than me, promised to take a look.

comment:5 by ffyte, 3 years ago

Did some testing on the nightly version, and while it's more accurate on the timing, it seems now to be faster than native.

See: https://youtu.be/y7AumdH3HWc
Also tested the end with a windows stopwatch and ms-dos from windows xp on a laptop I have handy.
https://www.youtube.com/watch?v=7YXxGV7HgTo

Not sure if there's any RNG involved or if the native interpreters wait timers were slightly different. Obviously the transition effect in scummvm and dos are different (fade from dock to underwater).

comment:6 by eriktorbjorn, 3 years ago

Resolution: fixed
Status: closednew

Interesting. It seems the VGA floppy version (SCUMM version 4) is slower than the VGA CD version (SCUMM version 5), which is the one I used while testing.

At a quick glance, I suspect it may have something to do with https://github.com/scummvm/scummvm/commit/61e6a7225297b798a7d5d34f5b66f157d8ce4d70 since that apparently changes the timer frequency depending on SCUMM version.

In which case making ScummVM obey its timer was just a good first step. So I'm reopening the bug report.

comment:7 by AndywinXp, 3 years ago

Hiya! An update on my end: I've been researching SCUMM (v0-8) timers from scratch during the last two weeks, and I'm working towards making a PR which expands on the work done by eriktorbjorn.

Whenever I mention a divisor in the following notes, I'm talking about the divisor for this operation:
1193182Hz / divisor,
which yields the target frequency of the Programmable Interrupt Timer (PIT) in Hz.

Right now I have taken these notes from my own disasms:

  • MANIAC v1 sets the PIT only when playing sound, and when it's not doing that the divisor is defaulted to 0, which means 65536; this would yield a game frequency of about 18.21Hz, which is very slow, but the reality is that SCUMM works with quarter frames and the game processes three game frames for each engine frame, so we have to multiply 18.21 by 4 * 3, finally obtaining about 218.48Hz which matches the original speed (this was compared on DosBox);
  • v2-4 games all appear to run on PIT in which the divisor is 0x13B1 (5041), yielding about 236.6955Hz; this is true also for the shake effects, but be aware that the game timer and the shake timer are different entities and are not interchangeable (unlike UrQuan did on his implementation); I was able to match DosBox time very accurately when drowning Guybrush in MI1 EGA and VGA.
  • Starting from v5, the interpreters set up some kind of timer orchestrator which handles several "sub-timers"; the base timer seems to be the iMUSE timer (divisor: 0x1000 or 4096 in decimal, yielding about 291.3042 Hz) which is used as the shake timer as well. The game timer was a bit tricky to obtain, here's the algorithm for reference:

1) Launch the timer orchestrator at a rate of 1193182.0 / 4096 == 291.3042Hz, whose interrupt will call a function which we'll call advanceGameTimers() for convenience.
2) At each run of advanceGameTimers() a global counter will be increased by 3433;
3) If the global counter is lower than 4166 (or in some games 4167... I have to recheck my disasms) goto 4a, else goto 4b;

4a) Exit the interrupt function.
4b) We've reached the moment in which we have to mark the quarter frame tick: decrement the global counter by 4166 (or, again, 4167) and increment the SCUMM quarter frame tick count.


  • Given what happens above, I found the frequency to be calculated like this: (1193182.0 / 4096) * (3433 / 4166) == 239.992Hz; testing the drowning scene in MI1 CD yields accurate-to-frame timings with respect to DosBox. Remember, the shake time is regulated only by the 4096 iMUSE divisor set in the IMS driver;
  • v6 games appear to do the same thing, only with 4167 in place of 4166 (at least this is what the Sam&Max disasm tells me). I still have to do some heavy testing to see if this is accurate. I recall, for some reason, DOTT and S&M to be slower than the other games (about 236Hz) but I have no evidence for this right now, but I do have evidence for the fact that the shake timer is still regulated by the 4096 divisor;
  • v7 games do something similar, again, but set up a base timer with a divisor equal to 3977, and the subtimer has a count of 4971, which means: 1193182.0 / 3977 * 3977 / 4971 = 240.029Hz. I haven't checked the shake timer for this version yet but I suppose they are using the 1193182.0 / 3977 == 300Hz base frequency. Also, both DIG and FT do something strange in the SCUMM main loop when fetching the whole frame timer counter... anybody know what the heck is this? It seems to decrement something out of the quarter ticks counter every time it wraps around the negative numbers...
VARS(VAR_TIMER) = (qTicks - (__CFSHL__(qTicks >> 32, 2) + 4 * (qTicks >> 32))) >> 2;
  • v8 appears to run perfectly as-is :-) COMI is a Windows game after all, and it just uses the Win95 equivalent of the timer routines we already use (targeting a base rate of 60Hz, or 240Hz when thinking quarter frames).
Version 3, edited 3 years ago by AndywinXp (previous) (next) (diff)

comment:8 by AndywinXp, 3 years ago

A little update for v6 games:

  • DOTT behaves like this: (1193182.0 / 4096) * (3433 / 4237) == 236.027Hz;
  • S&M behaves like this: (1193182.0 / 4096) * (3433 / 4167) == 239.992Hz;

...jeez, I'd better check every single game one by one to see if there are any other pranks like this

Last edited 3 years ago by AndywinXp (previous) (diff)

comment:9 by jepael, 3 years ago

Hi, just a few things that might be good to note.

Even the same DOS game executable can use a different PIT divisor depending on which sound driver is selected. IIRC it will differ whether you use Monkey Island with Adlib, Speaker, GameBlaster or none. So please make sure you know which sound device you used to find your divisors. I may be able to figure out a couple of cases. It will also be trivial to read back PIT divisor value written to it, or just debug it with DOSBOX to see which value gets written to it.

And since you have used so many decimals in the calculations, if it matters, the PIT clock rate is not really 1193182.0 Hz. If you are anyway using 0.1 Hz units, it is closer to 1193181.8 Hz. The theoretical value is a fraction that is exactly 105 / 88 in MHz.

comment:10 by AndywinXp, 3 years ago

Hi jepael, thanks for your comment! You are certainly right, it changes based on the device: I made sure to always use Adlib and/or MT-32 (both used the same divisor everywhere I checked, but please, let me know if that's not true) as the reference timer.

But, again, I'd really be happy if you'd like to help me double-check the timings, at least for the early non-iMUSE games.

You're also right that the rate is 1193181.818181... Hz, the reason why I kept it rounded is because by using the full precision number the difference with the truncated frequency rate in the calculations is in the order of 10-5 (so very, very small: our ScummVM timer can't pick up that kind of granularity).

Thanks again, and please feel free to signal me any other discrepancy, to double-check any of the numbers, or to discuss the code here: https://github.com/scummvm/scummvm/pull/3853

comment:11 by bluegr, 3 years ago

Owner: changed from eriktorbjorn to AndywinXp
Resolution: fixed
Status: newclosed

The PR has been merged, so this can be closed now. Thanks AndywinXp for your work on this!

Note: See TracTickets for help on using tickets.