hushaojie04 / libgdx

Automatically exported from code.google.com/p/libgdx
0 stars 0 forks source link

Backward compatibility fix #73

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
When building libgdx in android 2.2 SDK to support SD card installation as 
mentioned in http://developer.android.com/guide/appendix/install-location.html, 
libgdx breaks(crash) on devices running android 1.6 or lower due to using 
undefined API to check for multitouch: PackageManager.hasSystemFeature()

This patch fixes that by using reflection method mentioned in 
http://developer.android.com/resources/articles/backward-compatibility.html to 
bind the API method dynamically for checking multitouch. When API method is not 
found, we can safely assume that there's no multitouch anyways.

It's sad that I did not notice this until it was too late. My game did not run 
for all 1.6 devices and got bad 1 star rating and > 50% uninstall rate due to 
this. :-( Really hope those guys would come back and give our game another try 
before rating us crappy 1 star again. Ah well, what's done is done. At least 
the bug is spotted and fixed. ;-)

Btw, I actually am reusing my lockless queue input patch hence I actually 
reverted my changes to build this patch. So it's untested but it should work.

Original issue reported on code.google.com by liquidro...@gmail.com on 12 Dec 2010 at 3:38

Attachments:

GoogleCodeExporter commented 9 years ago
We actually bind different touch handlers by checking the SDK string so you 
have multitouch on > 2.0 and single touch on <= 1.6. However, the call to 
PackageManager.hasFeature() is called naked in the wild, a shitty bug indeed. 
It was unnoticed as we did not call this method in any of our tests. I added a 
new test that checks for all potentially problematic methods.

I'm sorry this didn't get caught before you released your game to the market. 
While we try to have as good code coverage as possible we're still not there in 
the tests. I highly suggest to always test your app on 1.5, 1.6, 2.0(.1), 2.1, 
2.2 and 2.3 emulators. 

I went through all the Android backend to seek out any other problematic 
passages that invoke APIs > 1.5 (by setting the build target to 1.5) and 
couldn't find any other bollocks invocation. Again, sorry this didn't get 
caught in time. One liners can get lost pretty easily...

Concerning the lockless input event queue: I'm currently working on a 
modification that should bring down the latency of input events a little. Note 
however that it will still have a worst case latency of 16ms (or whatever your 
game can achieve) since it runs on the rendering thread which is vsynched on 
Android (no workaround possible). This could be fixed by making your game logic 
run on a separate thread which brings its own problems with it (synch with 
rendering thread still has to happen at some point so you can't totally escape 
the vsynch).

Again, sorry. I'll commit your patch later this evening. 

Original comment by badlogicgames on 12 Dec 2010 at 4:03

GoogleCodeExporter commented 9 years ago

Original comment by badlogicgames on 12 Dec 2010 at 4:03

GoogleCodeExporter commented 9 years ago
Ok, on further inspection i wonder why in your version of libgdx that method is 
called on a 1.6 device. Here's the relevant code part that performs the SDK 
level check as well as calling the PackageManager (revision 1164, 
AndroidInput.java):

        handle = new Handler();
        this.app = activity;
        this.sleepTime = sleepTime;
        int sdkVersion = Integer.parseInt(android.os.Build.VERSION.SDK);
        if (sdkVersion >= 5)
            touchHandler = new AndroidMultiTouchHandler();
        else
            touchHandler = new AndroidSingleTouchHandler();
        hasMultitouch = touchHandler instanceof AndroidMultiTouchHandler
                && ((AndroidMultiTouchHandler)touchHandler)
                        .supportsMultitouch(activity);

The method AndroidMultiTouchHandler.supportsMultitouch() is only called when 
the touchHandler is an AndroidMultiTouchHandler. And that is only the case when 
the SDK level is >= 5. I tested this on a 1.5 device as well as on a 1.6 
emulator and there was no problem. We'd have caught that a lot earlier already 
as this is in the constructor of AndroidInput and is thus called every time you 
start up your libgdx app on an Android phone (and we test heavily on 1.5 for 
benchmarking). So, what did you change in your working copy of AndroidInput for 
AndroidMultiTouchHandler.supportsMultitouch() to be called?

Original comment by badlogicgames on 12 Dec 2010 at 6:25

GoogleCodeExporter commented 9 years ago
Ack. why was I on my company login?

Ok. I know it's supposed to work with the sdk check, but I believe 
android.os.Build.VERSION.SDK returned 8 in my case due to building with the 2.2 
SDK. I suspect that value is compiled in instead of accessed from the actual 
executing OS. Either way, it actually crashes when I tested on the 1.6 emulator 
with the exception saying hasSystemFeature method does not exist.

From now on, I'm resorting to testing all my final release on the emulator. 
Though it really sucks that emulator runs hell slow and is impossible to even 
properly test them except for trying to start them up. I can't even play word 
zen on the emulator! *_* Personally, I'm glad that libGDX actually have a 
desktop build to allow me the ease of testing the game logic and rendering 
before delving into the emulator which is impossible to develop on.

Original comment by lf3th...@gmail.com on 12 Dec 2010 at 7:36

GoogleCodeExporter commented 9 years ago
Oh, btw as mentioned. The reason why I'm using a 2.2 SDK to build while 
supporting 1.6 OS is due to the request of others to make our game installable 
on the SD card; which is a feature only available on the 2.2 SDK.

Original comment by lf3th...@gmail.com on 12 Dec 2010 at 7:39

GoogleCodeExporter commented 9 years ago
IF the VERSION.SDK would be compiled into the dex file that would be a major 
bug in the Android SDK. I'll test it out in a bit and report back here. Thanks 
for pointing that out. 

I can feel your pain having to test on the emulator :/ Imagine how fun it is to 
run all the libgdx tests manually on the different Android versions. No 
unit-testing possible...

Original comment by badlogicgames on 12 Dec 2010 at 7:43

GoogleCodeExporter commented 9 years ago
ok, i tested it with the gdx-tests-android project. I tested on a 1.6 emulator, 
a 2.0 emulator, a 2.2 emulator and on my Hero with 1.5. I used a build target 
of Android 1.6 as well as 2.2. I could not reproduce the issue :/

Works on my 2.0.1 Droid and 2.2.1 Nexus One as well. 

Puzzling.

Original comment by badlogicgames on 12 Dec 2010 at 9:25

GoogleCodeExporter commented 9 years ago
that's odd. I was certain it was crashing as all the reports I got from players 
who could not run the game were using 1.6 os. And my changes fixed it. :/

hmmm...

Original comment by lf3th...@gmail.com on 12 Dec 2010 at 9:33

GoogleCodeExporter commented 9 years ago
I will try to revert my changes and try it on the emulator again just to double 
confirm.

Original comment by lf3th...@gmail.com on 12 Dec 2010 at 10:03

GoogleCodeExporter commented 9 years ago
Ok, did a revert into vanila input and got the crash again with the 1.6 
emulator.

This is what it says:
12-17 07:40:11.402: ERROR/dalvikvm(213): Could not find method 
android.content.pm.PackageManager.hasSystemFeature, referenced from method 
com.badlogic.gdx.backends.android.s.<init>
12-17 07:40:11.402: WARN/dalvikvm(213): VFY: unable to resolve virtual method 
132: Landroid/content/pm/PackageManager;.hasSystemFeature (Ljava/lang/String;)Z
12-17 07:40:11.402: WARN/dalvikvm(213): VFY:  rejecting opcode 0x6e at 0x00a5
12-17 07:40:11.402: WARN/dalvikvm(213): VFY:  rejected 
Lcom/badlogic/gdx/backends/android/s;.<init> 
(Lcom/badlogic/gdx/backends/android/AndroidApplication;Landroid/view/View;)V
12-17 07:40:11.412: WARN/dalvikvm(213): Verifier rejected class 
Lcom/badlogic/gdx/backends/android/s;

Note: I'm using proguard so class names got mangled into 's' but from my 
mapping, 's' is AndroidInput.

Original comment by lf3th...@gmail.com on 17 Dec 2010 at 7:47

GoogleCodeExporter commented 9 years ago
Hm, can't reproduce without Proguard. Will try with Proguard enabled, that 
might be it. In any case, i'll simply apply your patch. If it's Proguard 
related i guess we can file a bug over at the Android tools bugtracker. 

Original comment by badlogicgames on 17 Dec 2010 at 9:19

GoogleCodeExporter commented 9 years ago
Oh. never thought it was a proguard problem. I guess that could be the case 
since proguard does do some "optimization" trick which might have modified the 
behavior of that code.

So the solution is to either use dynamic reflection or specifically tell 
proguard not to touch the input part of libGDX.

Original comment by lf3th...@gmail.com on 17 Dec 2010 at 10:07

GoogleCodeExporter commented 9 years ago
I revisited this now and it is indeed a problem with Proguard. Given that 
Proguard has a couple of other nice bugs i'm reluctant to apply your patch at 
this point. I'd suggest actually not using Proguard. I also implemented 
lockless event handling in the android backend. It's not enabled yet by 
default, i'll do that for the 0.9 release. 

Original comment by badlogicgames on 21 Jan 2011 at 9:27