Gemba
commented
11 months ago - Added addlib function (
sudo ./retropie_packages.sh gmloader addlib <apk>) addslibc++_shared.soto other APK a user may deploy - Added RetroPie exit emulator button combo
Closed Gemba closed 10 months ago
Gemba
commented
11 months ago sudo ./retropie_packages.sh gmloader addlib <apk>) adds libc++_shared.so to other APK a user may deploy
s1eve-mcdichae1
commented
11 months ago I saw the link, but I don't feel much smarter. What is the libc++_shared.so for?
s1eve-mcdichae1
commented
11 months ago Some testing this morning:
I remembered (/slash rediscovered) why I didn't use the "optimized" version of Spelunky initially -- it wouldn't run on faf3970, only gave segfault error. "Final" version is 10x the size, but at least it run.
faf3970 don't build in current Raspbian -- I didn't do a full system upgrade but I update all the package's depends, and got "cannot find source file lodepng/lodepng.h" error after cmake. Is this something that used to be provided at the system level by one of the depends, but no longer is?
I can get it to build if I clone lodepng (https://github.com/lvandeve/lodepng) inside the droidports repo before cmake.
However, then (as with the current master/HEAD version), none of the games will play without the libc++_shared.so file. Is this also something that used to be provided by a depend?
My old installed version of gmloader is still playing my old installed versions of the games no problem (no lodepng.h, no libc++_shared.so, just the app and the .apk). But when I build master/HEAD or clone lodepng and re-build faf3970, they won't load without the libc++_shared.so.
That's the part that's most weird to me. The faf3970 that built a year ago works just fine, but that same faf3970 built today, don't work without the extra file. So, like... is it a case of, this file used to be available "to the system" and so it was built-in to the executable at compile time but, no longer provided at a system level, the file is not currently built-in and so must be provided individually within each game? And if that's the case, can we put it back in the system/executable so we don't have to pack it back into every game?
Tagging @JohnnyOnFlame in case they have any insights.
JohnnyonFlame
commented
11 months ago Requiring libc++_shared.so is a change introduced two years later on cf97077 in order to support newer versions of the GM runner and provide a more accurate version of these symbols. Keep in mind I will not provide support for older versions.
libc++_shared.so can be sourced from any NDK install, here's one I use for Spelunky on RG35xx: libc++_shared.zip.
Here are the scripts and packaging I provide for Spelunky on RG35xx and locomalito games for the Evercade as examples on how to provide packaging and scripts for droidports.
Anything else I can do to help you migrate to HEAD?
Gemba
commented
11 months ago Thanks for the prompt review @s1eve-mcdichae1. Will reflect your feedback in the next days.
Amending to @JohnnyonFlame: The libc++_shared.so has to be packaged to an APK if an APK binary relies on that lib, a system libc++ won't do it. This was introduced by "android development rules" (there might be an official name) a while ago and after the both sample APK/games provided with this script have been released and were no longer maintained. As gmloader plays by the android development book this adding/repackaging is needed. I got this libc copy from a NDK / Android Studio install.
s1eve-mcdichae1
commented
11 months ago It looks to work if libc++_shared.so exists in CWD so we don't need to pack it in each game, just copy it once to $md_inst and pushd there before launch.
is there an official online source the module can download this from, rather than distributing it along with?
JohnnyonFlame
commented
11 months ago I'm sourcing from my own NDK install but you could probably just use https://chromium.googlesource.com/android_ndk.git/+/refs/heads/main/sources/cxx-stl/llvm-libc++/libs/armeabi-v7a/
Just remember to strip the binary, otherwise you're wasting ram/disk with debug nonsense. It goes from 4mb to 500kb.
s1eve-mcdichae1
commented
11 months ago Looks like we also do not need the config-dir patch any longer, we can just define that with GMLOADER_SAVEDIR= at runtime.
Here's my current suggestions: https://github.com/Gemba/RetroPie-Extra/compare/fix_gmloader_scriptmodule...s1eve-mcdichae1:RetroPie-Extra:fix_gmloader_scriptmodule
s1eve-mcdichae1
commented
11 months ago Just remember to strip the binary, otherwise you're wasting ram/disk with debug nonsense. It goes from 4mb to 500kb.
(should it be executable? Does it matter? This one came that way but the other ones didn't. Should I unset it or just leave it be? It seems to work either way.)
JohnnyonFlame
commented
11 months ago Flags shouldn't matter at all, seems correct.
Gemba
commented
11 months ago Incorporated the suggested changes. Factorized a small launcher. Unset execute bits on the lib as it is OS default.
Having to have the branch name in the repo url should be fixed in RetroPie-Setup itself. Either the parameter is mandatory or optional but not something else/depending on the function called. Will table it at RetroPie-Setup.
BTW: Do you know what the amr2 lines are for, when they where added and why?
JohnnyonFlame
commented
11 months ago Incorporated the suggested changes. Factorized a small launcher. Unset execute bits on the lib as it is OS default.
Having to have the branch name in the repo url should be fixed in RetroPie-Setup itself. Either the parameter is mandatory or optional but not something else/depending on the function called. Will table it at RetroPie-Setup.
BTW: Do you know what the amr2 lines are for, when they where added and why?
Meant to be used with AM2R apk built from one of the patchers provided by the AM2R Community Developers. You can also factor out some of their changes and provide your own installer, just don't redistribute the AM2R 1.1 data, users should source that on their own.
s1eve-mcdichae1
commented
11 months ago What advantage does this extra script have over just putting a one-liner in the emulator command, as in:
addPort "$md_id" "droidports" "Maldita Castilla" "pushd $md_inst; GMLOADER_SAVEDIR=$home/.config/gmloader/%BASENAME%/ ./gmloader %ROM%; popd" "$maldita_file"
I feel like sourcing this file should happen in the sources function. Reason: it's a (very) niche case but I can think of one situation where it would matter: an advanced user with knowledge of the various retropie_packages.sh functions, goes online and performs depends and sources, and later intends to run build, install, configure and clean while offline. If the file is sourced during install then it will fail when this user runs it from offline.
Are there any other modules that use an online feature outside of depends, sources and install_bin? Answering my own question, yeah this very module and many others often acquire gamedata in configure.
So it's not unheard-of, but install still feels like maybe only the 3rd best place to put it.
While we're on this file, do we need to (un-)flag it? Seems to work fine either way so what's the rationale of changing from how it comes delivered?
Oh, I see Unset execute bits on the lib as it is OS default.
I'll take your word for it. I don't know what "should" be one way or another so I scanned my system and saw lots of .so files both with and without the x bits, so I eventually have just left the sourced file as-is, seeming that it doesn't matter either way.
I don't know if it's relevant, but the official RetroPie modules never quote chown $user:$user. It's a break from the common mantra of "always quote variables" but maybe since it's a username, it literally can't have spaces or any of the other special characters that would make the quoting matter? I dunno, but they never do it, so I never do either.
And again with the a-x; this time the files downloaded don't even have the bit -- is there a compelling reason to try and remove it, then?
Another thing that "I always do the way they always do," is single-quote the elements of $md_ret_files, at https://github.com/Exarkuniv/RetroPie-Extra/pull/161/files#diff-a5d92dc54968117a93d91fe0ec5a499bb79a7f824cd5ab2e4dd9fd9dcfc00d4aR39-R43
Something they never do, is mkdir build && cd build in one line like I have done. That's an artefact from when I was first learning how to even do a build, let alone make it into a scriptmodule, and someone recommended me these steps and they've been in here ever since. If we're "fixing" this (which I think is really only even broken if a build is failed or aborted before the clean step and in any case, the only remedy is to clean and start over anyway), then why don't we just make it two lines like every other module:
mkdir build
cd buildThis doesn't introduce any additional failures and will yield success in some situations (build folder already exists but is still useable -- mkdir will fail but cd will still execute and remainder of build can continue from the correct location) that previously would not (mkdir fails and so cd is not attempted, and remainder of build is attempted from the wrong location.)
BTW: Do you know what the amr2 lines are for, when they where added and why?
They are for AM2R: Another Metroid 2 Remake, the whole reason I began looking at gmloader in the first place. It's a free fan-created game but Nintendo forced them to cease distribution, so you have to source the game yourself, but once you do, this creates a launch script for it. Those particular lines were added in efb4af4 when I rewrote the configure function to download the free games and write specific launch scripts for them (and for AM2R if provided), rather than only scan the directory and bulk-add generic-named scripts for whatever was present there before install.
s1eve-mcdichae1
commented
11 months ago BTW: Do you know what the amr2 lines are for, when they where added and why?
In case you're referring to the purpose of:
if [[ -f "$am2r_file" || "$md_mode" == "remove" ]]; then
addPort ......the idea is, if the file exists, do the function always. But even if the file doesn't exist, then if we are in remove mode, still do it anyway.
addPort in remove mode does: remove the specific emulator, if no emus left for this port remove the launch script, if no ports left at all remove the "Ports" system. They may have already removed their game file so it might not exist, but we still want to do the function just in case (it doesn't hurt anything because it'd happen anyway, if the file was still there -- as it does for the other 2 games regardless. It just cleans up some loose threads that might otherwise be left behind.)
Gemba
commented
11 months ago The comments on 8988bcf were on a interims commit. Just to keep us both busy I pushed a new version. The majority of things are addressed.
The things I did not change are for reasons:
find /lib/ -name "*.so" -type f -executable (and in /usr/lib)s1eve-mcdichae1
commented
11 months ago The things I did not change are for reasons:
- I did not find many libs on my Rpi setup with executable bit, the vast majority is non-executable marked. Check for yourself:
find /lib/ -name "*.so" -type f -executable(and in/usr/lib)
I'll defer to you on this one (with long-winded dissent, to follow):
- The link to https://github.com/JohnnyonFlame/droidports is there to hint which library is expected and where.
Maybe to someone who knows more coding than I do, but I'll have to take your word for it again. The only thing I recognize in that excerpt is the filename, and that's only because we're already talking about it. But if you think it's useful to leave in, sure.
- I prefer to have a launcher just for the DRY principle and the rule that code is more read than written and the addPort lines are shorter.
This is fine, I've come around on the launcher. Another benefit is just having a system-wide way to launch the app from outside of RetroPie, with the altered save dir and without having to manually enter from in the console.
I still would choose to leave the AM2R configuration as a "hidden Easter-egg" and not call it out with a comment.
The PR has unrelated commits now, can you rebase it with just the changes to this module?
Gemba
commented
11 months ago Some more adaptions to reach consent.
Exarkuniv
commented
10 months ago
- I even assume it may raise the same question (why is this there without being referred to before) for next contributors being not involved now.
so you know, you assume too much. other then a very few people. manly your self and s1eve-mcdichae1 most people dont look or care about the scripts. as long as they work.
yes there are going to be some people who do and will look, since i took this over in 2021 its been mainly myself, then s1eve-mcdichae1, and now you. just giving you some context.
if you enjoy trying to get everything perfect and inline with how RetroPie does it (they will never include most if not all of these scritps) go for it.
i just dont want you to waste your time for nothing.
thats all
s1eve-mcdichae1
commented
10 months ago Almost there! I have only a couple tiny little minor cosmetic/syntax related nit-picky thoughts:
I see mostly if X; then or for Y; do on a single line, with no space before the semicolon. I would remove the newline at 55-56 and the spaces at 58, and 65.
And since we're on this module already, I've been wondering for a while now:
Is the disconnect between gmloader <--> droidports taking it too far? I had modeled it after like prboom <--> doom and eduke32 <--> duke3d but those have the benefit of name-recognition. "Droidports" isn't a popular game people have heard of like those are, and there's not likely to be other "droidports" emulators that can just be a drop-in replacement for gmloader, the way one might switch between console emulators. Even I find myself looking first for a "gmloader" romdir and config dir that aren't there, and I'm the one who made it that way.
Do y'all think it would be better to drop the "droidports" name and call those dirs "gmloader" like the module name? (Would change rom dir in apk_dir= and config dir in the addPort commands.)
Also:
Have you, or are you able to test AM2R on RPi3? When I first became aware of this project, there were some extra steps (scroll down to "extra steps") required to make it run on RPi3. I am not sure if this was needed for all of gmloader or just for am2r specifically, or whether it's still needed at all two years later, and, since I don't have a RPi3, I am unable to test it myself.
Gemba
commented
10 months ago I see mostly
if X; thenorfor Y; doon a single line, with no space before the semicolon. I would remove the newline at 55-56 and the spaces at 58, and 65.
These escaped during the merge. Thanks, done.
Gemba
commented
10 months ago Have you, or are you able to test AM2R on RPi3?
I am not going to investigate into this. The rpi3 compliance is only checked for the two "shipped" APKs with this module.
Gemba
commented
10 months ago Do y'all think it would be better to drop the "droidports" name and call those dirs "gmloader" like the module name?
If it should be changed remember to maintain compability to existing installations (even if these are currently only a few). Updating the config folder is one, but what about any APKs the user added, their absolute path to the APK must also be updated. Additionally to these considerations: I woudn't change it as anyone dealing with APK should be able to make the link to (An)droid. Thus droidports, even if not exactly like the module id should be easy to be identified as being related to this module.
s1eve-mcdichae1
commented
10 months ago I am not going to investigate into this. The rpi3 compliance is only checked for the two "shipped" APKs with this module.
Fair enough.
Line 57 escaped again.
Exarkuniv
commented
10 months ago so is this good?
s1eve-mcdichae1
commented
10 months ago Yeah looks good to me.
libc++_shared.so[1],apktoolto the rescueTested both sample games without flaws on a Rpi3 and Rpi4 with current RetroPie-Setup (86f65883).
[1] https://developer.android.com/ndk/guides/cpp-support#libc