Android restructure to allow rotation and game running in background #532

Merged
merged 253 commits into from May 17, 2017

Conversation

Projects
None yet
5 participants
@tom-pratt
Member

tom-pratt commented Jan 9, 2017

Most of the UI has been replaced, a MainActivity shows pre game menus. GameActivity shows loading and gameplay. Game objects are held in an Android service called GameService to allow game to run in background. The jsettlers.graphics.androidui module still contains some unused files and there is general tidying up to be done when more detail is implemented.

tom-pratt added some commits May 27, 2016

main menu detects ongoing game and allows yout o resume. incase activ…
…ities are alldestroyed but game service continues
register and unregister broadcast receiver in sync with the service b…
…inding as they are only relevant at the same time
Added quit cancelled and made quit buttons on game menu dialog and ma…
…in menu fragment respond to quit and quit cancelled broadcasts
@DerKrawallkeks

This comment has been minimized.

Show comment
Hide comment
@DerKrawallkeks

DerKrawallkeks Apr 25, 2017

Yes I've tested it on a Sony Z3 running Android 6.0.1.
They don't disappear. Trying to swype them away results only in scrolling the map. (As far as I know, swyping away softtouch buttons is not a normal android function)

I think it would be best if they'd disappear after 5 seconds, and could be pulled in for 5 seconds. On devices with softtouch buttons only, they can always be pulled in, so simply hiding them/not showing them should do the job.

Yes I've tested it on a Sony Z3 running Android 6.0.1.
They don't disappear. Trying to swype them away results only in scrolling the map. (As far as I know, swyping away softtouch buttons is not a normal android function)

I think it would be best if they'd disappear after 5 seconds, and could be pulled in for 5 seconds. On devices with softtouch buttons only, they can always be pulled in, so simply hiding them/not showing them should do the job.

@tom-pratt

This comment has been minimized.

Show comment
Hide comment
@tom-pratt

tom-pratt Apr 25, 2017

Member

I tried it with immersive mode with soft buttons and it seems to be working nicely for both the status bar and the soft buttons bar. Although I would suggest that immersive mode is only applied to the GameActivity and not to the MainActivity. It felt a bit weird in the menu screens as they are such standard Android UI at the moment and the back button is quite useful at this point. Is it easy to do it on a per activity basis? It might be nice to have an option on whether or not to use immersive mode in game, I find that on my phone with physical buttons and on my 10inch tablet, its nice to be able to see the translucent status bar. That option can always be added later though, immersive-on by default will be good until then.

Member

tom-pratt commented Apr 25, 2017

I tried it with immersive mode with soft buttons and it seems to be working nicely for both the status bar and the soft buttons bar. Although I would suggest that immersive mode is only applied to the GameActivity and not to the MainActivity. It felt a bit weird in the menu screens as they are such standard Android UI at the moment and the back button is quite useful at this point. Is it easy to do it on a per activity basis? It might be nice to have an option on whether or not to use immersive mode in game, I find that on my phone with physical buttons and on my 10inch tablet, its nice to be able to see the translucent status bar. That option can always be added later though, immersive-on by default will be good until then.

@tom-pratt

This comment has been minimized.

Show comment
Hide comment
@tom-pratt

tom-pratt Apr 25, 2017

Member

That Android Annotations library looks quite nice. I haven't used it but it looks like aspects of Dagger, ButterKnife and Retrofit rolled into one. I was quite tempted to use Dagger and Butterknife for the stuff I did so far but didn't want to add too many dependencies that others might not be familiar with!

I think I would prefer to use Butterknife personally. It's more specific to wiring up view related boilerplate which is probably at least 90% of what we would use AndroidAnnotations for in JSettlers. AndroidAnnotations does have some nice features that butterknife doesnt have like that @UIThread annotation and also the @receiver annotation for registering broadcast receivers. But realistically I don't think these scenarios occur often enough in the code for it to make that much of a difference.

Also the dependency injection looks quite limited to system services in AndroidAnnotations, unless theres some documentation I'm not seeing. I think if we wanted to use dependency injection the aim should be to be able to create complex object graphs such as the Presenters and all of their dependencies. Dagger would probably be the better tool for things like that.

Happy to let you decide though. AndroidAnnotations looks pretty good and easy to use to me.

Member

tom-pratt commented Apr 25, 2017

That Android Annotations library looks quite nice. I haven't used it but it looks like aspects of Dagger, ButterKnife and Retrofit rolled into one. I was quite tempted to use Dagger and Butterknife for the stuff I did so far but didn't want to add too many dependencies that others might not be familiar with!

I think I would prefer to use Butterknife personally. It's more specific to wiring up view related boilerplate which is probably at least 90% of what we would use AndroidAnnotations for in JSettlers. AndroidAnnotations does have some nice features that butterknife doesnt have like that @UIThread annotation and also the @receiver annotation for registering broadcast receivers. But realistically I don't think these scenarios occur often enough in the code for it to make that much of a difference.

Also the dependency injection looks quite limited to system services in AndroidAnnotations, unless theres some documentation I'm not seeing. I think if we wanted to use dependency injection the aim should be to be able to create complex object graphs such as the Presenters and all of their dependencies. Dagger would probably be the better tool for things like that.

Happy to let you decide though. AndroidAnnotations looks pretty good and easy to use to me.

@tom-pratt

This comment has been minimized.

Show comment
Hide comment
@tom-pratt

tom-pratt Apr 25, 2017

Member

Something I just thought of...

I don't think any of the files in jsettlers.graphics.androidui are used anymore. You can probably delete the whole package.

Member

tom-pratt commented Apr 25, 2017

Something I just thought of...

I don't think any of the files in jsettlers.graphics.androidui are used anymore. You can probably delete the whole package.

@tom-pratt

This comment has been minimized.

Show comment
Hide comment
@tom-pratt

tom-pratt Apr 25, 2017

Member

I can see the mini map code is in there but isnt currently being used. We can always grab it out of an older commit when it comes to implementing minimap if you do decide to delete that package now. Can't see anything else in there that will be relevant to the new android stuff.

Member

tom-pratt commented Apr 25, 2017

I can see the mini map code is in there but isnt currently being used. We can always grab it out of an older commit when it comes to implementing minimap if you do decide to delete that package now. Can't see anything else in there that will be relevant to the new android stuff.

@andreas-eberle

This comment has been minimized.

Show comment
Hide comment
@andreas-eberle

andreas-eberle Apr 27, 2017

Member

@tom-pratt: From what I know/saw in the documentation, Android Annotations should contain most of the functionalities of Butterknife and Dagger. So I would go with Android Annotations.

What did you mean with butterknife beeing "more specific"?

I'll remove the code that we don't need. Thanks for brining it up.

You're right with the immersive mode, we should only use it in the actual game to prevent hiding the soft buttons.

@DerKrawallkeks: Because @tom-pratt reported that his soft buttons dissapear, I think your phone might be configured to not let the soft buttons dissapear. Some ROMs do have such options; some have it hard-wired this way... For the moment we'll stick to the Android standard behavior of the immersive mode. We can introduce changes later on.

Member

andreas-eberle commented Apr 27, 2017

@tom-pratt: From what I know/saw in the documentation, Android Annotations should contain most of the functionalities of Butterknife and Dagger. So I would go with Android Annotations.

What did you mean with butterknife beeing "more specific"?

I'll remove the code that we don't need. Thanks for brining it up.

You're right with the immersive mode, we should only use it in the actual game to prevent hiding the soft buttons.

@DerKrawallkeks: Because @tom-pratt reported that his soft buttons dissapear, I think your phone might be configured to not let the soft buttons dissapear. Some ROMs do have such options; some have it hard-wired this way... For the moment we'll stick to the Android standard behavior of the immersive mode. We can introduce changes later on.

@tom-pratt

This comment has been minimized.

Show comment
Hide comment
@tom-pratt

tom-pratt Apr 27, 2017

Member

Just that I tend to prefer several libraries that do one thing each. Rather than one bigger library that does several quite different things. I found it a bit weird that AndroidAnnotations contains that rest api stuff which seems totally unrelated to the rest of its functionality and is not something that all apps will need to use.

I'm happy with AndroidAnnotations though. It looks like it has lots of users and lots of development keeping it up to date. Also it's not like a big framework where you have to inherit from their activity and fragment baseclasses (which i hate!). It looks more like a collection of helpful utilities so I think it's a good choice.

Member

tom-pratt commented Apr 27, 2017

Just that I tend to prefer several libraries that do one thing each. Rather than one bigger library that does several quite different things. I found it a bit weird that AndroidAnnotations contains that rest api stuff which seems totally unrelated to the rest of its functionality and is not something that all apps will need to use.

I'm happy with AndroidAnnotations though. It looks like it has lots of users and lots of development keeping it up to date. Also it's not like a big framework where you have to inherit from their activity and fragment baseclasses (which i hate!). It looks more like a collection of helpful utilities so I think it's a good choice.

@andreas-eberle

This comment has been minimized.

Show comment
Hide comment
@andreas-eberle

andreas-eberle Apr 30, 2017

Member

@tom-pratt: Can you join the Discord, so I can ask you some questions to better understand the code? https://discord.gg/6gXQ6

Member

andreas-eberle commented Apr 30, 2017

@tom-pratt: Can you join the Discord, so I can ask you some questions to better understand the code? https://discord.gg/6gXQ6

Start using AndroidAnnotations. Changed manifests to generated activi…
…ties and use AndroidAnnotations in SettingsActivity.
@tom-pratt

This comment has been minimized.

Show comment
Hide comment
@tom-pratt

tom-pratt May 2, 2017

Member

Yeah sure, can you send a new link. This one expired before i saw it

Member

tom-pratt commented May 2, 2017

Yeah sure, can you send a new link. This one expired before i saw it

@andreas-eberle

This comment has been minimized.

Show comment
Hide comment
Member

andreas-eberle commented May 2, 2017

@andreas-eberle

This comment has been minimized.

Show comment
Hide comment
@andreas-eberle

andreas-eberle May 17, 2017

Member

I think this is good to go into master now. @tom-pratt: Can you test everything again to see if I broke something with the whole AndroidAnnotations refactoring?

Member

andreas-eberle commented May 17, 2017

I think this is good to go into master now. @tom-pratt: Can you test everything again to see if I broke something with the whole AndroidAnnotations refactoring?

@tom-pratt

This comment has been minimized.

Show comment
Hide comment
@tom-pratt

tom-pratt May 17, 2017

Member

Just ran it on my phone and everything seems to be working as far as I can tell 👍

Member

tom-pratt commented May 17, 2017

Just ran it on my phone and everything seems to be working as far as I can tell 👍

@andreas-eberle

This comment has been minimized.

Show comment
Hide comment
@andreas-eberle

andreas-eberle May 17, 2017

Member

Great, so I'll merge it as soon as Travis is done! Thanks @tom-pratt for all your work! I'm looking forward to your next PRs ;)

Member

andreas-eberle commented May 17, 2017

Great, so I'll merge it as soon as Travis is done! Thanks @tom-pratt for all your work! I'm looking forward to your next PRs ;)

@andreas-eberle andreas-eberle merged commit 838730d into jsettlers:master May 17, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tom-pratt tom-pratt deleted the tom-pratt:android_restructure_and_gameservice branch May 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment