#480 Chuckie Egg 2 does not load with --detect-loader --no-accelerate-loader

[Alberto Garcia]

The problem was originally found and discussed here:

https://sourceforge.net/p/fuse-emulator/bugs/431/#f102/321b/2a87/880b/7b02/d82e/62d9/ff63/1f78/8c4c/8871

So Chuckie Egg 2 does not load with --detect-loader --no-accelerate-loader. There are more details in the link above, but here's the summary:

• The data block is shorter than what the header says.
• This produces a loading error, but that's what the game expects. The return address after the loading error is what starts the game.
• However with --detect-loader enabled the loading error does not happen as expected: Fuse plays the tape and tries to finish loading the block. This rewinds the tape to the beginning and starts again from there. There are no pauses between the end of the (incomplete) data block and the first block of the tape.

[Alberto Garcia]

I think the problem is that when Fuse receives FLAGS_STOP from libspectrum it stops the tape immediately and ignores the last edge.

This effectively removes the embedded pause at the end of the last data block, and when Fuse's loader detection code plays the tape again (this time from the beginning) it is as if there was no pause between both blocks.

Here's a proposal to fix the problem.

[Fredrick Meunier]

This is interesting, it seems to fix two loading problms and breaks three others! Below were tested with fure tape regression tests again, so no .fuserc and the tape just supplied at the command line.

Fixed:
Gauntlet 2 - Side 1.tzx: expected False, got True
splATTR.tzx: expected False, got True

Broken:
Dan Dare.tzx: expected True, got False
Switch Blade.tzx: expected True, got False
Turbo The Tortoise - Side 1.tzx: expected True, got False

[Alberto Garcia]

Oh, I see, this is endless ... ':-D

It seems that all "broken" games load, it's just that the loader detection or the tape traps don't work. I'll have a look at those too.

[Fredrick Meunier]

True, that's why the regression tests were created :D

[Alberto Garcia]

Where are those tests? Are they published in a repository that I can download and try myself?

[Fredrick Meunier]

Yes they are public in https://sourceforge.net/p/fuse-emulator/fuse-automation/ci/master/tree/ . The tape files are not part of the release and are unfortunately just identified by zxdb ID and sha256sum.

[Alberto Garcia]

I see, thanks !

[Sergio Baldoví]

The tape files are not part of the release and are unfortunately just identified by zxdb ID and sha256sum.

I hope you know that it's possible to import ZXDB and query more info:

select p.id, p.zxdb_id, e.title, p.release
from fuse_automation.programs as p, zxdb.entries as e
where p.zxdb_id = e.id
order by e.title;

[Fredrick Meunier]

I did not know, but am glad that you pointed this out!

[Alberto Garcia]

Ok, this is not directly related to the bug, but while running the tests I thought that it would be much simpler to use SQLite to store the database (it's a very small one after all) rather than have a full instance of MySQL/MariaDB installed and running.

Here's the patch in case it's useful for anyone.

[Alberto Garcia]

Ok, here's version 2 of the patch.

The previous one would keep the embedded pause at the end of a block in two cases;

1) When libspectrum tells fuse to stop (e.g. because we reached the end of the tape)
2) When autoplay and tape_traps is on and the following block is a standard ROM loader.

Keeping the embedded pause in case (1) is what fixes the bug with Chuckie Egg 2 (and with 1942 from what I can see after running the tests).

Keeping the embedded pause in case (2) confuses the loader detection routines and breaks the test cases that you mentioned.

So this new patch updates case (1) but leaves (2) untouched. This is a more conservative patch, it fixes Chuckie Egg 2 and 1942 but does not fix Gauntlet 2 or SplATTR. It also doesn't cause any regression.

About the regressions, however: from what I could see those games still load fine, just differently from what check_loaders.py expects, so arguably we could update the test expectations. But we can leave that for another moment.

One more comment about the patch: as you can see it introduces a new variable called tape_stop_pending. An alternative approach would be to turn tape_playing into an enum with three possible values, something like _STOPPED, _PLAYING, _STOP_PENDING. Also, tape_playing is defined as extern in tape.h but I don't see anyone else using it, so we could make it static. I can update the patch if you think any of this makes sense.

[Alberto Garcia]

An alternative approach would be to turn tape_playing into an enum

Well, I decided to upload this version as well so you can see the difference and judge for yourself.