-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use nativeSetenv() provided by SDL2 and cleanups #1690
Conversation
Ok, I took a look and it seems fine to me (also I have tested it for python2 and python3...and all goes well). Still I prefer that @Jonast or @inclement approve this changes (they made the latest sdl2 related changes in |
@@ -404,7 +404,7 @@ public static void stop_service() { | |||
} | |||
|
|||
|
|||
public static native void nativeSetEnv(String j_name, String j_value); | |||
public static native void nativeSetenv(String name, String value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this required given this is already defined in the parent class SDLActivity
? I'm not 100% brushed up on my java knowledge but this looks like it might be redundant to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh right nevermind this is in a non-SDL2 bootstrap, I missed that for a second. all fine 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very reasonable to me! I like that you renamed it for the other bootstraps as well so it's still unified. I can't see anything obviously wrong with it, so if this builds & runs fine then it looks good as far as I'm concerned 👍
Great, thanks, especially unifying the other bootstraps :) |
Since 16b07ed (bumping SDL2 version from 2.0.4 to 2.0.9),
pythonforandroid/recipes/sdl2/add_nativeSetEnv.patch
that providesnativeSetEnv()
is obsolete (in terms of patching location). Furthermore, we don't even need this patch now because SDL2-2.0.9 itself provides the same functionality callednativeSetenv()
(NOTnativeSetEnv()
, beware the character's case!!) as follows:This PR is to
nativeSetEnv()
withnativeSetenv()
of SDL2-2.0.9.Note: the occurrences of
nativeSetEnv()
inbootstraps/pygame/build/src/org/renpy/android/SDLSurfaceView.java
are intact since this is code is based on SDL1.2 which is now a legacy (and so is pygame...).