Skip to content

Fall back to global system dir if content path is empty when 'System Files are in Content Directory' is enabled#16175

Merged
LibretroAdmin merged 1 commit intolibretro:masterfrom
bslenul:contentless-system-dir
Feb 1, 2024
Merged

Fall back to global system dir if content path is empty when 'System Files are in Content Directory' is enabled#16175
LibretroAdmin merged 1 commit intolibretro:masterfrom
bslenul:contentless-system-dir

Conversation

@bslenul
Copy link
Copy Markdown
Contributor

@bslenul bslenul commented Jan 29, 2024

Description

Kind of a followup to #16170

Currently when running a core that requires firmware files without content with "System Files are in Content Directory" enabled, RetroArch will look for the firmware files in either an empty path or the previous system dir used.

So for example with "System Files are in Content Directory" enabled:

  • If I start RA and I run LRPS2 contentless, the path "" will be checked.
  • If I start RA, run a PS1 game from C:\Games\PS1, then run LRPS2 contentless, the path "C:\Games\PS1" will be checked.

(and to be clear: this is not a regression from my previous PR, it was already acting like that before :p )

This PR tries to solve this by falling back to the global system dir instead. So for example if you want to use LRPS2, PUAE or melonDS DS contentless while "System Files are in Content Directory" is enabled, it will now work properly if you have the correct files in your system dir.
If both system dir and content paths are empty not much can be done tho, so it will still keep checking for files from an empty path like before...

Also adjusted my previous PR so that the slash isn't removed from the system path if it's root dir. Although I'm starting to wonder if I should keep that slash removal at all tbh... it's really just there for consistency and cosmetic purpose, idk if it's worth keeping :/

Reviewers

Anyone really, I'm open to any comment/suggestion.

@hizzlekizzle
Copy link
Copy Markdown
Collaborator

I wouldn't worry about the trailing slash. At least you'll know for sure that it's a directory

@LibretroAdmin
Copy link
Copy Markdown
Contributor

Regarding trailing slash, aren't there some libretro-common file_path functions we usually go through to deal with this kind of stuff? Might be preferable and less messy than having to sprinkle custom _WIN32 ifdefs throughout the code like this.

@bslenul
Copy link
Copy Markdown
Contributor Author

bslenul commented Jan 29, 2024

Couldn't find any that does what I'm looking for (my goal being to remove the trailing slash in path unless it's a root dir like "C:\" or "/" where in this case I want to keep it), but I just noticed there's string_count_occurrences_single_character(), I can use that to count slashes and if > 1 then remove the trailing one.

So

diff --git a/menu/menu_displaylist.c b/menu/menu_displaylist.c
index e3a2e21cfb..8bd69ca3dd 100644
--- a/menu/menu_displaylist.c
+++ b/menu/menu_displaylist.c
@@ -735,17 +735,11 @@ static int menu_displaylist_parse_core_info(
          else
          {
             size_t len       = strlen(tmp_path);
-#ifdef _WIN32
-            bool is_root_dir = tmp_path[len - 2] == ':'
-                            && tmp_path[len - 1] == '\\';
-#else
-            bool is_root_dir = string_is_equal(tmp_path, "/");
-#endif

             /* Removes trailing slash (unless root dir), doesn't really matter
              * but it's more consistent with how the path is stored and
              * displayed without 'System Files are in Content Directory' */
-            if (    !is_root_dir
+            if (     string_count_occurrences_single_character(tmp_path, PATH_DEFAULT_SLASH_C()) > 1
                   && tmp_path[len - 1] == PATH_DEFAULT_SLASH_C())
                tmp_path[len - 1] = '\0';
             firmware_info.directory.system = tmp_path;
diff --git a/runloop.c b/runloop.c
index caa2ce91e7..ff546b34f4 100644
--- a/runloop.c
+++ b/runloop.c
@@ -1975,7 +1975,6 @@ bool runloop_environment_cb(unsigned cmd, void *data)
                if (!fullpath_is_empty)
                {
                   size_t len;
-                  bool is_root_dir = false;
                   char tmp_path[PATH_MAX_LENGTH];

                   if (dir_system_is_empty)
@@ -1987,13 +1986,7 @@ bool runloop_environment_cb(unsigned cmd, void *data)

                   /* Removes trailing slash (unless root dir) */
                   len = strlen(tmp_path);
-#ifdef _WIN32
-                  is_root_dir = tmp_path[len - 2] == ':'
-                             && tmp_path[len - 1] == '\\';
-#else
-                  is_root_dir = string_is_equal(tmp_path, "/");
-#endif
-                  if (    !is_root_dir
+                  if (     string_count_occurrences_single_character(tmp_path, PATH_DEFAULT_SLASH_C()) > 1
                         && tmp_path[len - 1] == PATH_DEFAULT_SLASH_C())
                      tmp_path[len - 1] = '\0';

Would that be OK? Can't really think of anything else atm 🤔

@LibretroAdmin
Copy link
Copy Markdown
Contributor

I guess that would work.

@bslenul bslenul force-pushed the contentless-system-dir branch 2 times, most recently from 9a1e5f9 to 382f213 Compare January 30, 2024 12:21
@bslenul
Copy link
Copy Markdown
Contributor Author

bslenul commented Jan 30, 2024

One last force push, I removed the useless CONTENT_ST_FLAG_CORE_DOES_NOT_NEED_CONTENT flag check, if content path is empty it should fall back to global system dir no matter what, no reason for additional checks.

edit: Getting rid of an extra if statement and small cleanup, last force update (for real this time!), sorry about that :/ But the final PR has very minimal changes now.

@bslenul bslenul changed the title Fall back to global system dir for contentless cores when 'System Files are in Content Directory' is enabled Fall back to global system dir if content path is empty when 'System Files are in Content Directory' is enabled Jan 30, 2024
@bslenul bslenul force-pushed the contentless-system-dir branch from 382f213 to ab7282c Compare January 30, 2024 13:32
@LibretroAdmin LibretroAdmin merged commit 9e6b790 into libretro:master Feb 1, 2024
@bslenul bslenul deleted the contentless-system-dir branch February 1, 2024 17:49
Sunderland93 pushed a commit to Sunderland93/RetroArch that referenced this pull request Dec 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants