Draft: Fix #746 (CLI) Auto-detect Core when -L isn't present

[zig-for]

Guidelines

1. Rebase before opening a pull request
2. If you are sending several unrelated fixes or features, use a branch and a separate pull request for each
3. If possible try squashing everything in a single commit. This is particularly beneficial in the case of feature merges since it allows easy bisecting when a problem arises

Description

This allows Retroarch to attempt to pick the proper core when loading. Windows already does this on drag and drop.

The PR as is doesn't handle missing cores, nor does it delegate to the frontend if there's multiple possible cores. Happy to put work towards that if there's a desire for this to go in. The main desire is to try to push for a "good" solution even if no perfect solution exists (I'd rather a happy path of being able to easily set file extensions associated with RA to do the right thing, then give up on every case just because some cases are hard to handle).

To repeat - as is, the PR is not ready, just looking for direction on both implementation and probability of acceptance later

EDIT: Additional changes:

• Always allows downloading cores when suggesting cores
• Fixes bug that caused cancelling "Core Downloader" to pop the entire menu stack if run from anywhere but "Online Updater"
• The core download menu is now filtered when appropriate

Current bugs:

• [ ] When backing out from the content suggester, the main menu on the left isn't scrolled properly
• [ ] Suggested cores don't show licenses (is this desired? I kind of like the slim, clean look)

Related Issues

https://github.com/libretro/RetroArch/issues/746 https://github.com/libretro/RetroArch/issues/9275 https://forums.libretro.com/t/autodetect-core-when-opening-a-rom-file/1863 https://github.com/libretro/RetroArch/issues/1704 https://github.com/libretro/RetroArch/issues/8557

Reviewers

[If possible @mention all the people that should review your pull request]

[hizzlekizzle]

I think this is welcome functionality and indeed picking some core behind the scenes is better than failing with no core at all. For multiple cores, I think the ideal behavior would be to launch into the menu at the point of the core picker dialog, just as you would if you just 'load content'.

[zig-for]

I attempted to get @hizzlekizzle 's idea working, but can't figure out how to control the menu programatically. I tried something along the lines of menu_displaylist_ctl(DISPLAYLIST_CORES, &info, settings); and deferred_push_core_list(&info); (with empty lists for now), but neither control the menu. I'm not sure if:

1. Calling these with empty lists just doesn't work
2. I'm calling the wrong function
3. The menu system isn't in a proper state to accept control at this point in execution

Could I get a hint for what to do?

[zig-for]

Made some progress, with the caveat that it involves moving menu initialization earlier. :(

[zig-for]

Looking at the menu code, it's ok to initialize the menu multiple times, even early. @LibretroAdmin could you comment on direction here? The other option is figuring out some deferral method that waits until after the menu is actually initialized (some sort of hook in the main menu?).

There's a case that doesn't work as well as I'd like - downloading:

1. Cores aren't filtered by the current content type (I think this is fixable, I can potentially read detect_content_path, please comment if this is desired)
2. if you download a core it doesn't launch the core (this doesn't really look fixable - core downloads are very asynchronous - unless I disallow swapping away from the core download screen or put up some sort of modal menu I don't see options, also open to direction) EDIT: 3. It would be neat if "suggested cores" also included a download button - and then also would be neat if downloading added to the stack rather than replacing.

I need to do another pass at C89 compat, but will do so after I know I'm not barking up the wrong tree.

[zig-for]

Fixed (2) and (3) - there was a bug in how the stack was popped - now downloading a core, waiting, and backing out does the right thing (it would be extra neat if the list refreshed itself, but I'm not super worried about it)

[zig-for]

....ok, now when you go to download the core, you're only presented with cores associated with the content. The way I implemented the clear on cancel seems wrong, please lemme know if there's a nicer way. What I really want to do is associate a cancel option with the window, not every item on the window, even though I get why the extra flexibility would be nice. Please lemme know the "proper" way of doing this, I feel like I took a sledgehammer to the menu system.

[hizzlekizzle]

being able to download a core from there is a great idea, and having it only show eligible cores is even better. :)

good stuff!

[keithbowes]

I had been thinking about trying my hand at this, but I hadn't started yet. I have written a shell script that looks at .info files for file extensions and then prompts you to enter a number which core you want if more than one core supports a file extension, and was thinking about doing the same in RetroArch when it is built with --disable-menu.

[LibretroAdmin]

Hi, quite a few CI tasks are failing. Can you check it out and fix it?

[zig-for]

Yes, of course. I'm out of town for four days, but I will circle back to this next week.

[LibretroAdmin]

Let us know when you're ready to look into the matter.

[RobLoach]

Just sharing that this has been on my wishlist FOREVER. Thanks a lot for picking it up.

I tried building locally, and got the following...

runloop.c:(.text+0x3652): undefined reference to `file_load_with_detect_core_wrapper'

As a side-note to https://github.com/libretro/RetroArch/pull/15458#issuecomment-1622944645 , make sure to Update Assets.

[zig-for]

I am going to try to work on this tomorrow. Got back from vacation, got slammed with work, and now MAGWEST is upon me. :)

[zig-for]

@LibretroAdmin fixed the build errors

[zig-for]

Not for a few weeks. Slammed with work.Sent from my iPhoneOn Sep 13, 2023, at 5:19 AM, gouchi @.***> wrote: @gouchi commented on this pull request.

In runloop.c:

@@ -3615,9 +3680,51 @@ bool runloop_init_libretro_symbols(

            if (string_is_empty(path))
            {

•              RARCH_ERR("[Core]: Frontend is built for dynamic libretro cores, but "
  

•              const char* content_path = path_get(RARCH_PATH_CONTENT);
  

•              if (!string_is_empty(content_path))
  

•              {
  

•                 bool core_loaded = auto_load_core(content_path);
  

•                 if (!core_loaded) {
  

•                    struct menu_state* menu_state = menu_state_get_ptr();
  

•                    CORE_SYMBOLS(SYMBOL_DUMMY);
  

•                    runloop_set_current_core_type(CORE_TYPE_DUMMY, false);
  

+#ifdef HAVE_MENU

•                    /* this is a hack, we should be deferring the core list until after we have loaded menu */
  

•                    /* TODO: ask how to unhackify it */
  

Hi @zig-for will it be possible to rebase so that you can get the two fixes for the option --disable-menu ? Thank you.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

[RobLoach]

Thanks, brought in the latest from master.

[gouchi]

@zig-for It will be a great feature to have !

Current status some conflict with current latest cb69d25

menu_displaylist.c.rej

--- menu/menu_displaylist.c
+++ menu/menu_displaylist.c
@@ -1925,7 +1926,7 @@ static unsigned menu_displaylist_parse_system_info(file_list_t *list)
             uint64_t memory_used  = memory_total - frontend_driver_get_free_memory();
             if (memory_used != 0 && memory_total != 0)
             {
-               _len = strlcpy(entry, 
+               _len = strlcpy(entry,
                      msg_hash_to_str(MSG_MEMORY), sizeof(entry));
                snprintf(entry + _len, sizeof(entry) - _len, ": %" PRIu64 "/%" PRIu64 " MB",
                      BYTES_TO_MB(memory_used), BYTES_TO_MB(memory_total));
@@ -1988,7 +1989,7 @@ static unsigned menu_displaylist_parse_system_info(file_list_t *list)
    {
       gfx_ctx_ident_t ident_info;
       video_context_driver_get_ident(&ident_info);
-      
+
       /* Video Context Driver */
       snprintf(entry, sizeof(entry), "%s: %s",
             msg_hash_to_str(MENU_ENUM_LABEL_VALUE_SYSTEM_INFO_VIDEO_CONTEXT_DRIVER),
@@ -1999,7 +2000,7 @@ static unsigned menu_displaylist_parse_system_info(file_list_t *list)
             MENU_ENUM_LABEL_SYSTEM_INFO_ENTRY, MENU_SETTINGS_CORE_INFO_NONE,
             0, 0, NULL))
          count++;
-      
+
       {
          gfx_ctx_metrics_t metrics;
          float val = 0.0f;
@@ -12171,7 +12172,7 @@ bool menu_displaylist_ctl(enum menu_displaylist_ctl_state type,
                char mixer_stream_str[128];
                unsigned id                 = info->type - MENU_SETTINGS_AUDIO_MIXER_STREAM_ACTIONS_BEGIN;
                size_t _len                 = strlcpy(mixer_stream_str, "mixer_stream_", sizeof(mixer_stream_str));
-               
+
                lbl[0]                      = '\0';
 
                snprintf(mixer_stream_str + _len, sizeof(mixer_stream_str) - _len, "%d", id);

menu_displaylist.h.rej

--- menu/menu_displaylist.h
+++ menu/menu_displaylist.h
@@ -316,9 +316,7 @@ enum menu_dl_flags
    MD_FLAG_NEED_CLEAR                    = (1 << 5), /* Should we clear the displaylist before we push
                                                      * entries onto it? */
    MD_FLAG_PUSH_BUILTIN_CORES            = (1 << 6),
-   MD_FLAG_DOWNLOAD_CORE                 = (1 << 7), /* Should a 'download core' entry be pushed onto the list?
-                                                     * This will be set to true in case there are no currently
-                                                     * installed cores. */
+   MD_FLAG_DOWNLOAD_CORE                 = (1 << 7), /* Should a 'download core' entry be pushed onto the list? */
    MD_FLAG_NEED_NAVIGATION_CLEAR         = (1 << 8)  /* Does the navigation index need to be cleared 
                                                       * to 0 (first entry) ? */
 };