#336 A lot of RZX files don't play properly

[BogDan Vatra]

windale reported this problem on Spectacol's bug tracker: https://github.com/bog-dan-ro/spectacol/issues/48

When I tried to play http://www.rzxarchive.co.uk/s/switchblade.rzx with fuse, it prints an error, then it loops the first part of the movie.

[Fredrick Meunier]

A RZX file without an initial snapshot and read inputs to go to the current auto-load position? It would be slightly slower because of ROM initialisation. If perhiperals are not disabled the playback could fail.

Yes, it would be on top of the settings being set by snapshots and happen after a hard reset.

It would be slower because of ROM initialisation, but hardware that expects some intialisation to run on boot would get to do it's initialisation. This approach wouldn't be compromise-free though, it wouldn't be compatible with custom ROMs that have more radical changes though (like single-key entry). I'm not planning on writing this code though :)

I'm fine with your new patch (though I'd suggest an enum to give the enable/disable peripherals flags meaningful names).

[Sergio Baldoví]

Yes, it would be on top of the settings being set by snapshots and happen after a hard reset.
It would be slower because of ROM initialisation, but hardware that expects some intialisation to run on boot would get to do it's initialisation. This approach wouldn't be compromise-free though, it wouldn't be compatible with custom ROMs that have more radical changes though (like single-key entry). I'm not planning on writing this code though :)

I agree that this method would initialize some peripherals, but RZX files are very contrained to time/frames. Any peripheral (or combination of peripherals) that page memory after a start would need a specific RZX file. A similar method would be to queue keypresses and keyrealeses (LOAD "" for 48k, Enter for 128K) that shall be consumed when the spectrum rom is paged, therefore not invalidated by peripheral initialisation. The machine could run at full throttle while autostarting. But that's a can I'm not willing to open.

I'm fine with your new patch (though I'd suggest an enum to give the enable/disable peripherals flags meaningful names).

Thanks for the suggestion! will do!

[Fredrick Meunier]

Autoload snapshots are gone as of 1.5.0 - any reaons not to add this patch now?

[Sergio Baldoví]

Autoload snapshots are gone as of 1.5.0 - any reaons not to add this patch now?

I think the patch can be simplified because of phantom typist. After loading a snapshot, inactive peripherals should always be disabled. Patch attached (untested).

[Fredrick Meunier]

I don't mind disabling optional peripherals while loading a snapshot, but I still don't see why the snapshot enabled functions for the peripherals shouldn't replace the current configuration of the peripheral if it isn't enabled in the snapshot.

I'm inclined to apply both the v2 and your v4 patch for the next release - any concerns?

[Sergio Baldoví]

I don't mind disabling optional peripherals while loading a snapshot, but I still don't see why the snapshot enabled functions for the peripherals shouldn't replace the current configuration of the peripheral if it isn't enabled in the snapshot.

Patch v4 already does it in a generic way. This code:

*(private->periph->option) = 0;

is equivalent to the v2 code:

if( !libspectrum_snap_melodik_active( snap ) )
  settings_current.melodik = 0;

There are some peripherals without snapshot_enabled handler, most notably, speccyboot and if2.

ay, joystick, scld and ula are enabled/disabled by other peripherals/machines.

I'm inclined to apply both the v2 and your v4 patch for the next release - any concerns?

Looks redundant to me but v2 code could improve readability.

[Fredrick Meunier]

I agree it's redundant in terms of the fix, just focussed on the readability and not being misleading with the peripheral snapshot enabling code.