NewsForumsCodeLogsGallerySpecsSearch

android - onBackPressed() not trapped

Monkey Forums/Monkey Bug Reports/android - onBackPressed() not trapped

Skn3(Posted 2013) [#1]
Hey,

I found a slight issue with onBackPressed not being trapped. Currently it is being handled where key states are watched and then a KEY_ESCAPE is faked into monkey world. A nice solution but monkey is not properly trapping all instances of back being pressed.

I am developing a chartboost module (ingame app advert lib) and the SDK requires that I manually close my advert if back is pressed when the advert is active. It doesn't open a separate view/activity but it does seem to override monkeys built in key monitoring. And so KEY_ESCAPE never gets trapped.

If I add this to the target Activity:
	@Override
	public void onBackPressed() {
		// --- handle back event pressed on android ---
		//we need to handle this as when cahrtboost is active, if back is pressed, monkey will exit the app
		if (ChartboostGlue::instance != null && ChartboostGlue::instance.cb.onBackPressed()) {
			//block the back event
			return;
		} else {
			//allow default back event
			super.onBackPressed();
		}
	}


Then everything works fine, but it would be good if monkey overloaded the onBackPressed() just to trap it further.


marksibly(Posted 2013) [#2]
So...what exactly do you want me to add, and where?

This doesn't even look like Java to me: 'ChartboostGlue::instance'

I'm surprised the CB view doesn't trap the back button itself - are you sure you're integrating it correctly? Tried 'setfocus' or 'activate' or something?


Skn3(Posted 2013) [#3]
Sorry that was poor bug posting on my behalf. I was half way through porting it from iOS to android.

https://help.chartboost.com/documentation/android
scroll down to step 5 to see official implementation.

Could monkey add to its back key trapping with something like this:


androidgame.monkey
class AndroidGame extends Activity{

That might interfere with monkeys existing key monitoring and cause a duplicate press events? It might require some fiddling around with your key monitoring method as well?


DGuy(Posted 2013) [#4]
On Android, I also did away with Monkeys mocking-up of a KEY_ESCAPE press/release and added a simple onBackPressed handler (it just calls' super.onBackPressed()).

My main reason was this:

When the Back button is pressed, the default/expected behavior is for the current activity to end. With Monkey, the only way to end the current activity is to generate an exception (using Error("")).

My concern was that during app review, the generation of an exception on the pressing of the Back button might be considered a "bug" and reason enough for rejection.

Food for thought ... :)


therevills(Posted 2013) [#5]
the default/expected behavior is for the current activity to end

Not quite... it is to pop an activity off the stack, once all the activities for the application are popped it then exits the application.

My concern was that during app review, the generation of an exception on the pressing of the Back button might be considered a "bug" and reason enough for rejection.

What app review? Google Play does not review apps.


marksibly(Posted 2013) [#6]
> When the Back button is pressed, the default/expected behavior is for the current activity to end.

Interesting discussion on this here:

http://stackoverflow.com/questions/2033914/quitting-an-application-is-that-frowned-upon/2034238#2034238

> Not quite... it is to pop an activity off the stack, once all the activities for the application are popped it then exits the application.

Hmm...does this mean Monkey's back button handling is not doing something it should be doing?


Samah(Posted 2013) [#7]
@marksibly: Hmm...does this mean Monkey's back button handling is not doing something it should be doing?

As far as I know, when you pop the final activity from the stack (Monkey only has one anyway), the application *should* be scheduled for cleanup.

The big difference to iOS is that on Android you can push (almost) any activity from an application onto any activity stack, which is how applications call each other but always preserve the "back button goes to the last thing I was doing" paradigm. This is why you can't guarantee that your application has been cleaned up, because some of its activities may exist on other stacks.


Skn3(Posted 2013) [#8]
My concern was that during app review, the generation of an exception on the pressing of the Back button might be considered a "bug" and reason enough for rejection.


Potentially I wonder if we could simulate a quit from MonkeyGame by calling onBackPressed() manually on the super of AndroidGame ?


DGuy(Posted 2013) [#9]
@therevills: What app review? Google Play does not review apps.

... but Amazon, Barnes & Noble and Samsung (who is particularly picky) do. :)


@marksibly: Hmm...does this mean Monkey's back button handling is not doing something it should be doing?

I think so ...

Maybe an Android specific CONFIG.MONKEY option to control what happens when "Back" is pressed: Mock up a KEY_ESCAPE press/release or call super.onBackPressed().

Also on Android instead of generating an exception on Escape(""), pop/end the activity in some other way (maybe call finish()?)


Skn3(Posted 2013) [#10]
It would be better if there was a delegate notification in app like OnWouldClose() which would allows us to return true or false. False means default behaviour, true means handled by user code.


marksibly(Posted 2013) [#11]
> My concern was that during app review, the generation of an exception on the pressing of the Back button might be considered a "bug" and reason enough for rejection.

Where did you hear this?

In the case of Error "", the exception is caught in any case so the OS never even knows it was thrown. It is really just used as a way to prevent code from continuing to execute on targets where the 'exit' mechanism is asynchronous (ie: app doesn't actually exit until later) - and I suspect Activity.finish (which is what Error "" should probably be using) does just this.

I've also had a look at the backPressed code and the flow of events seem to be: user hits back->View.onKeyDown->Activity.onBackPressed.

Monkey currently consumes 'back' in View.onKeyDown, but I think I'll take that out and do the 'fake' ESC key in Activity.onBackPressed instead - will that do the trick?

Also considering something like OnClose...


Skn3(Posted 2013) [#12]
An OnClose would be good. Any official monkey accesable hooks into the native api are good!


DGuy(Posted 2013) [#13]
Even though the exception generated by Error("") is caught, "Java exception:java.lang.Error:" still gets logged out (viewable with 'adb logcat').

Knowing log output is captured/viewed during app review (I've had log files mailed to me from both B&N and Samsung) and feeling of all the things that could occur on "Back" presses, generation of an exception should not be one of them, I decided to play it safe by skipping use of Error("") to exit Android apps and implement default handling of the onBackPressed event.


@marksibly: Monkey currently consumes 'back' in View.onKeyDown, but I think I'll take that out and do the 'fake' ESC key in Activity.onBackPressed instead - will that do the trick?

The issue for me was not the fake ESC key press, but rather not having a clean way to end the app.


@marksibly: Also considering something like OnClose...

I don't know ... this is a such an Android specific issue (i.e. the handling of Back presses) that extending the generic API to support it would be overkill. I mean what would the other targets do with an OnClose event? On most wouldn't it just duplicate OnSuspend?

I'm still in favor of an solution involving an android specific CONFIG.MONKEY option.


Skn3(Posted 2013) [#14]
How would you call custom user code from a config.monkey file though? I think it is good to have a distinction between closing and suspending because on different platforms they do mean different things.

On windows for example, OnSuspend could mean when the application loses focus and OnClose when exited.


DGuy(Posted 2013) [#15]
To back up a bit ...

I think the real question is who's at fault because the integration of the Chartboost SDK is preventing Monkey from receiving input: Monkey or Chartboost?

If the app was working fine before, I'd say the fault lies with, or is definitely closer to, the Chartboost integration and the way you've remedied it is the correct one: Crack open the Monkey source and add a little custom code to deal with this Chartboost related quirk.

Now, if this issue crops up again and again with other SDKs, then I'd start asking, "Ok, what is Monkey doing wrong?"

---
@Skn3: How would you call custom user code from a config.monkey file though?

The config.monkey option would be used to indicate how Monkey should handle Back presses.

For example:
#ANDROID_ON_BACK_PRESSED=0  ' Ignore the back press
#ANDROID_ON_BACK_PRESSED=1  ' call super.onBackPressed ("exit the app")
#ANDROID_ON_BACK_PRESSED=2  ' generate KEY_ESCAPE allowing the app to handle the press with custom code


@Skn3: I think it is good to have a distinction between closing and suspending because on different platforms they do mean different things.

I must admit, I do view it as short-coming of Monkey that, on the Desktop, no notification is given on app exit (at least not on the GLFW target on Mac OS X.)

I added the signaling of an OnSuspend event to the GLFW target, soon after the app learns it's about to close, which allows me to handle OnSuspend uniformly across iOS/Android & GLFW: Save the game state and don't make assumptions about when or if the app will resume.

I'm just wary of adding an event to Monkey that will never be received and/or always be a NO-OP on certain targets.


Skn3(Posted 2013) [#16]
Well it's true that chartboost could handle the integration better but there is no harm in implementing lonBackPressed properly It can perform the same monkey behaviour just in a way that provides more functionality. Of course every quirk shouldn't be patched on to monkey. The config option is a good idea but what if you need to do something conditionally?

E.g. I only want to exit if I'm not in my i game menu, otherwise ignore and let my game close its menu.


DGuy(Posted 2013) [#17]
@Skn3: but there is no harm in implementing lonBackPressed properly

I agree ... :)


@Skn3: The config option is a good idea but what if you need to do something conditionally?

I'm thinking KEY_ESCAPE+custom-code w/ Error("") used to exit the app.

Some native code would probably be needed if a custom activity is involved but not much.


marksibly(Posted 2013) [#18]
> Even though the exception generated by Error("") is caught, "Java exception:java.lang.Error:" still gets logged out (viewable with 'adb logcat').

It isn't here - if I add this to the top of OnUpdate in bouncyaliens...

		If KeyHit( KEY_ESCAPE ) 
			Print "Bye!"
			Error ""
		Endif


...I get a clean(ish) exit, complete with 'Bye!' message.

All the OnBlah callbacks are wrapped in a try/catch for catching exactly this sort of stuff, so the exceptions shouldn't be making it back to the OS - if they are, something is wrong. But there shouldn't be any problem with using exceptions anywhere you want - I mean, it's nice to catch them (and Monkey SHOULD be catching Error "" throws) although IMO only if you can do something useful with them, but they're just a fancy way to transfer program flow.

Are you getting the same exception logcat output with bouncyaliens? Have you customized the code somehow?

Could be your OS is configured to dump exceptions when they're thrown/caught I guess - how about trying a little test that opens a non-existent image or something? That should also throw/catch an exception internally.

> The config.monkey option would be used to indicate how Monkey should handle Back presses

I'm honestly not keen on adding a bunch of ON_BACK_PRESSED options when the same thing can be achieved using existing code, as it can, although things can certainly be cleaned up a bit here.

I'm also not a big fan of callback code that returns True/False to indicate done/not done - I can never remember what means what! eg:

Method OnBack:Bool()
   Return True   'OK, what does this mean then?
End


...I much prefer it when code has to proactivaely take action, eg:

Method OnBack:Void()
   Error ""      'Should be obvious what this does!
End


This second case is effectively what we have now, and it can handle all the ON_BACK_PRESSED cases listed above without any 'external' stuff.

I'm also thinking 'Finish' or equivalent as an alternative to Error "" might be nice now...

> I must admit, I do view it as short-coming of Monkey that, on the Desktop, no notification is given on app exit (at least not on the GLFW target on Mac OS X.)

Damn straight! Probably a good time to sort this out too.

A couple of general approaches we could take to deal with this stuff (on all targets):

1) Add OnBack/OnClose methods to App.

2) Translate window close events to KEY_ESCAPE too.

3) Add KEY_BACK (already exists I think) and KEY_CLOSE pseudo-keycodes, ditch KEY_ESCAPE for android back (over time).

My preference would actually be for 3 - it's the simplest and easiest to expand on, ie: we just add more KEY_CODES if necessary.

Using KEY_ESCAPE for android back button was probably a mistake - it just forces you to have to enclose code in more #If TARGET... code in case you're also building for a targets with a 'real' escape key. Ditto, dismissal of the virtual keyboard should probably have its own pseudo keycode.

Thoughts?


DGuy(Posted 2013) [#19]
(Downloaded a fresh copy of MonkeyPro66b and made sure it was the one being used ...)

Made the changes to bouncyaliens and ran it. On tapping "Back" this is logged out:
[...]
I/[Monkey]( 2473): Bye!
I/[Monkey]( 2473): Java exception:java.lang.Error: 
[...]

Renamed one of the required *.png files and run. Monkey displays "Monkey Runtime Error: Attempt to access null object" and this is logged out:
[...]
I/[Monkey](30393): Java exception:java.lang.NullPointerException
[...]

Tested on two different devices, running two different OS versions (2.2.x & 4.0.x)


(Re. Adding config.monkey options ...)
This second case is effectively what we have now, and it can handle all the ON_BACK_PRESSED cases listed above without any 'external' stuff.

Agreed. I think the only requirement is a means of exiting the app/ activity in a more "Android Best Practices" manner ... :)


(Re. App close notification ...)
A couple of general approaches we could take to deal with this stuff (on all targets):

I'm thinking a combination of 1 & 3:

Definitely add a KEY_BACK constant (and maybe KEY_MENU/KEY_SEARCH ones while you're at it :) ) to represent the Android back button: There are already input related constants defined to represent platform & device specific keys & buttons so adding ones for Android would be absolutely an appropriate thing to do.

As far as an OnClose event, even though I recommended against it earlier, on further thought I think it's the way to go:

It's an app/system level event and should be handled like all others so receiving it via the input queue would just seem ... odd.

I believe the only target for which OnClose would not make sense would be iOS (assuming the apps' been coded in the apple recommended manner and exit()/abort() are not being called.) All other targets have the real possibility or at least some notion of closing or exiting an app. So 5 out of 6 ain't bad! :)


marksibly(Posted 2013) [#20]
> (Downloaded a fresh copy of MonkeyPro66b and made sure it was the one being used ...)

Aha...I was using latest v67, sorry.

Still, it's a [Monkey] message so you can remove it if it's making you nervous - see bb_std_lang.print in gxtkAlert ctor in mojo.android.java. It probably shouldn't be there in the first place, but I still maintain we should be able to use/log exceptions however we want without fear of rejection!

> I believe the only target for which OnClose would not make sense would be iOS

But OnClose means a request to close, doesn't it? In this case, it would only apply to desktop targets, ie: glfw and desktop xna. You can't request a html5/flash/android/ios/win8 app to 'close'...

Unless you're implying we overload it for android back button too? I'm not really keen on that as 'close' and 'back' are probably different enough we'd want separate handlers for them, ie: given the choice between:

#If TARGET="glfw"
Method OnClose:Void()
   ...handle window close click...
End
#ElseIf TARGET="android"
Method OnClose:Void()
    ...handle back button touch...
End
#End


...and...

Method OnBack:Void()
End

Method OnClose:Void()
End


...I think I prefer the latter. Unless OnClose handling for glfw/android will always be the same - will it?

Perhaps what's needed is something to notify of generic user events that may or may not be supported on various targets, eg:

Method OnUserEvent:Void( event:Int )
   Select event
   Case USEREVENT_BACK
   Case USEREVENT_CLOSE
   End
End


...no, not as sexy as OnBack, OnClose etc, but perhaps better than adding a bunch of callbacks to every target that will be only used on some?

> (and maybe KEY_MENU/KEY_SEARCH ones while you're at it :) )

Will look into 'menu', but I can't see a 'search' icon anywhere while Monkey apps run...when would this be triggered?

As for close, think I might just add KEY_CLOSE for glfw/desktop xna for now...


Skn3(Posted 2013) [#21]
Nothing wrong with a bunch of half used callbacks, most people won't touch them much anyway and realistically most game projects only target a few.. Err targets anyway. There won't be that many of the callbacks that it would become bloated surely?

If it was between KEY_CLOSE and OnEvent(EVENT_CLOSE) I'd choose OnEvent! But that's just personal preference.


DGuy(Posted 2013) [#22]
I was thinking of the closing of the browser window (HTML5/Flash) and the "closing" of an activity (Android), where for iOS there is only suspend. Maybe a little too broad an interpretation on my part ...

----
I'm favoring:

For Close notification:
Method OnClose:Void()
End
'OnClose' is a generic, near-univerial, system level event and should be handled as other such events (OnCreate, OnResume, etc.)


For Back press notification:
If KeyHit( KEY_BACK ) ...
The Back button is an Android specific, input realted key/event and should be handled in the same manner as, say, MOUSE_MIDDLE or JOY_LEFT.


@marksilby: Will look into 'menu', but I can't see a 'search' icon anywhere while Monkey apps run...when would this be triggered?

On Kindles and newer NOOK devices there's an always-visible system bar (i.e. it's not possible to have a completelly full-screen app) and one of the buttons is Search. Also, I believe, many Android phones with have physical Home/Menu/Back/Search buttons.

But seeing how a "Search" button, unlike Home/Menu/Back, is optional, not having a KEY_SEARCH constant would be totally fine. :)