SCI: LSL7: Ocean outside portholes jumps

[DanielSWolf]

Many rooms in LSL7 feature a porthole through which you can see the ocean slowly moving upwards and downwards. This ought to look rather smooth; compare the beginning of this video: ​https://www.youtube.com/watch?v=onIy436mqEk&t=172s.

However, in ScummVM, the ocean sometimes "flickers" up or down before resuming the smooth animation again. To reproduce, walk into the screen featuring (non-sexy) Victorian Principles and watch the ocean for 10-20 seconds.

I'm running the daily build from Sep 26 2017 on Windows 7 64-bit without any scalers.

[csnover]

I spent some time looking at this today and I am not entirely sure what is going on yet. The kMulDiv and kSinMult/kTimesSin implementations in ScummVM are not accurate to the original interpreter and return slightly different values pretty frequently, but I implemented some fully reversed versions of these functions (SinMult is definitely an interesting edge-case, since the original interpreter does an out-of-bounds read into the value table for atan when degrees = 90) and these implementation differences don’t appear to be the cause of the trouble. So, more investigation is needed.

[csnover]

Oh, I also forgot to mention, the values in and out of kMulDiv and kSinMult don’t seem to be abnormal when these glitches happen (no crazy out-of-range values).

[DanielSWolf]

I took a screen recording to illustrate the problem, attached above as sea.gif.

I also tracked the ocean's vertical motion and attached the resulting graph as sea-tracking.png. In this image, it's easy to see that the motion should consist of various sine waves blending into each other. However, close to the end, a segment of the sine wave seems to be offset in time, causing a backwards jerk at the beginning, then a forward jump.

Also, at the extreme positions, the sea tends to jump back and forth between two positions. I added sea-tracking-closeup.png to illustrate.

Unfortunately, I don't know how to debug SCI scripts, and I couldn't find a tutorial. Maybe someone with more SCI knowledge can help?

[sluicebox]

There are two problems with the ocean motion:

1. Two big jumps around the end of the ocean cycle. This is what the bug report is describing. This is due to our implementation of the modulo opcode (mod) being incorrect for this game. YIKES! Sierra rewrote it in assembly in SCI2.1 Late / SCI3 and changed it to treat the dividend as unsigned.

2. Mild jitter at the top and bottoms of some of the waves. This is due to different results from ScummVM's implementation of the kSinMult and kMulDiv functions versus Sierra's, as csnover identified. I also experimented with an implementation of SSCI-accurate versions, and that fixes the jitter.

I fixed the modulo bug so the big jumps are now fixed: ​https://github.com/scummvm/scummvm/commit/34664be3d35734a9e6f2ccaaf111af9c66a8bcb1

We can do the trigonometry functions later; the big bug is fixed for now.

[sluicebox]

It turns out that the jitter is just caused by kMulDiv discrepancies. That's the easy function to implement as SSCI did, so I've done it: ​https://github.com/scummvm/scummvm/commit/831db854121dcef503e603dd7f1df751bd35abde

The remaining inaccuracies from kSinMult have no negative or detectable effect here; it's just the difference between a few frames appearing 1/20th of a second earlier or later than they would otherwise.

[DanielSWolf]

@sluicebox Thanks for fixing this! I really appreciate the time and effort you put into fixing all these obscure bugs in Sierra games!

[sluicebox]

You're welcome! Thank you for your kind words -- I am highly susceptible to compliments.

This was a hard one, and highly intimidating since the uncanny csnover took an in depth look and didn't figure it out. I have different skills and many limitations so I had to think for a long time about how I could ever approach this. (I refuse to swim in disassembled exe's for more than a few minutes and have forgotten 100% of the trigonometry I've ever learned.) But it's a visually obvious bug, and reported by the honorable LarryScale author, so even though I don't know (care?) about LSL7 myself, I've really wanted to take this one down for years.

But I'm good with SCI scripts. (I suspect I'm the only person to have ever decompiled this game.) So for this, I modded the LSL7 ocean script to log the output of every motion to a text file. Wow that sentence makes it sound easy. It meant identifying a large swath of code I could safely overwrite and then manually writing this entire feature (with full file i/o, string formatting, etc) in byte code (pen and paper and hex editor) within those bytes plus a bunch of SCI3-specific patches to get the modified strings working (relocations!) and then, finally, patching scummvm to allow any of this arbitrary file access. But by the end of the day I had a modded script patch file I could drop in the game directory and run in dosbox and scummvm. I could then diff the log files to see where things really went significantly wrong. That narrowed this down to the exact state before the big problem. Further tweaks to my patch logged additional state and that let me somewhat tediously bisect further until I hit the mod instruction. At that point I had the answer, and while it was difficult, it didn't involve any x86 so I threw a party. At that point, I could quickly confirm my findings against many exe's to get the version check right.

I've never had to touch the core VM before, so this was a lot of fun. "SCI3 mod opcode is broken in 2022" is not what anyone would have expected from this.

[DanielSWolf]

Wow -- that's what I call dedication! Great job!