melonDS icon indicating copy to clipboard operation
melonDS copied to clipboard

Make archive detection more robust and add it to the CLI

Open Janfel opened this issue 3 years ago • 8 comments

This PR

  • consolidates the list of supported file extensions into three constants (NdsRomExtensions, GbaRomExtensions and ArchiveExtensions),
  • enables archive detection for the command line parameters, fixing #1393,
  • filters archive members by file extension, just like the default file picker,
  • makes the handling of command line paths more robust by adding safety checks to MainWindow::preloadROMs,
  • thereby fixing some crashes and hangs when calling melonDS with invalid parameters, and
  • adds .tar.zst, .tar.lz4 and short forms (.tgz, .txz, .tzst, ...) to the list of supported archive file extensions, fixing #1063.

If I have missed something or if MainWindow::preloadROMs isn’t the right place to do what I’m trying to do, please let me know.

Janfel avatar Mar 12 '22 18:03 Janfel

I've been (kinda) maintaining my own fork with the following changes:

  1. Enable fullscreen when loading from CLI
  2. Enable archive support from CLI
  3. Custom save directory for saves and savestates (#946) + Use save dir for archives

The third feature has been implemented in https://github.com/Arisotura/melonDS/pull/1333, this PR implements the second. The only thing left to do is making the emulator fullscreen when started from CLI. I don't mean to feature creep this PR and I'm thankful that somebody finally stepped up to write a proper solution but the first bullet point is literally the only thing left before I can finally switch back to upstream and delete my (stupid) fork. You can check out how I did it but my code is terrible. You really don't have to overengineer this - just go into fullscreen whenever a ROM is loaded over CLI (I don't see a reason to make this a flag because CLI usually means the emulator has been started from a frontend). Hopefully you can fit this into this PR because multiple people have been messaging me about keeping my fork up-to-date and I can finally see the horizon.

Anuskuss avatar Mar 13 '22 01:03 Anuskuss

When launched through a file association on Linux, the ROM path is passed to the executable as an argument, and in a case like that having the emulator suddenly open in full screen would be rather surprising and undesirable.

Open as fullscreen (among several other things) should be a command line option.

nadiaholmquist avatar Mar 13 '22 09:03 nadiaholmquist

Open as fullscreen (among several other things) should be a command line option.

Yeah, I know - obviously that would be the preferred solution. PRs are welcome. But if you aren't going to write that it's better to reduce this request to a minimum otherwise there's a chance that progress will stall and then nobody wins. If Janfel doesn't want to implement that, I'll just merge this PR into my own fork and apply my changes on top. Users wanting this functionality will hopefully find my fork and figure out how to download/compile it.

Anuskuss avatar Mar 13 '22 22:03 Anuskuss

I might look into improving the CLI, but that is a separate issue and out of scope for this PR.

Janfel avatar Mar 13 '22 23:03 Janfel

No worries, I'll just snatch your changes for the time being. I don't need an overcomplicated solution so I'm fine with using Anuskuss/melonDS.

Anuskuss avatar Mar 14 '22 04:03 Anuskuss

melonDS 0.9.5 has some basic functionality for this now. It's not perfect because it doesn't detect if it's a ROM or an archive and it doesn't autoload single ROM archives (you have to use it in conjunction with -a/--archive-file) but it's certainly good enough. Maybe this could be rebased though and finally merged to fill in the gaps?

Anuskuss avatar Nov 03 '22 23:11 Anuskuss

It does autoload archives iirc, in the same way that File>Open does

patataofcourse avatar Nov 04 '22 00:11 patataofcourse

I have rebased the changes on the current master (b069a2acf10e58579d82500fb057f275647507c0). Because didn’t know how to correctly rebase a pull request, I have opened a new one as #1560.

Janfel avatar Nov 10 '22 01:11 Janfel