#14699 closed defect (fixed)
AGI: Xmas Card (Tandy Color Computer 3 Demo) crashes on startup
| Reported by: | eriktorbjorn | Owned by: | sluicebox |
|---|---|---|---|
| Priority: | normal | Component: | Engine: AGI |
| Version: | Keywords: | ||
| Cc: | Game: |
Description (last modified by )
I downloaded the "Xmas Card (Tandy Color Coputer 3 Demo) from the ScummVM website, and tried running it in the current development version of ScummVM. It crashes with the following error:
ERROR: Illegal opcode ca in logic 13, ip 3!
But it may be dumb luck that it even gets that far. On the Android version, it crashed outright with no such error message. There are a couple of "uninitialized value" warnings when running it through Valgrind, which I guess could account for the different behavior:
==22093== Conditional jump or move depends on uninitialised value(s) ==22093== at 0x91A72B3: memmove (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==22093== by 0x28CD917: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2443) ==22093== by 0x28C96F7: Agi::cmdCall(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1173) ==22093== by 0x28C9799: Agi::cmdCallF(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1184) ==22093== by 0x28CDA3B: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2452) ==22093== by 0x28B17AC: Agi::AgiEngine::interpretCycle() (cycle.cpp:150) ==22093== by 0x28B21BF: Agi::AgiEngine::playGame() (cycle.cpp:448) ==22093== by 0x28B2542: Agi::AgiEngine::runGame() (cycle.cpp:549) ==22093== by 0x28AC2F3: Agi::AgiEngine::go() (agi.cpp:529) ==22093== by 0x28ACEDA: Agi::AgiBase::run() (agi.h:727) ==22093== by 0x25B22E3: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:326) ==22093== by 0x25B4567: scummvm_main (main.cpp:805) ==22093== ==22093== Conditional jump or move depends on uninitialised value(s) ==22093== at 0x91A71DE: memset (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==22093== by 0x28CD93E: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2444) ==22093== by 0x28C96F7: Agi::cmdCall(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1173) ==22093== by 0x28C9799: Agi::cmdCallF(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1184) ==22093== by 0x28CDA3B: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2452) ==22093== by 0x28B17AC: Agi::AgiEngine::interpretCycle() (cycle.cpp:150) ==22093== by 0x28B21BF: Agi::AgiEngine::playGame() (cycle.cpp:448) ==22093== by 0x28B2542: Agi::AgiEngine::runGame() (cycle.cpp:549) ==22093== by 0x28AC2F3: Agi::AgiEngine::go() (agi.cpp:529) ==22093== by 0x28ACEDA: Agi::AgiBase::run() (agi.h:727) ==22093== by 0x25B22E3: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:326) ==22093== by 0x25B4567: scummvm_main (main.cpp:805) ==22093== ==22093== Conditional jump or move depends on uninitialised value(s) ==22093== at 0x91A71E3: memset (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==22093== by 0x28CD93E: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2444) ==22093== by 0x28C96F7: Agi::cmdCall(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1173) ==22093== by 0x28C9799: Agi::cmdCallF(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1184) ==22093== by 0x28CDA3B: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2452) ==22093== by 0x28B17AC: Agi::AgiEngine::interpretCycle() (cycle.cpp:150) ==22093== by 0x28B21BF: Agi::AgiEngine::playGame() (cycle.cpp:448) ==22093== by 0x28B2542: Agi::AgiEngine::runGame() (cycle.cpp:549) ==22093== by 0x28AC2F3: Agi::AgiEngine::go() (agi.cpp:529) ==22093== by 0x28ACEDA: Agi::AgiBase::run() (agi.h:727) ==22093== by 0x25B22E3: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:326) ==22093== by 0x25B4567: scummvm_main (main.cpp:805) ==22093== ==22093== Use of uninitialised value of size 8 ==22093== at 0x91A71EB: memset (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==22093== by 0x28CD93E: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2444) ==22093== by 0x28C96F7: Agi::cmdCall(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1173) ==22093== by 0x28C9799: Agi::cmdCallF(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1184) ==22093== by 0x28CDA3B: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2452) ==22093== by 0x28B17AC: Agi::AgiEngine::interpretCycle() (cycle.cpp:150) ==22093== by 0x28B21BF: Agi::AgiEngine::playGame() (cycle.cpp:448) ==22093== by 0x28B2542: Agi::AgiEngine::runGame() (cycle.cpp:549) ==22093== by 0x28AC2F3: Agi::AgiEngine::go() (agi.cpp:529) ==22093== by 0x28ACEDA: Agi::AgiBase::run() (agi.h:727) ==22093== by 0x25B22E3: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:326) ==22093== by 0x25B4567: scummvm_main (main.cpp:805) ==22093== ==22093== Conditional jump or move depends on uninitialised value(s) ==22093== at 0x91A71F3: memset (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==22093== by 0x28CD93E: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2444) ==22093== by 0x28C96F7: Agi::cmdCall(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1173) ==22093== by 0x28C9799: Agi::cmdCallF(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1184) ==22093== by 0x28CDA3B: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2452) ==22093== by 0x28B17AC: Agi::AgiEngine::interpretCycle() (cycle.cpp:150) ==22093== by 0x28B21BF: Agi::AgiEngine::playGame() (cycle.cpp:448) ==22093== by 0x28B2542: Agi::AgiEngine::runGame() (cycle.cpp:549) ==22093== by 0x28AC2F3: Agi::AgiEngine::go() (agi.cpp:529) ==22093== by 0x28ACEDA: Agi::AgiBase::run() (agi.h:727) ==22093== by 0x25B22E3: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:326) ==22093== by 0x25B4567: scummvm_main (main.cpp:805) ==22093== ==22093== Conditional jump or move depends on uninitialised value(s) ==22093== at 0x91A720B: memset (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==22093== by 0x28CD93E: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2444) ==22093== by 0x28C96F7: Agi::cmdCall(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1173) ==22093== by 0x28C9799: Agi::cmdCallF(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1184) ==22093== by 0x28CDA3B: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2452) ==22093== by 0x28B17AC: Agi::AgiEngine::interpretCycle() (cycle.cpp:150) ==22093== by 0x28B21BF: Agi::AgiEngine::playGame() (cycle.cpp:448) ==22093== by 0x28B2542: Agi::AgiEngine::runGame() (cycle.cpp:549) ==22093== by 0x28AC2F3: Agi::AgiEngine::go() (agi.cpp:529) ==22093== by 0x28ACEDA: Agi::AgiBase::run() (agi.h:727) ==22093== by 0x25B22E3: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:326) ==22093== by 0x25B4567: scummvm_main (main.cpp:805) ==22093== ==22093== Conditional jump or move depends on uninitialised value(s) ==22093== at 0x91A7254: memset (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==22093== by 0x28CD93E: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2444) ==22093== by 0x28C96F7: Agi::cmdCall(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1173) ==22093== by 0x28C9799: Agi::cmdCallF(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1184) ==22093== by 0x28CDA3B: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2452) ==22093== by 0x28B17AC: Agi::AgiEngine::interpretCycle() (cycle.cpp:150) ==22093== by 0x28B21BF: Agi::AgiEngine::playGame() (cycle.cpp:448) ==22093== by 0x28B2542: Agi::AgiEngine::runGame() (cycle.cpp:549) ==22093== by 0x28AC2F3: Agi::AgiEngine::go() (agi.cpp:529) ==22093== by 0x28ACEDA: Agi::AgiBase::run() (agi.h:727) ==22093== by 0x25B22E3: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:326) ==22093== by 0x25B4567: scummvm_main (main.cpp:805) ==22093== ==22093== Conditional jump or move depends on uninitialised value(s) ==22093== at 0x91A7274: memset (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==22093== by 0x28CD93E: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2444) ==22093== by 0x28C96F7: Agi::cmdCall(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1173) ==22093== by 0x28C9799: Agi::cmdCallF(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1184) ==22093== by 0x28CDA3B: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2452) ==22093== by 0x28B17AC: Agi::AgiEngine::interpretCycle() (cycle.cpp:150) ==22093== by 0x28B21BF: Agi::AgiEngine::playGame() (cycle.cpp:448) ==22093== by 0x28B2542: Agi::AgiEngine::runGame() (cycle.cpp:549) ==22093== by 0x28AC2F3: Agi::AgiEngine::go() (agi.cpp:529) ==22093== by 0x28ACEDA: Agi::AgiBase::run() (agi.h:727) ==22093== by 0x25B22E3: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:326) ==22093== by 0x25B4567: scummvm_main (main.cpp:805) ==22093== ==22093== Conditional jump or move depends on uninitialised value(s) ==22093== at 0x91A7293: memset (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==22093== by 0x28CD93E: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2444) ==22093== by 0x28C96F7: Agi::cmdCall(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1173) ==22093== by 0x28C9799: Agi::cmdCallF(Agi::AgiGame*, Agi::AgiEngine*, unsigned char*) (op_cmd.cpp:1184) ==22093== by 0x28CDA3B: Agi::AgiEngine::runLogic(short) (op_cmd.cpp:2452) ==22093== by 0x28B17AC: Agi::AgiEngine::interpretCycle() (cycle.cpp:150) ==22093== by 0x28B21BF: Agi::AgiEngine::playGame() (cycle.cpp:448) ==22093== by 0x28B2542: Agi::AgiEngine::runGame() (cycle.cpp:549) ==22093== by 0x28AC2F3: Agi::AgiEngine::go() (agi.cpp:529) ==22093== by 0x28ACEDA: Agi::AgiBase::run() (agi.h:727) ==22093== by 0x25B22E3: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:326) ==22093== by 0x25B4567: scummvm_main (main.cpp:805) ==22093==
Change History (12)
comment:1 by , 12 months ago
| Description: | modified (diff) |
|---|
comment:2 by , 11 months ago
comment:3 by , 11 months ago
Well this is awkward. To quote our own wiki:
"There is a demo called Seasons Greetings From Tandy or AGI Demo for Radio Shack.
This demo is not a new demo. It is simply a corrupted version of the Christmas Card 1986 demo."
https://wiki.scummvm.org/index.php/Christmas_Card_1986
If that's true then shouldn't we remove it from the site and the detection table?
comment:4 by , 11 months ago
If that's true then shouldn't we remove it from the site and the detection table?
Is it the corrupted one we're providing, though? It's called "Xmas Card (Tandy Color Computer 3 Demo)" on our demos page, not "Seasonal Greetings".
comment:5 by , 10 months ago
There is only one AGI Christmas demo; it's from 1986. See the detection table and https://wiki.scummvm.org/index.php/Sierra_Game_Versions
I'm just learning AGI so I'm not setup to do a technical analysis yet. But as a reader, and someone who thinks that someone who uploads demos without testing them probably makes more than one mistake, the situation looks clear to me. =)
comment:6 by , 10 months ago
I got this from Tropp on Discord:
"I believe that although some work has been done on sound effects, none of the Tandy CoCo3 AGI games ever worked past identification. Otherwise, for this item, it's the same data if I re-extract it from a disk image of the demo (that appears to work in an emulator)."
comment:7 by , 10 months ago
I see what's going on at the file level. My tools can now decompile this version.
The volume file is in the AGI version 3 format. That breaks our parser (and other tools) because this is very much a version 2 game. I don't understand how that fits with the AGI timeline. It's also possible that the resource-length field just got duplicated for whatever reason, and that happens to look like the V3 format. Either way, the volume is in an unexpected format.
Adjusting for this, I can start the demo. It crashes on sound playback because the Tandy sound format isn't supported and we're missing an error check. Adding the error check fixes the crash and the demo runs, although with no sound.
I will add these once I know a bit more AGI. (You're on your own for implementing Tandy sound!)
comment:8 by , 10 months ago
| Owner: | set to |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
Fixed in: https://github.com/scummvm/scummvm/commit/a5656b43fe9a9d037a4603e7c4b022b0cfe58036
The subsequent sound crash is fixed in: https://github.com/scummvm/scummvm/commit/34a1e5aac4478854f98bab7567e77e76ffc4170e
Demo now runs. No sound though, as you said that format hasn't been implemented.
It runs faster than the PC version; I believe that's controlled in the game scripts. The 1 key brings up the help screen in this version. (It's F1 in the PC version.)
comment:9 by , 10 months ago
Tropp relaying the following: There are exactly two CoCo3 AGI games officially released, LSL1 and KQ3: https://www.mobygames.com/game/company:17/platform:trs-80-coco/sort:moby_score/page:1/
The remainder are unofficial releases based on the work of TRS-80 guru Guillaume Major, who developed some tools and methodology to port them and create disks. He did the lion's share of the work porting Sierra's other AGI games more than a decade ago, and later moved on to porting some AGI fan games as well. I believe this Christmas demo port is his work as well.
In a build with the fixes, the original CoCo3 LSL1 is still stuck with just the cursor loaded, whereas the original KQ3 is now able to advance and seems playable (possibly because of the sound error check). Same holds true with detected freeware fan AGI games GM ported - some now work, some still don't.
comment:10 by , 10 months ago
Thanks! I've been learning the Tandy stuff and came to similar conclusions.
Backing up, the first problem is that our detection tables are loaded with entries for these fan-made ports that relied on custom interpreters, with no mention that they're not legitimate releases. These have never worked in scummvm (or any AGI tool) because they have ahistorical files that were only understood by a custom interpreter. That's critical meta information! =)
The second problem: the LSL1 detection entry is not for Sierra's version, it's for Guillaume's, which of course doesn't work and never has. The archive that's floating around the internet with Coco3 images for these games has an LSl1 directory named "Original", but that's not true; it's Guillaume's. #TrustNo1
Step 1: Document this madness!
Step 2: Decide if these belong in scummvm at all. Why go to the trouble to support fan ports of DOS versions to Coco3, when we already support the DOS versions? I'm really asking, are there interesting things in them? Would anyone ever use this? I would not have added that AGI code if I knew this.
Let the record show that I didn't believe any of this was legitimate from day one. =)
comment:11 by , 10 months ago
Oh!! *Unless...* maybe Sierra's LSL1 introduced this weirdness where they're using V3 volume files with a V2 game with V2 directory files, and that's what Guillaume based his ports on. That would make much more sense to me, although it's weird that KQ3 doesn't have that. It would have gotten them compression for less disks in LSL1. If that's the case, then we have to support this mismatch between directory files and volume files since it's legitimate, at which point the fan ports would get a free ride.
My tools can now parse all of these, and so far I'm seeing that Guillaume put his name in the about boxes of the others, but not LSL1, so that suggests the LSL1 is legitimate. More research to confirm this! This is fun!
The error check and uninit memory are fixed: https://github.com/scummvm/scummvm/commit/0877da67b20fe570cf4812a6ca486a648cd108ab
That's a long time to have a demo on the website that doesn't work; we should verify that the files aren't corrupt. We had a corrupt copy of a PQ2 demo up for a while.