RetroPie / RetroPie-Setup

Shell script to set up a Raspberry Pi/Odroid/PC with RetroArch emulator and various cores
Other
10.06k stars 1.39k forks source link

Retroarch patching problems #1620

Closed sparkchaser closed 8 years ago

sparkchaser commented 8 years ago

I was doing a fresh install of RetroPie on an existing RPi setup (Ubuntu MATE 16.04), which involved needing to build practically everything from source. I ran into some problems with the script that builds Retroarch (scriptmodules/emulators/retroarch.sh).

The first time the script ran, Retroarch built unsuccessfully because of a dependency problem (my fault, easily fixed). The next time I tried to run the installer, building Retroarch would fail with "redefinition of ‘hotkey_counter’" compiler errors. I dug into the scripts and discovered that the problem is in the function sources_retroarch(). Every time the function is run, it applies 01_hotkey_hack.diff to input/input_driver.c. In the event that a previous compile failed, the source code is already present and this file will have already been patched. The function will blindly patch it a second time, mangling the contents.

I believe two things need to be changed to resolve this issue. First, the patching only needs to be done if the file hasn't been patched before. Something like this might work instead of the current patch call:

if [[ ! -f input/input_driver.c.bak ]]; then
    cp input/input_driver.c input/input_driver.c.bak
    patch -p1 <"$scriptdir/scriptmodules/emulators/$md_id/01_hotkey_hack.diff"
    [[ $? -ne 0 ]] && echo "Error: Failure patching input/input_driver.c"
fi

Second, the return value of the patch call needs to be checked to make sure that it worked. If patching didn't work correctly, it's better to fail out here with an error message than to wait and get a cryptic compiler error later.

In addition, your patch file doesn't seem to match the current Retroarch source code. The following is my updated version of the patch using the current codebase (needs cleanup, but it seems to work):

--- a/input/input_driver.c
+++ b/input/input_driver.c
@@ -100,6 +100,11 @@
 #endif
 static const input_driver_t *current_input        = NULL;
 static void *current_input_data                   = NULL;
+
+/* number of frames required to trigger the hotkey */
+#define HOTKEY_DELAY 5 
+static unsigned hotkey_counter = 0;
+
 static bool input_driver_block_hotkey             = false;
 static bool input_driver_block_libretro_input     = false;
 static bool input_driver_osk_enabled              = false;
@@ -560,9 +565,22 @@
    else
       input_driver_unset_hotkey_block();

+#if 0
    /* If we hold ENABLE_HOTKEY button, block all libretro input to allow
     * hotkeys to be bound to same keys as RetroPad. */
    return (use_hotkey_enable && enable_hotkey);
+#endif
+   if (use_hotkey_enable && enable_hotkey)
+   {
+      if (hotkey_counter < HOTKEY_DELAY)
+         hotkey_counter++;
+   }
+   else
+      hotkey_counter = 0;
+
+   /* If we hold ENABLE_HOTKEY button for HOTKEY_DELAY frames, block all
+    * libretro input to allow hotkeys to be bound to same keys as RetroPad. */
+   return (hotkey_counter == HOTKEY_DELAY);
 }

 /**
joolswills commented 8 years ago

Thanks - what dependency problem btw ? if we are missing a dependency we can add that too.

If adjusting the script you can also force a clean build by doing sudo ./retropie_packages.sh retroarch clean

I think I changed some logic recently so the source doesn't get cleaned if a build fails, as it was getting in the way of debugging/testing etc. Will look into making sure the patch is only applied once as you have detailed.

joolswills commented 8 years ago

I changed the patch to remove the empty line before /* number of frames required to trigger the hotkey */ as with that change patch knows it has already been applied and wouldn't apply again, but added additional safety measures also (but slightly differently from above, so any error would appear in a dialog if building via retropie-setup (eg if retroarch changes their code so the patch breaks)

Thanks!

sparkchaser commented 8 years ago

Thanks for the quick response!

The dependency problem was with sdl2. For some reason, the scripts could never detect that it was installed. The package name on my distro was a bit different than what the scripts expected and I did some renaming to get things working, but my apparent inability to spell correctly caused Retroarch to fail while linking (easy fix, it's building correctly now). It seems like a distro-specific problem and since it's not an officially-supported distro, it's probably nothing to worry about.

On Aug 7, 2016 2:36 AM, "Jools Wills" notifications@github.com wrote:

Thanks - what dependency problem btw ? if we are missing a dependency we can add that too.

If adjusting the script you can also force a clean build by doing sudo ./retropie_packages.sh retroarch clean

I think I changed some logic recently so the source doesn't get cleaned if a build fails, as it was getting in the way of debugging/testing etc. Will look into making sure the patch is only applied once as you have detailed.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/RetroPie/RetroPie-Setup/issues/1620#issuecomment-238068563, or mute the thread https://github.com/notifications/unsubscribe-auth/ABU_fwzxv4Nl78Ryob67q-BH-J1O0E8Kks5qdYrggaJpZM4JeZ8p .