Rebrand to PrimeHack (Linux)

[xiota]

Renames binary, configuration, and data storage locations. User responsibility to reconfigure or port data themselves. Supersedes #118.

This patch has been in use at aur/dolphin-emu-primehack-git since Dec 2023.

Rebranding for other OSes ideally done in separate PRs because I would not be able to test the changes.

Sharing data by default with dolphin-emu has already been discussed extensively elsewhere. It is user responsibility to configure such data sharing themselves. Settings to do so are already available. Symlinks may also be used. Further discussion on the topic should take place elsewhere.

[XargonWan]

Thanks from the RetroDECK Team, I hope this can be accepted and merged :)

[SirMangler]

Nitpik but, I realise our github still advertises us similarly (I had not realised this hasn't been changed in like 5 years lol) but it would be more apt to describe primehack more like A GameCube and Wii emulator fork bringing modern FPS features to the Metroid Prime series. The former description is really outdated compared to how much PrimeHack does now.

[xiota]

I'd prefer to avoid naming specific games, even though it's partly already in the project name.

[SirMangler]

This looks good. I don't think the Sys folder should be shared among versions, this folder can change between Dolphin releases, so it shouldn't be considered stable or safe for sharing, and it's not worth splicing it up, like you said, the user is more than able to do this themselves should they want to.

My main concern is the same as the last PR, many people will be effected by this and will have no idea why "my saves just got deleted after updating!". Because of this, I still consider this change to be harming more people than it aids. Especially when you factor in things like a significant number of people come to PrimeHack after trying out Vanilla Dolphin.

Putting aside a migration tool, I would at least like to see a popup that checks known Dolphin install folders and can report to the user that the primehack config folder has moved. Bonus points for if you check to see if the InitialPrimeHackRun setting in the Dolphin.ini is present before presenting this pop-up box.

The code for PrimeHack's initial run popup box is very simple and starts here:

dolphin/Source/Core/DolphinQt/Main.cpp

Line 257 in 03117e2

if (!Config::Get(Config::PRIMEHACK_INITIAL_RUN))

[SuperSamus]

Sharing data by default with dolphin-emu has already been discussed extensively elsewhere

I still want to argue that there isn't any risk in sharing the data folder (and only the configuration folder should be separated).
I expanded the comment on #118 (comment).

EDIT2: I admit my hypocrisy.

[xiota]

@SuperSamus The PR description says discussion on the topic should take place elsewhere. You previously wrote, "should be the decision of SirMangler", yet when SirManger expressed his opinion, you continue arguing when it is not wanted.

[xiota]

@SirMangler I'll look at the popup to add a notification. However, this patch has been in use for 4-5 months with no evidence of user confusion. 30-day metrics show ~18 recent downloads, so there are people using the patched version.

Since there is no official Linux binary distribution, these are people who are capable of setting up a dev environment and building the software themselves. The likelihood of "confusion" over something like this is small.

[SirMangler]

@SirMangler I'll look at the popup to add a notification. However, this patch has been in use for 4-5 months with no evidence of user confusion. 30-day metrics show ~18 recent downloads, so there are people using the patched version.

Since there is no official Linux binary distribution, these are people who are capable of setting up a dev environment and building the software themselves. The likelihood of "confusion" over something like this is small.

The 'official' way to install PrimeHack on the SteamDeck (and in general) is Discover/Flatpak. Taking a grain of salt, Valve estimated in 2023 that 39.33% of the "linux gaming community" is made up of Steamdeck users based on what devices are running Steam. For comparison, Arch is 8.33% and Ubuntu is 7.87%.

I can tell you that there are over 2.25 million downloads from the Flatpak. The '30 day metrics' show ~2,800 recent downloads.

It's unclear if they would join the Discord or GitHub to complain they have "lost" their saves or if they would just give up. I do believe Arch users tend to be more tech-competent and might not jump to the same conclusion so quickly, and I can see you have a pinned comment on the AUR explaining the settings folder is different. However, obviously, this information has to be somewhere accessible for non-AUR users, and made explicitly clear for less technically inclined individuals.

There's also an argument to be made that PrimeHack has only had 1 major update in 2 years, so the estimated ~90 users who have received this patch may have only done so with a new install rather than from updating from an old version, plus from my experience many of our users don't bother updating after the initial install without a specific reason. On the flip-side, flatpak would prompt them to update when this is released.

Hopefully this sheds some light behind my stance on this change and why we should be careful, considering how many users could be impacted.

[xiota]

I forgot people actually use Flatpak. Migration instructions for standard distros don't apply directly to Flatpak because it remaps directories. Each app has its own isolated home folder, so the data can be moved without any concern about whether it might interfere with a Dolphin instance. This can be done with a launcher script that is used only for the Flatpak release. Assuming variables and paths are set up inside the sandbox:

#!/usr/bin/env sh
_old_profile="$XDG_CONFIG_HOME/dolphin-emu"
_new_profile="$XDG_CONFIG_HOME/primehack"

if [ -e "$_old_profile" ] && [ ! -e "$_new_profile" ]; then
   mv "$_old_profile" "$_new_profile"
fi

exec primehack "$@"

[xiota]

The migration message will show the wrong paths on flatpak because the sandbox remaps folders.

[XargonWan]

There's also an argument to be made that PrimeHack has only had 1 major update in 2 years, so the estimated ~90 users who have received this patch may have only done so with a new install rather than from updating from an old version, plus from my experience many of our users don't bother updating after the initial install without a specific reason. On the flip-side, flatpak would prompt them to update when this is released.

In my experience users on Steam Deck just "update all" without even reading the version notes as there are some plugins that automatically updates all flatpaks with a single button press from game mode.
So if something needs to be told to the user you might want to make it tell by the software itself after its boot.

[SirMangler]

In my opinion, the most ideal solution would be to just automatically migrate if the folder doesn't exist / is empty. This would completely resolve the problem with flatpak altogether and there wouldn't be any risks of corruption.

If the folder isn't empty then you can assume someone manually migrated or has ran PrimeHack before, and if not, PrimeHack will still restore any missing configs and folders per Dolphin's normal behaviour.

The requirement of manual migration will be too problematic to make this PR worth it when I consider that we will have to herd hoards of inexperienced users to a new migration guide someone has to write, to download and run a robust migration tool someone has to write. PrimeHack itself can guarantee it will copy the right folder to the right destination at the right time with relatively zero risk. This would minimise the impact for all users, and at that point the popup-box wouldn't be required any more.

I would much prefer either making the primehack config folder optional, ie it checks for its existence to use it and otherwise falls back to the default folder, or auto-migration as stated above.

I do regret that PrimeHack on Linux wasn't using a custom config folder from the get-go and I realise that part of this ask is cleaning after up that oversight, but without migration, the users who benefit from this change are unfortunately niche relative to those who will be negatively effected for an otherwise insignificant change to them.

[xiota]

the most ideal solution would be to just automatically migrate

You can do this for Flatpak. As I show how in comment. This is relatively safe to do because the Flatpak sandbox ensures there is no conflicting Dolphin instance.

If the folder isn't empty...

You can use the same launcher script as Flatpak, but it's more risky on general Linux because there may be a Dolphin instance the move would interfere with.

Trying to move the config from within Dolphin itself is prone to mistakes. Also, Dolphin creates the new config folder before it even shows the GUI. So the new location won't be empty for a move. The best time to move is with a launcher script that can check folder locations before Dolphin is actually run.

... checks for its existence to use it ...

The more complicated you make it, the more likely you are to screw up users' data.

[SirMangler]

the most ideal solution would be to just automatically migrate

You can do this for Flatpak. As I show how in comment. This is relatively safe to do because the Flatpak sandbox ensures there is no conflicting Dolphin instance.

Using the path that Dolphin is about to load as it's user folder guarantees you're using the right path, there's no obscurity from sandboxes or how different environments may set the path, you can guarantee whatever path you're dealing with is what would have been used and that this will continue to work in the future. Using a script is relatively unsafe and requires set-up.

If the folder isn't empty...

Trying to move the config from within Dolphin itself is prone to mistakes. Also, Dolphin creates the new config folder before it even shows the GUI. So the new location won't be empty for a move. The best time to move is with a launcher script that can check folder locations before Dolphin is actually run.

Dolphin only starts to use the user folder after this line (same function as the initial run popup):

dolphin/Source/Core/DolphinQt/Main.cpp

Line 170 in 26beef4

UICommon::CreateDirectories();

You could do this in just a few lines of code, you can add an additional check to see if there's a dolphin-emu folder next to the new user folder, and copy the contents over.

This is the safest and most minimal approach.

... checks for its existence to use it ...

The more complicated you make it, the more likely you are to screw up users' data.

Creating a migration script and fitting it into flatpak's setup sounds an awful lot more convoluted, prone to mistakes and more to maintain compared to taking the existing code that handles creating the new user folder and making it copy dolphin-emu to the new user path if it exists. Here's an example of what I mean (untested but would most likely work):

  // UICommon.cpp:L251 - UICommon::CreateDirectories()
  std::string config_folder = File::GetUserPath(D_CONFIG_IDX);
  if (!File::Exists(config_folder) {
    File::CreateFullPath(config_folder);
    
    std::string parent_folder = config.folder_substr(0, s.rfind('/')); 
    if (File::Exists(parent_folder + "/dolphin-emu")) {
        File::Copy(parent_folder + "/dolphin-emu", config_folder);
    }
  }

[xiota]

@SirMangler I write launcher scripts all the time. They're simple and effective. After they've served their purpose, they can be disposed of. To do the same on the C++ side requires deeper familiarity with the program and many more lines of code. Developers also tend to be resistant to disposing of C++ when they're no longer needed. You should write the migration code yourself, especially if you insist it be done on the C++ side. Anyone else is likely to make a mistake. Also, after implementing your message-box suggestion, it's clear I do not agree with your idea of "simple" and "just a few lines of code".

@XargonWan The diff corresponding with this PR can be obtained by adding .diff to the PR URL: https://github.com/shiiion/dolphin/pull/166.diff. It should be suitable in its current state to keep PrimeHack and Dolphin configs separate on RetroDECK. Modifying build scripts to download and apply patches is a routine packaging activity. I have reverted the migration message to simplify the patch so it is easier for packagers to rebase as needed.

[SirMangler]

@SirMangler I write launcher scripts all the time. They're simple and effective. After they've served their purpose, they can be disposed of. To do the same on the C++ side requires deeper familiarity with the program and many more lines of code. Developers also tend to be resistant to disposing of C++ when they're no longer needed.

So to recap what you're saying:
We need to either
a) setup a launcher script, host it somewhere, inform users that they now need to download and teach people with very little linux experience how to use it, just to then dispose of it afterwards. Or
b) setup a launch script, write a new script into the flatpak to download the new launch script and make it use this instead of the executable.

Instead of adding a single code snippet to one function?

Anyone else is likely to make a mistake.

By the same token, your migration script mustn't be trusted then, this is compounded by the fact the paths are obtained externally to Dolphin, and do not follow what Dolphin ends up using (e.g sandbox/environment shenanigans or upstream changes).

You should write the migration code yourself, especially if you insist it be done on the C++ side.

Luckily for you, I did, I even gave you the exact line in the file you would insert this. Perhaps you missed it in my previous reply? #166 (comment)

However saying you need a "deep familiarity with the program" is simply not true. Otherwise, you yourself wouldn't have been able to implement something significantly more advanced than this in C++, your 43 lines of code involved opening and scanning a file to change the flow of logic. Your "deep familiarity with the program" even found the relevant file and function to modify, which I did not directly link you to. If what you're saying were true, then I don't see how you were able to write that migration script as it's doing effectively the same operation we need in C++.

it's clear I do not agree with your idea of "simple" and "just a few lines of code".

Clearly we don't. Even if we consider the route of forcing the majority of our users to download run your script, meaning there's no additional scripting work to be done, your script is a measly 9 lines, whereas the code I gave you is a whopping grand 9 lines.

I will reiterate once more, we aren't going to impact a majority of our community and force est. several thousands of inexperienced users to follow a migration guide that someone will need to write, for something that will literally have no benefit to them who just wanted to play the game and trusted us enough to keep auto-update on. This problem, the requirement to write new scripts, new guides, the burden of maintaining something external to the project and managing all the users flooding to us for answers and help, can be entirely avoided with minimal effort which won't need later removing or maintainance. 90% of this minimal effort has already been done for you.

[XargonWan]

@SirMangler I write launcher scripts all the time. They're simple and effective. After they've served their purpose, they can be disposed of. To do the same on the C++ side requires deeper familiarity with the program and many more lines of code. Developers also tend to be resistant to disposing of C++ when they're no longer needed. You should write the migration code yourself, especially if you insist it be done on the C++ side. Anyone else is likely to make a mistake. Also, after implementing your message-box suggestion, it's clear I do not agree with your idea of "simple" and "just a few lines of code".

@XargonWan The diff corresponding with this PR can be obtained by adding .diff to the PR URL: https://github.com/shiiion/dolphin/pull/166.diff. It should be suitable in its current state to keep PrimeHack and Dolphin configs separate on RetroDECK. Modifying build scripts to download and apply patches is a routine packaging activity. I have reverted the migration message to simplify the patch so it is easier for packagers to rebase as needed.

Thanks, I tried but unfortunately it fails applying the patches:

From f2d3b349d8487cf0ee50b36ba024ac513d36f826 Mon Sep 17 00:00:00 2001
From: Carles Pastor <[email protected]>
Date: Sat, 20 Aug 2022 13:35:32 +0200
Subject: [PATCH] Detect we are running inside a flatpak sandbox

---
 Source/Core/UICommon/UICommon.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Source/Core/UICommon/UICommon.cpp b/Source/Core/UICommon/UICommon.cpp
index 8d9dc2abb8..8a9d39a3d5 100644
--- a/Source/Core/UICommon/UICommon.cpp
+++ b/Source/Core/UICommon/UICommon.cpp
@@ -348,7 +348,7 @@ void SetUserDirectory(std::string custom_path)
     //    -> Use GetExeDirectory()/User
     // 2. $DOLPHIN_EMU_USERPATH is set
     //    -> Use $DOLPHIN_EMU_USERPATH
-    // 3. ~/.dolphin-emu directory exists
+    // 3. ~/.dolphin-emu directory exists, and we're not in flatpak
     //    -> Use ~/.dolphin-emu
     // 4. Default
     //    -> Use XDG basedir, see
@@ -381,7 +381,7 @@ void SetUserDirectory(std::string custom_path)
     {
       user_path = home_path + "." DOLPHIN_DATA_DIR DIR_SEP;
 
-      if (!File::Exists(user_path))
+      if (File::Exists("/.flatpak-info") || !File::Exists(user_path))
       {
         const char* data_home = getenv("XDG_DATA_HOME");
         std::string data_path =
-- 
2.37.1

[xiota]

@XargonWan I don't recognize that patch. Where is it from?

[XargonWan]

@XargonWan I don't recognize that patch. Where is it from?

Here, it's part of the flatpak manifest: https://github.com/flathub/io.github.shiiion.primehack

[xiota]

This PR doesn't alter UICommon.cpp, so don't know why you would have problems with that patch.

[XargonWan]

This PR doesn't alter UICommon.cpp, so don't know why you would have problems with that patch.

But probably the patches should be remade because:

-    // 3. ~/.dolphin-emu directory exists
+    // 3. ~/.dolphin-emu directory exists, and we're not in flatpak

I believe that the folder now are ~/.primehack or smilar, am I right?

[xiota]

I see what you mean now. The comment is wrong, it should use .primehack instead. But is that patch even needed? What happens if you don't use it?

[XargonWan]

I see what you mean now. The comment is wrong, it should use .primehack instead. But is that patch even needed? What happens if you don't use it?

Actually my build compiled now and the patches are applied, they were just outdated.
I don't really know what the patches implying but since they're applied to the original flatpak manfest, we are applying even on the RetroDECK's build.

Maybe @SirMangler knows more on the topic.

However albeit the build worked the patches might need an update, the first one is just a comment, but I don't know the implications of the second block that is actually code.

[xiota]

Looks like the patch makes it look for an additional config folder when running in flatpak. It's probably a backward compatibility hack. Since this PR changes the name of the folder, that whole block should be unused, and the patch shouldn't be needed. You should skip the extra patch.

[xiota]

Closing because, although not stated explicitly, this PR has been rejected in practice.