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

Android: Update the gradle file to use android studio 2.2 cmake. #4252

Merged
merged 1 commit into from Oct 6, 2016

Conversation

degasus
Copy link
Member

@degasus degasus commented Sep 27, 2016

This change is Reviewable

@degasus
Copy link
Member Author

degasus commented Sep 27, 2016

@sigmabeta May you review this PR please?

@sigmabeta
Copy link
Contributor

It looks good to me. I'm gonna take a quick minute to attempt a build before I pass the review, though.


Review status: 0 of 12 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


Source/Android/app/build.gradle, line 78 at r1 (raw file):

    // of product flavors are paid vs. free, ARM vs. x86, etc.
    productFlavors {
        dolphin {

If we're not actually differentiating flavors anymore, it's a good idea to do away with the entire productFlavors block.


Comments from Reviewable

@sigmabeta
Copy link
Contributor

Review status: 0 of 12 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


Source/Android/app/build.gradle, line 68 at r1 (raw file):

        externalNativeBuild {
            cmake {
                //arguments "-DANDROID_TOOLCHAIN=gcc"

I'd either get rid of this comment or explain why it might be necessary.


Comments from Reviewable

@sigmabeta
Copy link
Contributor

LGTM - build works, a few tiny nitpicks but otherwise this is all stuff that needs to be done.


Review status: 0 of 12 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


Comments from Reviewable

@sigmabeta
Copy link
Contributor

Reviewed 12 of 12 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


Comments from Reviewable

@shuffle2
Copy link
Contributor

shuffle2 commented Oct 3, 2016

is this uh, supposed to fail on the android buildslave?

@degasus
Copy link
Member Author

degasus commented Oct 3, 2016

@shuffle2 No, it's not. It needs manual interaction on the android buildbot. I'm waiting for the end of @delroth vacation.

@degasus
Copy link
Member Author

degasus commented Oct 5, 2016

This must be merged together with dolphin-emu/sadm#63


Review status: 6 of 14 files reviewed at latest revision, 2 unresolved discussions.


Source/Android/app/build.gradle, line 68 at r1 (raw file):

Previously, sigmabeta (Eder) wrote…

I'd either get rid of this comment or explain why it might be necessary.

Done.

Source/Android/app/build.gradle, line 78 at r1 (raw file):

Previously, sigmabeta (Eder) wrote…

If we're not actually differentiating flavors anymore, it's a good idea to do away with the entire productFlavors block.

Done.

Comments from Reviewable

@RisingFog
Copy link
Member

Builds fine with Android Studio 2.2 on Windows (using assembleDebug).

@dolphin-emu-bot
Copy link
Contributor

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

  • rs3-bumpmapping on dx-win-nv: diff

automated-fifoci-reporter

@degasus degasus merged commit f028b70 into dolphin-emu:master Oct 6, 2016
@degasus
Copy link
Member Author

degasus commented Oct 8, 2016

@SeannyM @sigmabeta - This PR breaks the gamelist on mobile devices. Android TV is still fine. But I have no clue why. As you're by far more in the android UI, do you have an idea how this could broke?

@JosJuice
Copy link
Member

JosJuice commented Oct 8, 2016

To clarify, the non-TV game list now always acts as if there are no games.

@sigmabeta
Copy link
Contributor

I have some time today and I can investigate why this is happening.

@degasus
Copy link
Member Author

degasus commented Oct 8, 2016

Thanks

@sigmabeta
Copy link
Contributor

PR with the fix open here: #4325

(worth noting, on my phone I can't launch any games, a JNI method error occurs and I'm not quite good enough to figure that one out instantly.)

@Hydr8gon
Copy link
Member

Hydr8gon commented Oct 8, 2016

Something I noticed, the buildbot builds all have 5.0-995 as their build number since this was merged. I haven't compiled myself yet (just got home) but I'm assuming this is a buildbot problem.

@JosJuice
Copy link
Member

JosJuice commented Oct 8, 2016

Good catch. I've been having the same problem when compiling locally (except it was claiming to be another version than 5.0-995).

@degasus degasus deleted the android branch February 3, 2019 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
7 participants