We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 3128 - Removing all the static variables from android SDLActivity and accompanying JNI calls.
Summary: Removing all the static variables from android SDLActivity and accompanying J...
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: main (show other bugs)
Version: HG 2.1
Hardware: All Android (All)
: P2 major
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-24 17:45 UTC by owen
Modified: 2017-08-12 22:29 UTC (History)
2 users (show)

See Also:

Attachments
this is a zip of all the files that I've updated. (29.80 KB, application/zip)
2015-09-24 17:45 UTC, owen 		Details
patch (3.10 KB, patch)
2016-10-24 07:39 UTC, Sylvain 		Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description owen 2015-09-24 17:45:53 UTC 
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.
Comment 1 owen 2015-09-24 18:03:34 UTC 
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.
Comment 2 Philipp Wiesemann 2015-09-24 20:43:00 UTC 
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. :)
Comment 3 owen 2015-09-25 07:47:46 UTC 
"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.
Comment 4 Sam Lantinga 2016-10-01 21:24:11 UTC 
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!
Comment 5 Sylvain 2016-10-24 07:39:53 UTC 
Created attachment 2591 [details]
patch

Here's a (new) patch version for the "removing the JNI calls from SDL_androidtouch.c" side.
Comment 6 Sylvain 2016-10-24 07:49:04 UTC 
Also, "Android_JNI_GetActivityClass()" is removed since it's not used elsewhere in SDL2.
Comment 7 Sam Lantinga 2017-08-12 19:25:58 UTC 
Sylvain's patch is in:
https://hg.libsdl.org/SDL/rev/856e5e3c1e86

Are there any other changes that should go in?
Comment 8 Sylvain 2017-08-12 21:02:20 UTC 
Seems ok.
There was more in Owen's source but now this is really outdated and it decreased readability.
Comment 9 Sam Lantinga 2017-08-12 22:29:01 UTC 
Okay, thanks!