| Summary: | Removing all the static variables from android SDLActivity and accompanying JNI calls. | ||
|---|---|---|---|
| Product: | SDL | Reporter: | owen <gurenchan> |
| Component: | main | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | major | ||
| Priority: | P2 | CC: | philipp.wiesemann, sylvain.becker |
| Version: | HG 2.1 | ||
| Hardware: | All | ||
| OS: | Android (All) | ||
| Attachments: |
this is a zip of all the files that I've updated.
patch |
||
also, there's the JNI code for flipbuffers, I didn't remove it in this patch because I wanted to hear why it's still in there. From what I see it looks like dead code, on the android platform at least but I wasn't sure. Thank you for working on a patch. Here are some quick and general comments: The "mSingleton" field is now not "static" and therefore also not really a "Singleton" anymore. Maybe it can be replaced with "this" everywhere (e.g. "SDLActivity.this" from inner classes). The "final String" and "final int" fields were "static" because they are constants. Removing the "static" there only made the class smaller. I would have kept it but this is more a style and compatibility question. Hard-coding source line numbers in error messages is not useful because they will become outdated after unrelated changes in the file. :) "The "mSingleton" field is now not "static" and therefore also not really a "Singleton" anymore. Maybe it can be replaced with "this" everywhere (e.g. "SDLActivity.this" from inner classes)." Thanks for the tip, In the beginning I avoided renaming variables so that I could just make sure everything works, you are right that mSingleton could be replaced with this "The "final String" and "final int" fields were "static" because they are constants. Removing the "static" there only made the class smaller. I would have kept it but this is more a style and compatibility question." okay, I am trying to keep things as close to the way SDL does it while expanding it's features. You guys could change this if you like. "Hard-coding source line numbers in error messages is not useful because they will become outdated after unrelated changes in the file. :)" Those line numbers were already outdated by the time I finished, it was just a quick way to search and find a line of code, even if the line number was wrong. Those can be removed. I think removing the JNI calls from SDL_androidtouch.c is something that should really be considered. Instead of providing a zip file, can you provide a unified diff patch? Some of those files have already been modified and I don't want to lose any other changes. Thanks! Created attachment 2591 [details]
patch
Here's a (new) patch version for the "removing the JNI calls from SDL_androidtouch.c" side.
Also, "Android_JNI_GetActivityClass()" is removed since it's not used elsewhere in SDL2. Sylvain's patch is in: https://hg.libsdl.org/SDL/rev/856e5e3c1e86 Are there any other changes that should go in? Seems ok. There was more in Owen's source but now this is really outdated and it decreased readability. Okay, thanks! |
Created attachment 2275 [details] this is a zip of all the files that I've updated. I removed all the static variables from SDLActivity.java Updated all the SDL_android.c jni calls as well I added a new function to SDL_android.c/ h void Android_JNI_SeparateEventsHint(const char* c); This is called by SDL_androidtouch.c so that this TU doesn't need to call any JNI functions. Also on SDLActivity.java I added some depreciation annotations because the lint warnings were just annoying to look at. You can diff these files with what's there now to see the major changes.