--connect and --libretro options used together

Hello,

I’ve been building a prototype netplay lobby viewer to solve some shortcomings and limitations present in both the application and web versions. It adds passworded room filtering, core/platform room filtering, non-connectable room filtering, more intuitive room sorting and pings.

Right now the prototype is just a console application printing the available rooms formatted to an unicode table.

I was researching into launching RetroArch directly from this viewer whenever the user wanted to join a room, coming across the -C/–connect and -L/–libretro options. Unfortunately, they don’t work; RetroArch always displays a “Failed to initialize netplay” message and no packet is ever sent to the host. Launching RetroArch with just the -C/–connect option and then loading the content manually works and so does -H/–host combined with -L/–libretro.

A look at the source code for the options -C/–connect and -L/–libretro didn’t show anything wrong (or at least obviously wrong).

Any ideas? This appears to be an initialization priority issue, but that wouldn’t explain why -H/–host works with -L/–libretro.

Thank you.

1 Like

Bump…

Still waiting for an answer on this. I need to know whether this is a RetroArch bug or an oversight on my part.

Tried doing it via stdin/network commands but came across two issues; the first is that there is no connect command and the second is that it appears to only work while in the menu.

However, I did come across a bug in input_driver.c.

input_driver_init_command:
         input_st->command[0] = command_stdin_new();
         if (!input_st->command[1])
            RARCH_ERR("Failed to initialize the stdin command interface.\n");

The correct should be:

         input_st->command[0] = command_stdin_new();
         if (!input_st->command[0])
            RARCH_ERR("Failed to initialize the stdin command interface.\n");

input_driver_deinit_command is called before input_driver_init_command, and input_driver_deinit_command sets all elements of the command array to NULL, causing that error to pop every single time if you’ve stdin commands enabled. The error is harmless, but it’s so simple to fix that there is no reason not to.

1 Like

I don’t have any answer for you and never heard any suggestions from the folks I shared this with, but can you post a log from one of the failures to connect?

Unfortunately I’d already tried running it with --verbose and the log shows absolutely nothing new. I am 99% sure that the error comes from function init_netplay on retroarch.c, and as you can see here:

   RARCH_WARN("%s\n", msg_hash_to_str(MSG_NETPLAY_FAILED));

   runloop_msg_queue_push(
         msg_hash_to_str(MSG_NETPLAY_FAILED),
         0, 180, false,
         NULL, MESSAGE_QUEUE_ICON_DEFAULT, MESSAGE_QUEUE_CATEGORY_INFO);
   return false;

The log just shows the same message, and the block that prevents the control flow from reaching it is this:

   if (p_rarch->netplay_data)
   {
      if (      p_rarch->netplay_data->is_server
            && !settings->bools.netplay_start_as_spectator)
         netplay_toggle_play_spectate(p_rarch->netplay_data);
      return true;
   }

So yeah, I’ll have to dig deeper since it seems no one knows about this issue, but thanks for asking them.

yeah, most of the netplay stuff was handled by one guy, Gregor, who isn’t really around anymore, so it’s a bit of a black box for most of us.

From the looks of it at netplay_frontend.c:netplay_new, the only things that can cause it to return NULL and let the control flow reach the error is malloc/calloc failing or init_socket/socket operations failing. 99.9% chance that malloc/calloc isn’t the problem, otherwise I would be seeing all kind of errors for heap space exhaustion, socket_nonblock is also unlikely to be the cause, this leave us with init_socket failing for whatever reason.

1 Like

Ugh, I feel kind of stupid right now; I should’ve tried this before on my own second instance.

The socket’s connect operation was the one failing. At the time of my tests, I’d just connected to someone via the built-in lobby system, so I knew it was working, but he must have closed it as I was setting up the command-line.

Testing on my own second instance, I was able to connect to it using the -C/–connect option combined with the -L/–libretro option.

Sorry for the bother that I may have caused; at least it served to find other issues/shortcomings related to netplay.

no worries. thanks for following up. I’m glad that one was a false alarm. Twinaphex is looking into the MITM issue right now.

Hey, if you don’t mind, also make sure to mention the bug on stdin command’s error message to anyone able to commit to the repository. It’s just a matter of changing an 1 to a 0.

1 Like

Ugh, it turns out it’s only working when connecting to my alternate instance, on any other room that I can connect via the built-in lobby system, I cannot connect via --connect + --libretro.

[INFO] [Netplay]: Connecting to netplay host
[ERROR] Failed to set up netplay sockets.
[WARN] Failed to initialize netplay.

No packets are sent; it’s failing before it sends a TCP SYN packet on socket_connect.

Attaching a debugger and finding the address of init_socket, I was able to determine that the reason it works for my own instance but not for remote ones is because my netplay IP address is set to 127.0.0.1; for whatever reason configuration_set_string(settings, settings->paths.netplay_server, optarg) is not sticking with the -C/–connect option, making RetroArch try to connect to the loopback everytime I launch it with that option combined with --libretro.

Now I need to figure out why it’s not sticking. Nothing in the source code indicates it’s getting overwritten, other than by -C or manual input in the menu. Sigh

EDIT: Okay, I know what’s happening. The command-line options are parsed and applied before the configuration file, which then overwrites netplay_server. All that happens before CMD_EVENT_NETPLAY_INIT is sent. I’ll try to manually remove netplay_server from the configuration file and see if it remains removed, if not, I’ll have to get creative with --appendconfig (lobby viewer creates a temporary file, add the netplay_server parameter to it, launch RetroArch and then gets rid of the file).

EDIT2: Yeah, removing it from the configuration file worked, but it’s written back on every launch. At this point, my only options that don’t include changing the source code and waiting for a new version is to either make it read-only or use --appendconfig. Read-only is a meh solution because I often change a thing or two in there.

1 Like

Updating this, --appendconfig doesn’t work with --libretro, at least not for netplay_ip_address and netplay_ip_port (if they are already set on the main config).

My final option will be to make a temporary copy of the main config, replacing those keys with the room I want to connect and then pass this temporary key via the --config option.

Hacky, but it’s the only solution that worked.

def make_config(self, **options: str) -> Optional[Path]:
    new_cfg: Optional[Path] = None

    try:
        with self.__main_config.open("r", encoding = "UTF-8") as cfg:
            with NamedTemporaryFile("w", encoding = "UTF-8", delete = False) as tmp_cfg:
                new_cfg = Path(tmp_cfg.name)

                key:   str
                value: str

                for line in cfg:
                    key, value = line.split('=', 1)

                    # Skip pre-defined keys.
                    if key.strip().casefold() not in options:
                        tmp_cfg.write(line)

                tmp_cfg.writelines(f'{key} = "{value}"\n' for (key, value) in options.items())
    except (OSError, ValueError):
        # Make sure to delete the temporary file on error.
        if new_cfg is not None:
            try:
                new_cfg.unlink()
            except OSError:
                pass

            new_cfg = None

    return new_cfg

...

ra_config: Optional[Path] = cast(RetroArch, config.retroarch).make_config(
    netplay_ip_address = join_room.address,
    netplay_ip_port = str(join_room.port),
    config_save_on_exit = "false"
)

if ra_config is None:
    Console.error("Failed to create temporary RetroArch configuration.")

try:
    cast(RetroArch, config.retroarch).launch(
        cast(Path, cast(Platform, join_room.platform).game(cast(str, join_room.game), cast(str, join_room.game), join_room.crc)),
        cast(str, cast(Platform, join_room.platform).core),
        "--config", str(ra_config),
        "--connect", join_room.address
    )
finally:
    # Wait a little bit before deleting as to allow RetroArch to read it.
    time.sleep(3)

    try:
        ra_config.unlink()
    except OSError:
        pass

Launching it from the lobby viewer on a random room:

[INFO] [Config]: Loading config from: "E:\Users\Admin\AppData\Local\Temp\tmp6xx4kozu".
[INFO] [Netplay]: Connecting to netplay host
[WARN] WARNING: A netplay peer is running a different version of RetroArch. If problems occur, use the same version.
[WARN] WARNING: A netplay peer is running a different version of the core. If problems occur, use the same version.
[INFO] Connected to: "Tiago"
[INFO] [netplay] You have joined as player 2

–appendconfig worked for me, but only if I don’t start RetroArch with content loaded.

Both things might be worth looking into.

I couldn’t find why --appendconfig wouldn’t work for netplay_ip_address when launching via --libretro. Everything in the source code indicated that it would be loaded right after retroarch.cfg, appending its content into the config object; I even got the “config appended” log without the failed one.

-C/–connect should be refactored into storing its argument separately from netplay_ip_address, with its format changed from “address” to either “address:port” or “address|port” to avoid conflict with netplay_ip_port aswell (–port option should be used for -H/–host only).

Good advice. I’ll pass it along. Do you use discord?

Currently, no social media for me (personal reasons), mainly communicating via forums or email. I did want to try some open-source ones that are faithful successors to IRC (Matrix, Tox), but no one uses them.

1 Like

Ha, yeah, I’m on tox. It’s all tumbleweeds and crickets.

Do you’ve a libretro group on Tox?

no, but I guess I could set something up.

I’ve fixed the -C/–connect option to use a different (global) string variable. Could you pass this along? I’d appreciate it if you could.

All changes are in retroarch.c.

Global scope:

#ifdef HAVE_NETWORKING
static const char *connect_host = NULL;
#endif

retroarch_parse_input_and_config function:

From:

            case 'C':
               retroarch_override_setting_set(
                     RARCH_OVERRIDE_SETTING_NETPLAY_MODE, NULL);
               retroarch_override_setting_set(
                     RARCH_OVERRIDE_SETTING_NETPLAY_IP_ADDRESS, NULL);
               netplay_driver_ctl(RARCH_NETPLAY_CTL_ENABLE_CLIENT, NULL);
               configuration_set_string(settings,
                     settings->paths.netplay_server, optarg);
               break;

To:

            case 'C':
               retroarch_override_setting_set(
                     RARCH_OVERRIDE_SETTING_NETPLAY_MODE, NULL);
               netplay_driver_ctl(RARCH_NETPLAY_CTL_ENABLE_CLIENT, NULL);
               connect_host = strdup(optarg);
               break;

command_event function:

From:

         /* init netplay manually */
      case CMD_EVENT_NETPLAY_INIT:
         {
            char       *hostname       = (char*)data;
            const char *netplay_server = settings->paths.netplay_server;
            unsigned netplay_port      = settings->uints.netplay_port;

            command_event(CMD_EVENT_NETPLAY_DEINIT, NULL);

            if (!init_netplay(p_rarch,
                     settings,
                     NULL,
                     hostname
                     ? hostname
                     : netplay_server, netplay_port))
            {
               command_event(CMD_EVENT_NETPLAY_DEINIT, NULL);
               return false;
            }

            /* Disable rewind & SRAM autosave if it was enabled
             * TODO/FIXME: Add a setting for these tweaks */
#ifdef HAVE_REWIND
            state_manager_event_deinit(&runloop_st->rewind_st);
#endif
#ifdef HAVE_THREADS
            autosave_deinit();
#endif
         }
         break;

To:

         /* init netplay manually */
      case CMD_EVENT_NETPLAY_INIT:
         {
            char       *hostname       = (char*)data;
            const char *netplay_server = NULL;
            unsigned    netplay_port   = 0;

            if (connect_host && !hostname)
            {
               struct string_list *addr_port = string_split(connect_host, "|");

               if (addr_port && addr_port->size == 2)
               {
                  char *tmp_netplay_server = addr_port->elems[0].data;
                  char *tmp_netplay_port   = addr_port->elems[1].data;

                  if (!string_is_empty(tmp_netplay_server) &&
                     !string_is_empty(tmp_netplay_port))
                  {
                     netplay_port = strtoul(tmp_netplay_port, NULL, 10);

                     if (netplay_port && netplay_port <= 0xFFFF)
                     {
                        netplay_server = strdup(tmp_netplay_server);

                        // This way we free netplay_server aswell when done.
                        free(connect_host);
                        connect_host = netplay_server;
                     }
                  }
               }

               string_list_free(addr_port);
            }

            if (!netplay_server || !netplay_port)
            {
               netplay_server = settings->paths.netplay_server;
               netplay_port   = settings->uints.netplay_port;
            }

            command_event(CMD_EVENT_NETPLAY_DEINIT, NULL);

            if (!init_netplay(p_rarch,
                     settings,
                     NULL,
                     hostname
                     ? hostname
                     : netplay_server, netplay_port))
            {
               command_event(CMD_EVENT_NETPLAY_DEINIT, NULL);

               if (connect_host)
               {
                  free(connect_host);
                  connect_host = NULL;
               }

               return false;
            }

            if (connect_host)
            {
               free(connect_host);
               connect_host = NULL;
            }

            /* Disable rewind & SRAM autosave if it was enabled
             * TODO/FIXME: Add a setting for these tweaks */
#ifdef HAVE_REWIND
            state_manager_event_deinit(&runloop_st->rewind_st);
#endif
#ifdef HAVE_THREADS
            autosave_deinit();
#endif
         }
         break;
1 Like