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: fix Gradle, CMake, asset copying; upgrade to AndroidX, fullscreen mode #10402

Merged
merged 2 commits into from Jan 2, 2020

Conversation

@NuclearFej
Copy link
Contributor

NuclearFej commented Dec 21, 2019

Android build now functions again. Compiled on Windows, tested on a Pixel 2 XL running Android 10.

Gradle now pulls from the main OpenRCT2 /bin/data folder to insert the assets into the APK. MainActivity now properly copies assets from within the APK. Dependencies have been updated and the support library has been upgraded to AndroidX. CMake libraries now link properly. Network (multiplayer), Discord, and HTTP_Twitch were disabled, in addition to the already-disabled HTTP, OpenGL, and Twitch, due to compile-time errors. Revision number incremented.

-Werror and -Wall were disabled (in Android only) for the time being due to compile-time errors caused by the disabled features.

Gradle now pulls from the main OpenRCT2 /bin/data folder to insert the assets into the APK. MainActivity now properly copies assets from within the APK. Dependencies have been updated and the support library has been upgraded to AndroidX. CMake libraries now link properly. Network (multiplayer), Discord, and HTTP_Twitch were disabled, in addition to the already-disabled HTTP, OpenGL, and Twitch, due to compile-time errors. Revision number incremented.
@Gymnasiast Gymnasiast requested a review from janisozaur Dec 21, 2019
@AaronVanGeffen

This comment has been minimized.

Copy link
Member

AaronVanGeffen commented Dec 21, 2019

Thanks for your work on this. It's nice to see the Android port getting some love.

android.applicationVariants.all { variant ->
variant.mergeAssets.doLast {
copy {
from '../../../data'
into "$variant.mergeAssets.outputDir/data"
}
download {
src 'https://github.com/OpenRCT2/openrct2-dependencies-android/releases/download/v0.8/g2.dat'
dest "$variant.mergeAssets.outputDir/data"
}
download {
src 'https://github.com/OpenRCT2/title-sequences/releases/download/v0.1.2c/title-sequences.zip'
dest new File(buildDir, 'title-sequence.zip')
}
copy {
from zipTree(new File(buildDir, 'title-sequence.zip'))
into "$variant.mergeAssets.outputDir/data/title"
}
download {
src 'https://github.com/OpenRCT2/objects/releases/download/v1.0.12/objects.zip'
dest new File(buildDir, 'objects.zip')
}
copy {
from zipTree(new File(buildDir, 'objects.zip'))
into "$variant.mergeAssets.outputDir/data/object"
}
}
Comment on lines -82 to -108

This comment has been minimized.

Copy link
@AaronVanGeffen

AaronVanGeffen Dec 21, 2019

Member

These are essential steps for generating (release) builds) in CI jobs. Why were these steps removed? It seems the code that replaces it does not do the same thing.

This comment has been minimized.

Copy link
@NuclearFej

NuclearFej Dec 21, 2019

Author Contributor

The download plugin does not work anymore. My best guess is that it doesn't support TLSv1.2 which GitHub now requires. These assets are all duplicated elsewhere in the build, so I just took them from there.

variant.mergeassets.outputDir no longer exists and I'm not sure why it was used in the first place - these files are copied out of the APK and into /sdcard/openrct2 and that's it, so I just placed the files into the standard assets folder which is included in all builds.

Fullscreen mode (sticky immersive) is enabled. The minimum SDK version was raised to 19 (4.4.2 KitKat, which is more than old enough) from 16 to enable this.

Additonally, deprecation compiler warnings were enabled.
@NuclearFej

This comment has been minimized.

Copy link
Contributor Author

NuclearFej commented Jan 2, 2020

One thing to note about fullscreen mode is that phones with rounded corners cut off bits of the UI elements in those positions. It ought to be fixed at some point but the effect is minor and the trade-off is well worth it for the extra screen real estate.

@NuclearFej NuclearFej changed the title Android: fix Gradle, CMake, asset copying; upgrade to AndroidX Android: fix Gradle, CMake, asset copying; upgrade to AndroidX, fullscreen mode Jan 2, 2020
@Gymnasiast Gymnasiast merged commit e87607f into OpenRCT2:develop Jan 2, 2020
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Gymnasiast

This comment has been minimized.

Copy link
Member

Gymnasiast commented Jan 2, 2020

Since @AaronVanGeffen appears to be busy and hasn't replied to the line note, I figured I'd merge it - after all, the Android port was in bad shape so there wasn't much that could go wrong here.

@Gymnasiast

This comment has been minimized.

Copy link
Member

Gymnasiast commented Jan 13, 2020

Unfortunately, the CI job still fails:

/usr/local/android-sdk/ndk-bundle/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/c++/v1/filesystem:1070: error: undefined reference to 'std::__ndk1::__fs::filesystem::path::__extension() const'

https://travis-ci.org/OpenRCT2/OpenRCT2/jobs/636595094?utm_medium=notification&utm_source=github_status

Would you be able to look into that?

@AaronVanGeffen

This comment has been minimized.

Copy link
Member

AaronVanGeffen commented Jan 13, 2020

Sounds like it may require explicit linking in a way similar to #10522

@NuclearFej

This comment has been minimized.

Copy link
Contributor Author

NuclearFej commented Jan 20, 2020

Sorry for taking a while to respond -

Which version of NDK is being used for the builds? It does NOT work on the latest version. I am using NDK 18.

@AaronVanGeffen

This comment has been minimized.

Copy link
Member

AaronVanGeffen commented Jan 20, 2020

Does this help?

- wget https://dl.google.com/android/repository/sdk-tools-linux-3859397.zip

OpenRCT2/.travis.yml

Lines 135 to 136 in f1dcf85

- 'echo y | "$ANDROID_HOME/tools/bin/sdkmanager" "platforms;android-25"'
- 'echo y | "$ANDROID_HOME/tools/bin/sdkmanager" "cmake;3.6.4111459"'

@NuclearFej

This comment has been minimized.

Copy link
Contributor Author

NuclearFej commented Jan 21, 2020

It's targeting API 28, not 25.

@AaronVanGeffen

This comment has been minimized.

Copy link
Member

AaronVanGeffen commented Jan 21, 2020

Then these paths will require updating. Could you PR?

@Invictaz

This comment has been minimized.

Copy link

Invictaz commented Feb 7, 2020

@NuclearFej Do you mean it will require Android 9? That's ruling a lot of devices out.

@janisozaur

This comment has been minimized.

Copy link
Member

janisozaur commented Feb 7, 2020

@Invictaz that also enables a whole lot more devices than no builds. We'd welcome a PR to use older target.

@Invictaz

This comment has been minimized.

Copy link

Invictaz commented Feb 7, 2020

After inspection it targets minver 4.4 so I hope the online inspection tool I used is correct.
Backporting stuff is an insanely huge workload for an Android build that is mostly neglected.

@Gymnasiast

This comment has been minimized.

Copy link
Member

Gymnasiast commented Feb 7, 2020

I took this screenshot on an Samsung SM-G110H running Android 4.4.2. As you can see, it works:

Screenshot_2020-02-07-23-24-38

@Invictaz

This comment has been minimized.

Copy link

Invictaz commented Feb 7, 2020

@Gymnasiast

Greatly appreciated

@NuclearFej

This comment has been minimized.

Copy link
Contributor Author

NuclearFej commented Feb 7, 2020

Good to see. Honestly, I wasn't worried about supporting devices that old, but if nothing requires raising the min SDK level then I wouldn't change it.

@Invictaz

This comment has been minimized.

Copy link

Invictaz commented Feb 7, 2020

Please don't. I come by piles of "older" Android devices wearing Android 4.
In essence, they are from 2014 and were never updated. 2014 isn't that old imo.

@Gymnasiast

This comment has been minimized.

Copy link
Member

Gymnasiast commented Feb 7, 2020

@NuclearFej Judging from the IDs, it looks like the Android version is compiled as debug. Could you verify if that's the case, and if so, change it to -O2 or something similar?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.