Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define a Flatpak App Manifest for Gearsystem #95

Merged
merged 19 commits into from
Jun 16, 2024

Conversation

TomChapple
Copy link
Contributor

@TomChapple TomChapple commented Feb 17, 2024

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

  • Ensure Gearsystem can start and can run ROMs.
  • Allow for Gearsystem to find and use ROMs through a user dialog without needing to specify a wide range of filesystem access.
    • Allow for the SAV files to be read without needing explicit access from the user.
  • Allow for external controllers to be used as input into Gearsystem

Documentation

Release

  • Decide where the source of truth for the manifest shall be (given that submission to FlatHub is via a pull request to their Git repository).
  • Decide whether the Flatpak application should be part of the CI/CD process and how that is accomplished.

This manifest allows for the Gearsystem application to start and execute
ROMs without hanging or being unable to render windows.

The `--share=ipc`, `--socket=x11` and `--device=dri` arguments along
with the GLEW and GLU shared modules are necessary for the GUI to appear
whereas the `--socket=pulseaudio` and SDL2 shared module are necessary
for audio to be processed without hitting an indefinite lock.

Filesystem access is still in development, meaning that the
`--filesystem=home` or some equivalent command is required to find ROMs
to load into Gearsystem.
@TomChapple
Copy link
Contributor Author

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
Copy link
Contributor Author

TomChapple commented Feb 18, 2024

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
Copy link
Contributor Author

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.

Given that there are some platform and/or code changes that are only
present on this branch, it makes sense to refer to the current
directory. I believe it's possible to refer a specific commit eventually
though.
Later versions of Native File Dialog Extended (NFDe) have support for
the Flatpak Portal APIs, allowing for native file chooser dialogs that
grant permission to the chosen file without requiring a wide-range of
permissions. This does not allow for SAV file discovery or saving,
however. This will be addressed later.

This is implemented as a separate module—compiled to a static library
rather than as additional source files—to minimise additional
dependencies that would otherwise be handled in its CMake configuration.
As a result, the source files under `nfd/` are not required for the
Flatpak platform and are excluded. Given the Portal API does not use
GTK3 either, the dependencies on this library have also be removed.

A CMake option of `-DCMAKE_INSTALL_INCLUDEDIR=include/nfd` is required
due to the application expecting headers for NFDe to be in a
subdirectory of `nfd/`. This is to mimic the existing include structure,
though it may be argued that keeping them at the root may be better
practice.

This commit necessitates a specific Makefile for the Flatpak platform
given it requires different libraries to link.
@TomChapple
Copy link
Contributor Author

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
Copy link
Contributor Author

TomChapple commented Feb 18, 2024

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
Copy link
Contributor Author

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

This utilises the `--device=all` option for Flatpak. It's not ideal
since it grants permission to *all devices*, but there is no better
option available just yet. It is kept separate as a means for a simple
update in the future.
@drhelius
Copy link
Owner

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
Copy link
Owner

drhelius commented Feb 18, 2024

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
Copy link
Owner

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

@TomChapple
Copy link
Contributor Author

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.

This is to avoid needing to rely on a Git submodule to build this
project. Instead, the equivalent files from these modules have been
placed in this repository.
This allows the Flatpak application to be added to a desktop environment
such as via a menu item. This is automatically added and removed as the
Flatpak application is installed and uninstalled respectively.

The method of installation was based off of the `.desktop` entry defined
for [Visual Studio Code's Flatpak app][1].

There are currently no icons associated with this application, so it
will appear blank for now.

[1]: https://github.com/flathub/com.visualstudio.code/blob/6ace278a5768a266188e18bc8864ff643cfe22e3/com.visualstudio.code.yaml#L57
@TomChapple
Copy link
Contributor Author

@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
Copy link
Owner

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

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

Now that these source files are in the project, the use of the module
sourced from the project's Git repository is no longer needed.
Appropriate libraries are included and linked as appropriate (i.e.
`dbus-1`).
@TomChapple
Copy link
Contributor Author

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.

This adds a series of PNG files to represent the Gearsystem application.
These adhere to [the Freedesktop Icon specification][1] and come in a
variety of sizes. The icons are based on the existing icon used for the
MacOS release.

Notably, this icon is not used in the panel/taskbar/menubar, but this is
to be investigated.

[1]: https://specifications.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html
@TomChapple
Copy link
Contributor Author

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):

  • I have been testing this icon on a Linux machine, but this icon could theoretically work on every platform since it uses the SDL API. I would need to check if endianness differs between the platforms, but would you want this icon to be across all platforms?
  • This currently involves embedding the icon as a string in a source file, but I'm curious of what a better representation of this would be in the build pipeline. Should this instead be kept in a file that is dynamically rendered as a source file in the Makefile? Or is it easier to store it statically in a source file (e.g. icon.c)?
  • The string currently stores the icon data as raw bytes without compression, totally about 235KiB. I can load the data in as an image format using SDL_image, but this is a separate library and would introduce a further dependency. Is it better off just keeping the icon as raw bytes for the time being?
  • I notice a few copyrights on the source files, but I'm not sure how these would work for any icon files (especially if they are statically kept in a C source file). Is there a general policy I should follow?

@TomChapple
Copy link
Contributor Author

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.

This file describes the metadata of the project. This can be used on a
variety of repositories, but is currently just used for the Flatpak
application for the time being.

I currently have this MetaInfo file distributed under the "CC0 1.0
Universal" license, essentially putting it in the public domain. This
only covers the metadata file itself and does not impact the license of
the project.

I would also like to mention in this commit that the prior commit does
not have issues with icons. It was instead a local configuration issue.
@TomChapple
Copy link
Contributor Author

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:

  • Explicit: After selecting a ROM file, users will choose whether to load a SAV file or create a new one. In either case, a prompt is opened to choose where this file will be located. There is also an option to forgo creating a SAV altogether.
  • Multi-Select: When choosing a ROM file, users may also multi-select a SAV file at the same time to load in that in with the ROM file. Only one ROM and SAV file may be chosen and an error is reported if this is not the case. If no SAV file is selected, the Explicit method is followed.
  • SAV Memory: A similar experience to Explicit, but Gearsystem persists previously loaded SAV files. If no SAV file has been loaded for that ROM (probably identified by name and/or hash), then the user is prompted on whether to create a new SAV, select one from the filesystem or continue SAV-less. The selection process is identical to Explicit. If a SAV was selected, this SAV location is kept in a location that Gearsystem has access to for runtime data. This is used when the ROM is loaded later to skip this SAV selection process altogether, as the abstracted location has already been persisted.
  • Directory: Rather than choosing a ROM file, Gearsystem instead tracks specific directories where ROM files may be kept. A new UI now lists ROMs that can be loaded from these directories, with SAV files being abstracted from the user altogether.

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
Copy link
Owner

drhelius commented Mar 3, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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?

This removes the option to store save and savestate files next to the
ROM being loaded. This is because the Flatpak containerisation cannot
support this natively, so indicating that it could would be a bad user
experience.
This guide will be very useful for other developers to define their
development environment when supporting the Flatpak application.

I have attempted to include all points that have been useful for me
during the setup of this Flatpak application.
@TomChapple
Copy link
Contributor Author

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.

This is a copy of the releases notes on the GitHub release.
@TomChapple TomChapple marked this pull request as ready for review April 5, 2024 07:44
I missed a forward slash to end a paragraph, unfortunately.
These are to pass the linting process for the AppStream MetaInfo file.
For the content rating, I used [an OARS tool][OARS] to generate the
appropriate tag. In particular, I saw this as an application without
internet connectivity and without any advertising or in-app purchases.
This appears to have generated the most basic content rating.

[OARS]: <https://hughsie.github.io/oars/generate.html>
@drhelius
Copy link
Owner

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

@TomChapple
Copy link
Contributor Author

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.

These comments are now more reflective of the current state of the
manifest, describing why there is a relative source and an ideal target
state.
@drhelius drhelius merged commit dc74d48 into drhelius:master Jun 16, 2024
9 checks passed
@drhelius
Copy link
Owner

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.

@TomChapple TomChapple deleted the flatpak-app branch June 20, 2024 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Create Flatpak App for Gearsystem
2 participants