Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fragment based libgdx in Android back-end #1286

Merged
merged 5 commits into from
Feb 12, 2014
Merged

Conversation

bartolkaruza
Copy link
Contributor

Added some minor abstraction of the AndroidApplication class to operate through an interface, so that a fragment can implement the same Application interface as an activity. Also added an implementation for the Fragment superclass that clients will extend, in the AndroidFragmentApplication class.

Bartol Karuza added 3 commits February 4, 2014 11:52
Signed-off-by: Bartol Karuza <bartol.karuza@klm.com>
…ity base classes.

Signed-off-by: Bartol Karuza <bartol.karuza@klm.com>
Signed-off-by: Bartol Karuza <bartol.karuza@klm.com>
@jrenner
Copy link
Member

jrenner commented Feb 4, 2014

This looks great. the android support lib should probably be mavenized I think?

@bartolkaruza
Copy link
Contributor Author

There is no recent android support library in the public maven repositories, the latest I could find is r7 (which does not include parent fragments). Seems like a bit of a work-around, but I couldn't figure out a better way to include it. Any ideas?

@jrenner
Copy link
Member

jrenner commented Feb 4, 2014

Well if the PR is accepted @badlogic will know what what should be done with regards to maven

@MobiDevelop
Copy link
Member

Look OK to me, though I've not actually tested it. Perhaps AndroidApplicationBase would be a better name than AndroidGraphicsApplication. I can't say I have ever wanted to use libgdx as a Fragment, but this has come up before so I guess some folks want it.

Would be nice to not have to maintain the support library in the repo, but such is life.

The troublesome thing will be communicating to folks that they need to add the support library to their android project themselves. For gradle/maven the jar can be added automatically, but not everyone uses those.

@VinceAngel
Copy link
Contributor

Looks great too :-)
Using libgdx as a fragment would be very useful to manage GDX view in a more complex Android application 👍

@MobiDevelop
Copy link
Member

Yeah, I suppose it isn't all that unlike the LwjglCanvas bits in the lwjgl backend.

@jrenner
Copy link
Member

jrenner commented Feb 4, 2014

Fragments could also provide for some creative solutions for playing videos

Bartol Karuza added 2 commits February 4, 2014 16:26
@badlogic
Copy link
Member

Looks awesome. The only issue is the silly Maven dependency. Why they don't publish the support lib to central is beyond me. We need a Wiki page on the fragment support, can i ask OP to do that? I'll see what i can do about the support lib + Maven.

@badlogic
Copy link
Member

So, it appears that Google is just incapable of getting their build shit together. The support library will not be published to SonaType because "reasons". I'm not sure how to resolve this tbh. If we have the Fragment based activity in the gdx-backend-android.jar, without the user having their project setup to have the support lib in the classpath, compilation for Android will fail.

@bartolkaruza
Copy link
Contributor Author

I would be glad to write a page for the usage of the Fragment Android back-end. It is not clear to me where in the wiki this would fit though, can you give me a hint on this? Thanks.

@badlogic
Copy link
Member

You could extend this page https://github.com/libgdx/libgdx/wiki/Starter-classes-%26-configuration there's an Android section in there.

It appears i can compile apps without the support library. Still testing, but this looks good. So we could have it in the Android backend jar without messing up builds of people who don't use the support lib. Nice.

@badlogic badlogic merged commit 0abd6da into libgdx:master Feb 12, 2014
@badlogic
Copy link
Member

Merged your changes, fixed the conflicts and tested on 2.2/2.3. We now have fragments support! Thanks for your contribution, really appreciated.

People wanting to use libgdx via fragments need to manually add the support library. If you update the wiki page, please include instructions for that, preferably for Eclipse, Intellij and Maven/Gradle.

Thanks!

@andzdroid
Copy link
Contributor

People wanting to use libgdx via fragments need to manually add the support library.

I downloaded the nightly, and the support library is included in gdx-backend-android.jar.

It is causing conflicts because I already have the support library in my project.

@johnnyapol
Copy link
Member

@andzdroid

Right click on project.
Properties -> Java Build Path -> Order and export -> Uncheck Private Libraries

Could you try that? Thanks.

@MobiDevelop
Copy link
Member

It should not be packed in the backend, are you using maven/gradle or the normal jar distribution?

@andzdroid
Copy link
Contributor

@unkn0wn0ne I'm using android studio

@MobiDevelop I downloaded the nightly jar distribution, added it to my project's libs folder, and I have this in gradle dependencies: compile fileTree(dir: 'libs', include: '*.jar')

(I'm in the irc channel if it's easier to chat for you guys)

@Kangou
Copy link

Kangou commented Feb 14, 2014

compile 'com.android.support:support-v4:19.0.1'

is working if the "Android Support Repository" is installed with the Android SDK Manager.

@andzdroid
Copy link
Contributor

After switching to using gradle to manage libgdx, it seems to work fine.

@cypherdare
Copy link
Member

@unkn0wn0ne
That works, but cannot be done if using the v7 support library, because that would get removed as well. I'm using Eclipse.

@MobiDevelop
I downloaded the nightly two days ago, and all the v4 support files are inside android-backends.jar, preventing me from building.

@MobiDevelop
Copy link
Member

I just removed the support library from being bundled with the Android backend. Let me know if it presents any issues.

// erase touched state. this also sucks donkeyballs...
Arrays.fill(touched, false);

if (getActivity().isFinishing()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't agree with that : the activity manage one or more fragment so we need to listen to the fragment state.
I tried with isRemoving() or isDetached() but none is true at this point.
The better could be to implement the onDestroy() maybe.
I'm not at all good at Android stuff, but I'm sure here we have a bug to fix. For example when the Fragment is detached from the activity, the fragment onPause() calls the ApplicationListener to pause(), then the fragment onDestroy() but the ApplicationListener never dispose()... so when the fragment onCreate() is called again, the ApplicationListener create() is done without the dispose() before... and this can cause some bad things !

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think this would leak if a Fragment were removed or replaced without the activity being finished (which is not all that uncommon).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, thanks for the vigilance. I'll submit a new pull request with this fix. The onDestroyView() callback seems the most logical place for this clean-up, since it will not be guaranteed to be destroyed (onDestroy()) even after it is removed from the Activity. OnDestroyView() is the tear-down equivalent of onCreateView() in a fragment.

@bartolkaruza
Copy link
Contributor Author

@MobiDevelop I added a short snippet in the wiki page. @badlogic asked me if I could add something about the support library as well, but I'm unsure what if anything needs to be done to make that work with eclipse/maven builds. I only use Android Studio/Gradle. Do you think something needs to be added to the wiki about this still?

bartolkaruza referenced this pull request in davebaol/libgdx Apr 11, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants