[Regression] ScummTR 0.4.2 corrupts LOOM-EGA

[dwatteau]

Summary

With LOOM-EGA-EN (either the 1.0 or 1.1 version from 1990), if one just does this to the original game with ScummTR 0.4.2 on Windows:

# This just reimports the original text of the game with no modification
scummtr -g loom -of tmp.txt
scummtr -g loom -if tmp.txt

and then, if you start a new game, to the forest on the left, and click on the first tree hole, then a graphical glitch will appear in the original DOS interpreter, and ScummVM will plainly refuse it:

ScummTR 0.4.0 and 0.4.1 didn't have this problem on Windows.

Games

Loom

Versions

v0.4.2 current Git HEAD

Relevant output (error messages, warnings, or any useful info)

No response

I own a legitimate game

• [X] I promise that my game does NOT come from an illegal source, such as "abandonware" websites

[dwatteau]

Here's something strange… the bug appears to only be triggered by some compiler versions.

• g++ 2.95.3 on Haiku: ✅
• g++ 4.3.2 on Debian 5: ✅
• g++ 4.4.7 on Debian 7: ✅
• g++ 4.4.7 on Debian 11 (from the Dockerfile.linux86 CentOS 6 builder): ✅
• g++ 4.6.3 on Debian 7: ❌
• g++ 4.7.1 on Debian 7: ❌
• g++ 4.8.7 on CentOS 7: ❌
• g++ 6.3.0 on Windows: ❌
• g++ 9.3.0 on Debian 11: ❌
• g++ 10.2.1 on Debian 11: ❌
• clang++ 11.0 on Debian 11 (libstdc++): ❌
• g++ 11.2.0 on maOS 10.15: ❌
• clang++ 12 on macOS 10.15 (libc++): ✅

Compiling with or without optimisations made no difference (!)

So it's not necessarily related to v0.4.1 vs. v0.4.2, because I was comparing them on Windows, but v0.4.1 was compiled with Visual C++ Express 2005, while v0.4.2 was compiled with g++ 6.3.0.

Needs more investigation…

(FWIW, PR #42 doesn't fix the problem.)

[dwatteau]

So it looks like that the best test environment for this is an Ubuntu 12.04.5 VM:

• g++ 4.4.7 on Ubuntu 12.04: ✅
• g++ 4.5.3 on Ubuntu 12.04: ❌
• g++ 4.6.3 on Ubuntu 12.04: ❌

So the answer is maybe somewhere in https://gcc.gnu.org/gcc-4.5/changes.html. It could also be a libstdc++ change, since clang++ 11.0 on Debian 11 with libstdc++ also has the same bug.

Doing a hybrid -O0 build between g++ 4.4 and g++ 4.5 reveals that the problem is somewhere in src/ScummRp/block.cpp.

[dwatteau]

After more trial-and-error splitting block.cpp itself between g++ 4.4 and g++ 4.5, it appears that the problem is located in void OldRoom::_getOIInfo().

The values to oiOffset[0] and oiOffset[11] sometimes get permuted when compiling with g++ > 4.4:

oiOffset[0]: 23521
oiOffset[11]: 0

[dwatteau]

This was due to an unstable usage of std::sort().

It was changed to std::stable_sort() in commit a3955043322e02d2a531c217cbd137311443d8bd, and the strict weak ordering of OIInfo was also fixed in commit 8322e6da739a82dd4511474f152679ee303a0bd7.

EDIT: The second 8322e6da739a82dd4511474f152679ee303a0bd7 commit actually broke Maniac/Zak in 0.5.0, so it was reverted. Only commit a3955043322e02d2a531c217cbd137311443d8bd (i.e. using std::stable_sort()) is kept at the moment.

[dwatteau]

Can't reproduce it anymore with the last two fixes, and it appears that only scummtr-0.4.2-win32.zip had this bug in the official pre-built releases (the other ones were OK because I was using old compilers on purpose after such a long hiatus).

I've added a scummtr-0.4.2-win32-MSVC.zip variant, which was built with Visual C++ 2005 Express instead of MinGW-w64, thus avoiding the bug in this earlier release, too.