From 3cb647bb2d20b2c111f968d465770cfd2098c2f8 Mon Sep 17 00:00:00 2001 From: Laserlicht <13953785+Laserlicht@users.noreply.github.com> Date: Wed, 15 Jan 2025 20:30:48 +0100 Subject: [PATCH] code review --- .../main/java/eu/vcmi/vcmi/util/FileUtil.java | 29 +++++++--------- ...erfile => BuildAndroid-aarch64.dockerfile} | 0 docs/developers/Building_Android.md | 6 ++-- launcher/firstLaunch/firstlaunch_moc.cpp | 16 ++------- launcher/helper.cpp | 33 ++++++++++++++++++- launcher/helper.h | 6 +++- launcher/mainwindow_moc.cpp | 19 +---------- launcher/modManager/chroniclesextractor.cpp | 17 ++-------- launcher/modManager/cmodlistview_moc.cpp | 19 ++--------- 9 files changed, 60 insertions(+), 85 deletions(-) rename docker/{BuildAndroid.dockerfile => BuildAndroid-aarch64.dockerfile} (100%) diff --git a/android/vcmi-app/src/main/java/eu/vcmi/vcmi/util/FileUtil.java b/android/vcmi-app/src/main/java/eu/vcmi/vcmi/util/FileUtil.java index c37fbb16b..53031b3dc 100644 --- a/android/vcmi-app/src/main/java/eu/vcmi/vcmi/util/FileUtil.java +++ b/android/vcmi-app/src/main/java/eu/vcmi/vcmi/util/FileUtil.java @@ -1,25 +1,23 @@ package eu.vcmi.vcmi.util; import android.app.Activity; -import android.content.Context; -import android.net.Uri; import android.content.ContentResolver; -import android.provider.OpenableColumns; +import android.content.Context; import android.database.Cursor; +import android.net.Uri; +import android.provider.OpenableColumns; import androidx.annotation.Nullable; import androidx.documentfile.provider.DocumentFile; -import org.libsdl.app.SDL; - import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.List; import java.lang.Exception; +import java.util.List; import eu.vcmi.vcmi.Const; import eu.vcmi.vcmi.Storage; @@ -115,12 +113,11 @@ public class FileUtil } @SuppressWarnings(Const.JNI_METHOD_SUPPRESS) - private static void copyFileFromUri(String sourceFileUri, String destinationFile) + private static void copyFileFromUri(String sourceFileUri, String destinationFile, Context context) { try { - final Context ctx = SDL.getContext(); - final InputStream inputStream = new FileInputStream(ctx.getContentResolver().openFileDescriptor(Uri.parse(sourceFileUri), "r").getFileDescriptor()); + final InputStream inputStream = new FileInputStream(context.getContentResolver().openFileDescriptor(Uri.parse(sourceFileUri), "r").getFileDescriptor()); final OutputStream outputStream = new FileOutputStream(new File(destinationFile)); copyStream(inputStream, outputStream); @@ -132,14 +129,12 @@ public class FileUtil } @SuppressWarnings(Const.JNI_METHOD_SUPPRESS) - private static String getFilenameFromUri(String sourceFileUri) + private static String getFilenameFromUri(String sourceFileUri, Context context) { + String fileName = ""; try { - String fileName = ""; - - final Context ctx = SDL.getContext(); - ContentResolver contentResolver = ctx.getContentResolver(); + ContentResolver contentResolver = context.getContentResolver(); Cursor cursor = contentResolver.query(Uri.parse(sourceFileUri), null, null, null, null); if (cursor != null && cursor.moveToFirst()) { @@ -149,14 +144,12 @@ public class FileUtil } cursor.close(); } - - return fileName; } catch (Exception e) { Log.e("getFilenameFromUri failed: ", e); - - return ""; } + + return fileName; } } diff --git a/docker/BuildAndroid.dockerfile b/docker/BuildAndroid-aarch64.dockerfile similarity index 100% rename from docker/BuildAndroid.dockerfile rename to docker/BuildAndroid-aarch64.dockerfile diff --git a/docs/developers/Building_Android.md b/docs/developers/Building_Android.md index 2fb7a41c7..133423e97 100644 --- a/docs/developers/Building_Android.md +++ b/docs/developers/Building_Android.md @@ -71,14 +71,14 @@ You can also see a more detailed walkthrough on CMake configuration at [How to b ## Docker -For developing it's also possible to use docker to build an android APK. As requirement only a installed docker is needed. The container image contains all other prerequisites. +For developing it's also possible to use Docker to build android APK. The only requirement is to have Docker installed. The container image contains all the other prerequisites. To build using docker just open a terminal with `vcmi` as working directory. Build the image with (only needed once): -`docker build -f docker/BuildAndroid.dockerfile -t vcmi-android-build .` +`docker build -f docker/BuildAndroid-aarch64.dockerfile -t vcmi-android-build .` After building the image you can compile vcmi with: `docker run -it --rm -v $PWD/:/vcmi vcmi-android-build` -The current dockerfile is ARM64 only but can adjusted manually for ARM32. +The current dockerfile is aarch64 only but can adjusted manually for armv7. diff --git a/launcher/firstLaunch/firstlaunch_moc.cpp b/launcher/firstLaunch/firstlaunch_moc.cpp index 8ed22cdb6..d7992c656 100644 --- a/launcher/firstLaunch/firstlaunch_moc.cpp +++ b/launcher/firstLaunch/firstlaunch_moc.cpp @@ -369,19 +369,9 @@ void FirstLaunchView::extractGogData() QString tmpFileExe = tempDir.filePath("h3_gog.exe"); QString tmpFileBin = tempDir.filePath("h3_gog-1.bin"); -#ifdef VCMI_ANDROID - auto copy = [](QString src, QString dst) - { - auto srcStr = QAndroidJniObject::fromString(src); - auto dstStr = QAndroidJniObject::fromString(dst); - QAndroidJniObject::callStaticObjectMethod("eu/vcmi/vcmi/util/FileUtil", "copyFileFromUri", "(Ljava/lang/String;Ljava/lang/String;)V", srcStr.object(), dstStr.object()); - }; - copy(fileExe, tmpFileExe); - copy(fileBin, tmpFileBin); -#else - QFile(fileExe).copy(tmpFileExe); - QFile(fileBin).copy(tmpFileBin); -#endif + + Helper::performNativeCopy(fileExe, tmpFileExe); + Helper::performNativeCopy(fileBin, tmpFileBin); logGlobal->info("Installing exe '%s' ('%s')", tmpFileExe.toStdString(), fileExe.toStdString()); logGlobal->info("Installing bin '%s' ('%s')", tmpFileBin.toStdString(), fileBin.toStdString()); diff --git a/launcher/helper.cpp b/launcher/helper.cpp index 9b4cec55e..bc1deb57b 100644 --- a/launcher/helper.cpp +++ b/launcher/helper.cpp @@ -1,5 +1,5 @@ /* - * jsonutils.cpp, part of VCMI engine + * helper.cpp, part of VCMI engine * * Authors: listed in file AUTHORS in main folder * @@ -15,6 +15,11 @@ #include #include +#ifdef VCMI_ANDROID +#include +#include +#endif + #ifdef VCMI_MOBILE static QScrollerProperties generateScrollerProperties() { @@ -44,4 +49,30 @@ void enableScrollBySwiping(QObject * scrollTarget) scroller->setScrollerProperties(generateScrollerProperties()); #endif } + +QString getRealPath(QString path) +{ +#ifdef VCMI_ANDROID + if(path.contains("content://", Qt::CaseInsensitive)) + { + auto str = QAndroidJniObject::fromString(path); + return QAndroidJniObject::callStaticObjectMethod("eu/vcmi/vcmi/util/FileUtil", "getFilenameFromUri", "(Ljava/lang/String;Landroid/content/Context;)Ljava/lang/String;", str.object(), QtAndroid::androidContext().object()).toString(); + } + else + return path; +#else + return path; +#endif +} + +void performNativeCopy(QString src, QString dst) +{ +#ifdef VCMI_ANDROID + auto srcStr = QAndroidJniObject::fromString(src); + auto dstStr = QAndroidJniObject::fromString(dst); + QAndroidJniObject::callStaticObjectMethod("eu/vcmi/vcmi/util/FileUtil", "copyFileFromUri", "(Ljava/lang/String;Ljava/lang/String;Landroid/content/Context;)V", srcStr.object(), dstStr.object(), QtAndroid::androidContext().object()); +#else + QFile(src).copy(dst); +#endif +} } diff --git a/launcher/helper.h b/launcher/helper.h index 2549a4aa4..f33c36d84 100644 --- a/launcher/helper.h +++ b/launcher/helper.h @@ -1,5 +1,5 @@ /* - * jsonutils.h, part of VCMI engine + * helper.h, part of VCMI engine * * Authors: listed in file AUTHORS in main folder * @@ -9,10 +9,14 @@ */ #pragma once +#include + class QObject; namespace Helper { void loadSettings(); void enableScrollBySwiping(QObject * scrollTarget); +QString getRealPath(QString path); +void performNativeCopy(QString src, QString dst); } diff --git a/launcher/mainwindow_moc.cpp b/launcher/mainwindow_moc.cpp index a9dad4d76..caf8afef4 100644 --- a/launcher/mainwindow_moc.cpp +++ b/launcher/mainwindow_moc.cpp @@ -13,11 +13,6 @@ #include -#ifdef VCMI_ANDROID -#include -#include -#endif - #include "../lib/CConfigHandler.h" #include "../lib/VCMIDirs.h" #include "../lib/filesystem/Filesystem.h" @@ -269,19 +264,7 @@ void MainWindow::dropEvent(QDropEvent* event) void MainWindow::manualInstallFile(QString filePath) { -#ifdef VCMI_ANDROID - QString realFilePath{}; - if(filePath.contains("content://", Qt::CaseInsensitive)) - { - auto path = QAndroidJniObject::fromString(filePath); - realFilePath = QAndroidJniObject::callStaticObjectMethod("eu/vcmi/vcmi/util/FileUtil", "getFilenameFromUri", "(Ljava/lang/String;)Ljava/lang/String;", path.object()).toString(); - } - else - realFilePath = filePath; - -#else - QString realFilePath = filePath; -#endif + QString realFilePath = Helper::getRealPath(filePath); if(realFilePath.endsWith(".zip", Qt::CaseInsensitive) || realFilePath.endsWith(".exe", Qt::CaseInsensitive)) switchToModsTab(); diff --git a/launcher/modManager/chroniclesextractor.cpp b/launcher/modManager/chroniclesextractor.cpp index 9005e8c4a..44ae06bae 100644 --- a/launcher/modManager/chroniclesextractor.cpp +++ b/launcher/modManager/chroniclesextractor.cpp @@ -15,11 +15,7 @@ #include "../../lib/filesystem/CArchiveLoader.h" #include "../innoextract.h" - -#ifdef VCMI_ANDROID -#include -#include -#endif +#include "../helper.h" ChroniclesExtractor::ChroniclesExtractor(QWidget *p, std::function cb) : parent(p), cb(cb) @@ -243,15 +239,8 @@ void ChroniclesExtractor::installChronicles(QStringList exe) // so make a copy in directory to which vcmi always has full access and operate on it QString filepath = tempDir.filePath("chr.exe"); logGlobal->info("Copying offline installer from '%s' to '%s'", f.toStdString(), filepath.toStdString()); -#ifdef VCMI_ANDROID - { - auto srcStr = QAndroidJniObject::fromString(f); - auto dstStr = QAndroidJniObject::fromString(filepath); - QAndroidJniObject::callStaticObjectMethod("eu/vcmi/vcmi/util/FileUtil", "copyFileFromUri", "(Ljava/lang/String;Ljava/lang/String;)V", srcStr.object(), dstStr.object()); - }; -#else - QFile(f).copy(filepath); -#endif + + Helper::performNativeCopy(f, filepath); QFile file(filepath); logGlobal->info("Extracting offline installer"); diff --git a/launcher/modManager/cmodlistview_moc.cpp b/launcher/modManager/cmodlistview_moc.cpp index 1a1fb2241..72ac5411b 100644 --- a/launcher/modManager/cmodlistview_moc.cpp +++ b/launcher/modManager/cmodlistview_moc.cpp @@ -17,11 +17,6 @@ #include #include -#ifdef VCMI_ANDROID -#include -#include -#endif - #include "modstatemodel.h" #include "modstateitemmodel_moc.h" #include "modstatecontroller.h" @@ -744,18 +739,8 @@ void CModListView::installFiles(QStringList files) // TODO: some better way to separate zip's with mods and downloaded repository files for(QString filename : files) { -#ifdef VCMI_ANDROID - QString realFilename{}; - if(filename.contains("content://", Qt::CaseInsensitive)) - { - auto path = QAndroidJniObject::fromString(filename); - realFilename = QAndroidJniObject::callStaticObjectMethod("eu/vcmi/vcmi/util/FileUtil", "getFilenameFromUri", "(Ljava/lang/String;)Ljava/lang/String;", path.object()).toString(); - } - else - realFilename = filename; -#else - QString realFilename = filename; -#endif + QString realFilename = Helper::getRealPath(filename); + if(realFilename.endsWith(".zip", Qt::CaseInsensitive)) mods.push_back(filename); else if(realFilename.endsWith(".h3m", Qt::CaseInsensitive) || realFilename.endsWith(".h3c", Qt::CaseInsensitive) || realFilename.endsWith(".vmap", Qt::CaseInsensitive) || realFilename.endsWith(".vcmp", Qt::CaseInsensitive))