fix autostart not working on reset

[Jamiras]

ensures the disk is detached before trying to reattach it in the autostart code.

also modifies the emu_reset call to allow the vice_reset config to be processed when resetting.

[sonninnos]

Sorry, but no VICE codebase changes without __LIBRETRO__ guard. Unless of course it is a backport that will be in the future version anyway.

Also could you tell the steps required to make this problem happen? I get autostart on reset just fine..

But yes, looks like that emu_reset() is supposed to be -1 indeed, even though 0 is already autostart. But then again that trigger is meant for restarting, not resetting.. As in getting the same outcome as if relaunching content, not for resetting the machine. So if anything it should obey the "Autostart" core option instead, not "Reset Type".

[Jamiras]

The frontend that I'm using calls fopen_s in it's implementation of RETRO_ENVIRONMENT_GET_VFS_INTERFACE, which the documentation indicates locks the file:

The fopen_s and _wfopen_s functions can't open a file for sharing. If you need to share the file, use _fsopen or _wfsopen with the appropriate sharing mode constant—for example, use _SH_DENYNO for read/write sharing.

As a result, when autostart_disk is called on reset, file_system_attach_disk fails unless the previous handle is released first. The exact same logic occurs about 40 lines later the next time file_system_attach_disk is called.

I didn't dig into the details. Perhaps the current implementation (though working in RetroArch) leaks a file handle?

[sonninnos]

Wouldn't be the first time, since this was added for 3.5 https://github.com/libretro/vice-libretro/pull/381 and it got included in 3.6, which is now backported here. (I think I'll be updating to the next version once it arrives later this year).

So in the meantime please wrap it with __LIBRETRO__ so that it won't get lost after rebasing, and looks like emu_reset(0) is already taking the autostart core option into account, so that can be left alone. Since the point is to have the menu Restart be a total relaunching hard restart, and have hotkey/VKBD reset obeying the Reset Type option.

[Jamiras]

please wrap it with __LIBRETRO__

Done.

looks like emu_reset(0) is already taking the autostart core option into account, so that can be left alone

The autostart option is only looked at if emu_reset is called with -1:

https://github.com/libretro/vice-libretro/blob/db8476c7ca5a09e4e6dc9625b4c799a139437fcc/libretro/libretro-core.c#L7217

You can reproduce this in RetroArch. If you set the restart option to anything other than autostart, it still autostarts on reset.

[sonninnos]

Sure, because it is by design to have 2 separate restart hotkeys. If RA restart would obey the core option, one would have to do constant menu juggling if the reset type is cartridge freeze for example, when full restart is also needed. So this way frontend restart will act as content restart without total core restart, and core-side reset will act according to the core option.

If they would be one and the same, the core-side reset hotkey would be useless.

Case 0 in there will check for noautostart which is what we really want.

[Jamiras]

noautostart is initialized from the vice_autostart config. There's a second config vice_reset that can override the autostart behavior when resetting. By passing 0, it's ignoring the vice_reset setting as opt_reset_type is only used in the line I referenced above, and only if emu_reset is called with -1.

https://github.com/libretro/vice-libretro/blob/db8476c7ca5a09e4e6dc9625b4c799a139437fcc/libretro/libretro-core.c#L6681-L6689

It looks like emu_reset(-1) is currently only being called from emu_function(EMU_RESET). Maybe it would be better to call that here?

[Jamiras]

If I'm understanding what you're suggesting, then I've been misunderstand what "Restart Content" does. I was under the impression that it behaved like a soft reset (i.e. pushing the Reset button on a NES/SNES). Functionally, it just calls retro_reset on the core, and the documentation for that say "resets the current game" (not the system).

vice_reset as a System setting suggests the behavior should occur when the system is reset, regardless of how. If it's tied to a restart hotkey, shouldn't it be an Input setting?

[sonninnos]

Yes, EMU_RESET is the core-side hotkey reset, which defaults to "End" key (Core Options > Hotkey Mapping), and that is supposed to follow the Reset Type core option, like the reset button in the VKBD.

Frontend restart is supposed to be "power off/on" thing so that the core does not need to be closed entirely to start from scratch.

Not having those separate would be a step back in QoL.

[sonninnos]

Okeydokey, thanks, merging.