drhelius / Gearsystem

Sega Master System / Game Gear / SG-1000 emulator for macOS, Windows, Linux, BSD and RetroArch.
https://x.com/drhelius
GNU General Public License v3.0
247 stars 45 forks source link

Define a Flatpak App Manifest for Gearsystem #95

Closed TomChapple closed 2 weeks ago

TomChapple commented 4 months ago

This closes #93 by introducing a Flatpak App manifest, thereby allowing Gearsystem to be packaged and distributed as a Flatpak App. This enables the application to be available for use on a variety of Linux distributions within a sandbox containing all of its dependencies; there is no prerequisite to download any dependencies beforehand.

This Flatpak application is intended to work out-of-the-box, with the greatest amount of feature parity available all whilst maintaining a secure working environment for its users. This includes the ability to load and run the same ROMs as its native release and its ability to use controllers for input.

Although this is very similar to the linux platform, it is kept in its own distinct flatpak platform so that any Flatpak-specific functionality does not impact the existing build.

For testing purposes, this Flatpak app can be built with the following commands in the platforms/flatpak directory:

flatpak run org.flatpak.Builder --force-clean --user --install build-dir io.github.drhelius.Gearsystem.yml
flatpak run io.github.drhelius.Gearsystem

This will create a temporary directory, build-dir, where build files are kept. When run, the --filesystem=home will grant access to the "$HOME" directory. Additional options for --filesystem can be found in the Flatpak Command Reference.

TO-DO

Functional

Documentation

Release

TomChapple commented 4 months ago

My current item of investigation is the File Dialog and how to integrate this with the Flatpak Portal APIs. This project uses source code from another project, Native File Dialog Extended (NFDe) to abstract over the GTK API so that it may work as intended across multiple operating systems.

However, this version of NFDe that is being used is using GTK's FileChooserDialog which does not support the Portal APIs. Later versions of the library appear to support this API, so there may be a good reason to update the dependency on this library rather than using this specific version of it's source code.

Despite this however, the options for the File Dialog from the Portal API are limited to "OpenFile", "SaveFile" and "SaveFiles", each returning a URI to the file contained in /run/user/$UID/doc. In particular, these will only grant access to the specific files or directories selected and nothing more. This has the most impact on the SAV files as these are never explicitly selected. To replicate the current workflow in Flatpak would result in one of: (a) saving the current state to become impossible; or (b) have saving become completely internalised, requiring users to have specific knowledge to find their SAV files.

I think this will likely require a change in UX for loading ROMs and their SAV files, at the very least for the Flatpak package. I'm happy to explore some potential options.

TomChapple commented 4 months ago

I got NFDe v1.1.1 to work with the build and correctly use the Flatpak Portal API mostly building NFDe as a separate module, excluding any source files in nfd/ and linking against libnfd.a and libdbus-1.so. When built this way, I can give Gearsystem no permissions and it will be able to search for and load a ROM that I choose. Later references to it will identify it by its /run URI, but that's okay.

In particular, this involves the addition of the following module in the Flatpak manifest:

  - name: nfde
    buildsystem: cmake
    config-opts:
      - -DNFD_PORTAL=ON
      - -DCMAKE_BUILD_TYPE=Release
      - -DNFD_BUILD_TESTS=OFF
    sources:
      - type: git
        url: https://github.com/btzy/nativefiledialog-extended.git
        tag: v1.1.1
        commit: 5786fabceeaee4d892f3c7a16b243796244cdddc

...and a new Makefile for flatpak containing:

include ../desktop-shared/Makefile.sources

CPPFLAGS += `pkg-config --cflags gtk+-3.0` # Can be removed
LDFLAGS += `pkg-config --libs gtk+-3.0` # Can be removed
LDFLAGS += -lnfd -ldbus-1

include ../desktop-shared/Makefile.common
include ../desktop-shared/Makefile.install

This would make for a very clean alternative for the Flatpak builds, if it weren't for the fact I needed to alter gui.h to point to "nfd.h" instead of "nfd/nfd.h". This discrepancy would make the builds between Flatpak and the other platforms inconsistent. I either need to find a way to install NFDe into a subdirectory for its include files or be happy with altering the existing code for this support (either through preprocessor directives or moving the NFDe files up a directory).

Edit: I found out I don't even need the GTK includes or libraries now as NFD was the only component using them. I've commented the Makefile as appropriate.

TomChapple commented 4 months ago

Turns out that having the include files installed to a nfd/ directory can be done by using CMake's GNUInstallDirs module. By specifying -DCMAKE_INSTALL_INCLUDEDIR=include/nfd, the include headers are stored in /app/include/nfd/ rather than simply /app/include. I'm not sure if it's better practice to have them at the root, but I'd prefer to avoid altering the codebase of this application as much as possible to improve compatibility.

TomChapple commented 4 months ago

Looking at game controllers now, external controllers are not currently detected, but can be if --device=all is specified either on the command-line or in finish-args. Although this is what is referenced in the Flatpak documentation, it would be good to see if there are alternatives to that to prevent such a wide amount of access.

TomChapple commented 4 months ago

The issue flatpak/xdg-desktop-portal#1238 looks quite relevant to my internal desires, but—like they mention—it is probably more appropriate to wait for an Input portal to exist given that controllers could be coming from bluetooth. From that, I reckon that --device=all is probably the best we'll get.

TomChapple commented 4 months ago

The more relevant issue to the above point looks to be flatpak/xdg-desktop-portal#536 for those interested.

drhelius commented 4 months ago

Thanks for your work, this is turning out quite well!

But I want to share some feedback about git submodules: I don't really like them at all 😆

They add a lot of complexity and most of the users of my emulators don't know how to deal with them.

I don't know if this is something common when using Flatpak but is there any way to avoid them? I really don't want to add git modules to the project. As you can see all other dependencies are in the repo one way or another.

drhelius commented 4 months ago

I like it being a different platform directory.

You can add any additional include or source file you need (like those for NFD portal) and it's fine using the preprocessor and makefiles to choose between different platforms.

drhelius commented 4 months ago

And regarding CMake, I'm also trying to avoid it as much as possible, sorry 😆

TomChapple commented 4 months ago

Thanks for the comments!

But I want to share some feedback about git submodules: I don't really like them at all 😆

They add a lot of complexity and most of the users of my emulators don't know how to deal with them.

I don't know if this is something common when using Flatpak but is there any way to avoid them? I really don't want to add git modules to the project. As you can see all other dependencies are in the repo one way or another.

I can understand that, they are a weird construct. I have been using it mostly since that's what Flathub recommends, but it was something I have been meaning to ask. Not using the submodule is fine in my eyes and I can migrate what is existing there into the modules by hand.

You can add any additional include or source file you need (like those for NFD portal) and it's fine using the preprocessor and makefiles to choose between different platforms.

And regarding CMake, I'm also trying to avoid it as much as possible, sorry 😆

That's fair. I have been taking the opportunity to do minimal changes to the codebase where I can avoid it. I did attempt to include the NFDe source files earlier but was having trouble. In retrospect, I believe I forgot to include some of the source files in SOURCES_CXX. Moving those files to this repository would remove the need for CMake, but that really comes down to how those libraries were structured themselves and I think it's more stable to use the build method the developers used for that library. Given that the build pipeline is embedded within flatpak-build or org.flatpak.Builder, I think using multiple build pipelines is less of an issue.

If I introduce that newer NFDe version as source files, I might raise that as a separate pull request so that support on other platforms can be verified independently of this Flatpak build.

Again, thanks for the feedback! I'll make sure to take that into account.

TomChapple commented 4 months ago

@drhelius, I'm not having much luck finding an icon for Gearsystem at the moment. Is there one defined for it that I'm missing?

drhelius commented 4 months ago

There is an icon for Mac, but honestly, it is quite poor.

https://github.com/drhelius/Gearsystem/blob/master/platforms/macos/iconfile.icns

TomChapple commented 4 months ago

Well at the very least I was able to put some transparency on it. It's not amazing, but it's functional at least.

512x512

I'm in the process of seeing how to get the icon visible in the menu and taskbar at the moment, but not much significant progress yet.

TomChapple commented 4 months ago

Took me a while to figure out what was going on for the window icon, but I believe I have something worked out now (in particular bit depth and the RGBA masks). I do have a couple of questions I would like to hear your opinion on before I push the commit for it though (the previous commit being icons kept for the desktop environment):

TomChapple commented 4 months ago

Okay, turns out my icon troubles with the panel/taskbar (whatever the bottom menu bar is called) were due to having Gearsystem installed via snap and it overwriting the existing icon set. Uninstalling that uses the icons I provided in the previous commit. As such, all that's left is the MetaInfo and SDL icons, though the latter could be their own pull request potentially.

TomChapple commented 4 months ago

Okay, functionality-wise, I think it's time to think about the elephant in the room: SAV files.

The Portal API used for Flatpak applications only permits access to a set of files or directories specifically selected by the user; other files cannot be accessed. As such, if a user chooses a ROM file, Gearsystem cannot see an adjacent SAV file. Similarly, Gearsystem does not get visibility where nor does it get permission to write a SAV file related to that file. I think the only way for this to work would require a UX change for Gearsystem (at the very least when run in Flatpak).

I have the following experiences written down as options:

The Explicit experience is close to the existing experience and likely has the least amount of programming overhead to implement. It is a little awkward to choose a SAV each time, however, and it can be unknown if one is truly necessary until the ROM has started. Multi-Select can reduce the number of clicks, but I think it's a bit unintuitive, personally. SAV Memory gets closer to the current experience compared to Explicit, but it requires additional implementation to take advantage of a persisted, Gearsystem-only filesystem to memorise these locations. It is also unclear how users would control this memory.

The Directory option is one that a few popular emulators are already using to good effect. However, this is a large deviation to how Gearsystem currently works and would really only make sense to implement if the non-Flatpak releases also wanted this experience. When implemented however, I believe that experience would be entirely consistent across all platforms.

Do you think there would be other options to explore, or does one of these stick out to you as the way to go? It would be great to hear your opinion on this.

drhelius commented 4 months ago

Could a fixed location for saves and savestates may be used when using Flatpak?

For example something like /home/username/.local/share/gearsystem/

The desktop app has an option to choose between this folder, a custom folder and the folder where the rom is. I'm using SDL_GetPrefPath for it.

TomChapple commented 4 months ago

Possibly. It appears that Flatpak apps tend toward XDG directories such as ~/.var/app/io.github.drhelius.Gearsystem/data/saves based on their documentation on Filesystem access from the sandbox. The quote in that document stating "If some home directory access is absolutely required, using XDG directory access only" is particularly potent to me, but nothing actively stops us from specifying that specific directory inside home.

To be honest: I was never aware of the feature to store saves in another location. I might familiarise myself with it a bit as that fits perfectly architecturally (aside from the same folder) from my perspective.

TomChapple commented 3 months ago

I've just taken a look at the save and save state folders and it looks like, when in a Flatpak application, SDL manages to choose an appropriate default location! For example: ~/.var/app/io.github.drhelius.Gearsystem/data/Geardome/Gearsystem/.

Given that reading these files from the ROM folder is infeasible, would it be a good idea to disable that option as a preprocessor flag?

TomChapple commented 3 months ago

Something I have noticed during the final steps of this build is that—as of right now—I require the source to be a relative directory (i.e. { type: dir, path: ../..} in the YAML file) as there is no other commit that has the platforms/flatpak subdirectory. This likely will not be accepted when sent for submission in its current state, but I imagine that can be updated with an appropriate commit hash after this has been merged to avoid a dependency on a branch.

I have also noticed I will likely need to update the MetaInfo file to include the latest release and its notes.

drhelius commented 3 weeks ago

Just reviewing all open issues... sorry I didn't notice before. Is this ready to merge?

TomChapple commented 3 weeks ago

Now that I look at it again, there are a couple of things I want to tweak on the documentation side (i.e. newline inconsistencies and outdated comments), but otherwise it is in a merge-ready state.

The most obvious outstanding item is the "sources" reference in the manifest file. In particular, this should point to a specific commit for Gearsystem, but the only commits with the platforms/flatpak directory are in this branch. After this is merged, however, this can point to a commit in main's history.

After that, there would be requirements to maintain these files and distribute them as Gearsystem is updated. I personally feel that this case be automated off of GitHub releases, but I want you to know this in advance in case that influences whether you want to merge this or not.

drhelius commented 2 weeks ago

There it goes!! Thanks a lot for the effort, this has been a huge work on your side. I will try to find the time and include this as part of the GH Actions workflow and then extend it to Gearboy and Gearcoleco.