code review

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.ContentResolver;
 import android.content.Context;
+import android.database.Cursor;
 import android.net.Uri;
-import android.content.ContentResolver;
 import android.provider.OpenableColumns;
-import android.database.Cursor;
 
 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;
     }
 }

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.

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<jstring>(), dstStr.object<jstring>());
-		};
-		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());

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 <QObject>
 #include <QScroller>
 
+#ifdef VCMI_ANDROID
+#include <QAndroidJniObject>
+#include <QtAndroid>
+#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<jstring>(), 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<jstring>(), dstStr.object<jstring>(), QtAndroid::androidContext().object());
+#else
+	QFile(src).copy(dst);
+#endif
+}
 }

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 <QString>
+
 class QObject;
 
 namespace Helper
 {
 void loadSettings();
 void enableScrollBySwiping(QObject * scrollTarget);
+QString getRealPath(QString path);
+void performNativeCopy(QString src, QString dst);
 }

launcher/mainwindow_moc.cpp

@@ -13,11 +13,6 @@
 
 #include <QDir>
 
-#ifdef VCMI_ANDROID
-#include <QAndroidJniObject>
-#include <QtAndroid>
-#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<jstring>()).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();

launcher/modManager/chroniclesextractor.cpp

@@ -15,11 +15,7 @@
 #include "../../lib/filesystem/CArchiveLoader.h"
 
 #include "../innoextract.h"
-
-#ifdef VCMI_ANDROID
-#include <QAndroidJniObject>
-#include <QtAndroid>
-#endif
+#include "../helper.h"
 
 ChroniclesExtractor::ChroniclesExtractor(QWidget *p, std::function<void(float percent)> 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<jstring>(), dstStr.object<jstring>());
-		};
-#else
-		QFile(f).copy(filepath);
-#endif
+
+		Helper::performNativeCopy(f, filepath);
 		QFile file(filepath);
 
 		logGlobal->info("Extracting offline installer");

launcher/modManager/cmodlistview_moc.cpp

@@ -17,11 +17,6 @@
 #include <QCryptographicHash>
 #include <QRegularExpression>
 
-#ifdef VCMI_ANDROID
-#include <QAndroidJniObject>
-#include <QtAndroid>
-#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<jstring>()).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))