Opened 12 years ago

Closed 12 years ago

#3542 closed defect (fixed)

COMI: Banjo duel seems harder than in the original

Reported by: eriktorbjorn Owned by: fingolfin
Priority: normal Component: Engine: SCUMM
Keywords: script Cc:
Game: Monkey Island 3


Latest SVN snapshot
English version of the game

This is one of those things that pops up every now and then, but I can't find any open bug report about it.

The banjo duel is a bit harder than in the original, in the sense that you get less time to pick the correct note. Sometimes, your time runs out pretty much at the same time as the music ends. (You don't have to wait for it to end before picking the note, so you actually have plenty of time, but that's not obvious to the player.)

I only have a vague idea about how the banjo duel scripts actually work. Script-216 seems to be the main one. When it's your turn to play, it starts room-25-2013 (approximately at the same time as the music starts), which is the one keeps track of when your time runs out. It's simple enough:

Script# 2013
[0000] (65) while (localvar0 <= var688) {
[0010] (6E) localvar0++
[0015] (67) breakHere()
[0016] (**) }
[001B] (79) startScript(0,213,[])
[002B] (7B) stopObjectCode()

In "Mega Monkey" mode, var688 is set to 60 (I think it's 75 in easy mode), and each "tick" in the loop is ~75 ms, so you get about 4.5 seconds from when the music starts.

I did some very unscientific timings with a stop-watch. With ScummVM, I get ~4.6 seconds before my time is up. In the original (running under Wine) I get ~5.7 seconds. That would make a "tick" in the original 90-95 ms.

Ticket imported from: #1861582. Ticket imported from: bugs/3542.

Attachments (3)

comi.s10 (72.6 KB) - added by eriktorbjorn 12 years ago.
Save game at banjo duel ("Mega Monkey" mode)
savegame.001.gz (76.8 KB) - added by eriktorbjorn 12 years ago.
Savegame from original interpreter ("Mega Monkey" mode)
scumm-timers.patch (4.2 KB) - added by fingolfin 12 years ago.

Download all attachments as: .zip

Change History (15)

Changed 12 years ago by eriktorbjorn

Attachment: comi.s10 added

Save game at banjo duel ("Mega Monkey" mode)

Changed 12 years ago by eriktorbjorn

Attachment: savegame.001.gz added

Savegame from original interpreter ("Mega Monkey" mode)

comment:1 Changed 12 years ago by eriktorbjorn

File Added: savegame.001.gz

comment:2 Changed 12 years ago by bluegr

I remember there were some similar bug reports concerning the banjo duel... not sure if they're connected with this somehow, just linking them here:
[ 1296826 ] COMI: No time to click while Banjo duel
[ 1051049 ] COMI: Banjo duel timing problem

comment:3 Changed 12 years ago by fingolfin

Hm, OK. Would be interesting to verify this in other spots, too. Ideally, a check against disasm would be nice.

To adjust this delay, of course one would modify line 1749 in scumm.cpp by changing the 15 to something higher, like 18 / 19 / 20. This value is multiplied by the return value of scummLoop() (which in COMI is apparently usually 5) to compute the length of a tick (which hence is variable).
So, with 15, there are 1000/15 = 66.6 ticks per second. With 20, it would be 100 TPS. Side remark: In the good old days, timers usually would be running at something like 60 Hz, which would correspond to a tick time of 16 or 17.

Also, if it turns out COMI really does the timing a bit differently, then I wonder whether FT or Dig do it differently, too...

comment:4 Changed 12 years ago by cyxx

Looking at disasm, COMI seems to be 60hz based.
Question : where does 15 come from exactly ? shouldn't we use something like diff * 60 / 1000 for each cycle instead ?

I also noticed a difference with scummvm and Dig/FT/COMI (didn't check other) in the handling of VAR_TIMER.
In ScummVM it doesn't seem to be set at all, in the original interpreters, it contains the current tick (not sure if any script reads its value) but then ScummEngine::cyclePalette seems to use it). For reference, COMI updates it that way :
VAR_TIMER = (GetCurrentTime() - _lastCycleTimeStamp) * 60 / 1000

comment:5 Changed 12 years ago by cyxx

I meant "diff * 1000 / 60 for each cycle", <sigh>

comment:6 Changed 12 years ago by fingolfin

Yes, "diff * 1000 / 60" would seem to make more sense. However, it could cause regressions, esp. in Loom CD which is quite sensitive to syncing issues. So we'd have to be careful about it. But in general, sounds like a good change.

Regarding VAR_TIMER: We do set it to 0 at the end of scumm_loop. And that code goes back to the very first versions of ScummVM. It would be interesting to check the MI2 disassembly to see whether that did this, or whether this was a mistake in the original translation ludde made.

comment:7 Changed 12 years ago by eriktorbjorn

If we make any changes that affect the timing in Monkey Island 2, it would be interesting to see how they affect the two hacks (that I can remember) to work around timing issues in that game:

* In ScummEngine_v5::o5_startSound(), we wait for Largo's theme to finish before allowing the Woodtick music to start.

* In IMuseInternal::handle_marker(), we ignore an iMUSE command to work around an animation finishing slightly too quickly. (Bug #1324106)

Changed 12 years ago by fingolfin

Attachment: scumm-timers.patch added

comment:8 Changed 12 years ago by fingolfin

The attached patch implements what I believe to be a more faithful reproduction of the SCUMM main loop. Torbjörn, does it solve your problem? Cyx, does this match the COMI engine disasm?

Note that the "MUSIC_DELAY" code in scummLoop probably should be revised. Maybe we should pass the milliseconds diff to scummLoop as a second param? Does MUSIC_DELAY need more fine tuning?
File Added: scumm-timers.patch

comment:9 Changed 12 years ago by eriktorbjorn

I didn't notice the patch when I wrote my earlier comment.

The Largo/Woodtick workaround is probably still a good idea. If I let the scene run its course with (I believe) the default subtitle speed, the music seems perfectly timed. If I skip subtitles to speed things up, it still triggers the workaround.

The workaround for bug #1324106 is no longer needed with the patch.

comment:10 Changed 12 years ago by cyxx

That patch makes the timing similar to what is done in COMI disasm.

I also (re)checked MI2 disasm, and I can confirm it's 60hz too. Bug report #1035739 gives some details about the timing (basically the ticks/jiffies counter is incremented at frequency of 1193180/5041/4 hz, which is close to 60Hz).

comment:11 Changed 12 years ago by fingolfin

OK, so I just commited this to the trunk.

comment:12 Changed 12 years ago by fingolfin

Owner: set to fingolfin
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.