Fix: Android pause/resume cycle not working (#11511)

* Fix Concurrency being zombified but still tasked to run stuff

* Do not tie Android logcat output to whether Gdx was built for debugging

* Simplify GameStartScreen

* Add wiki page on Debugging/Building for Android

android/build.gradle.kts

@@ -55,13 +55,17 @@ android {
     }
 
     buildTypes {
-        getByName("release") {
+        debug {
+            isDebuggable = true
+        }
+        release {
             // If you make this true you get a version of the game that just flat-out doesn't run
             isMinifyEnabled = false
             proguardFiles(getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro")
+            isDebuggable = false
         }
-
     }
+
     lint {
         disable += "MissingTranslation"   // see res/values/strings.xml
     }

android/src/com/unciv/app/AndroidLogBackend.kt

@@ -3,9 +3,9 @@ package com.unciv.app
 import android.app.Activity
 import android.app.ActivityManager
 import android.content.Context
+import android.content.pm.ApplicationInfo
 import android.os.Build
 import android.util.Log
-import com.badlogic.gdx.backends.android.BuildConfig
 import com.unciv.utils.LogBackend
 import com.unciv.utils.Tag
 
@@ -14,11 +14,31 @@ private const val TAG_MAX_LENGTH = 23
 /**
  *  Unciv's logger implementation for Android
  *
+ *  Will log depending on whether the build was a debug build, with a manual override.
+ *  - An APK built from Studio's "Build APK's" menu is still a debug build
+ *  - A store-installed APK should be a release build - UNTESTED
+ *  - The override can be set in Studio's Run Configuration: "General" Tab, "Launch Flags" field: `--ez debugLogging true` or `--ez debugLogging false`
+ *  - Setting the override without Studio - from the device itself or simple adb - is possible, it's an activity manager command line option.
+ *    Terminal or adb shell: `am start com.unciv.app/com.unciv.app.AndroidLauncher --ez debugLogging true` (Tested)
+ *
  *  * Note: Gets and keeps a reference to [AndroidLauncher] as [activity] only to get memory info for [CrashScreen][com.unciv.ui.crashhandling.CrashScreen].
  *
  *  @see com.unciv.utils.Log
  */
 class AndroidLogBackend(private val activity: Activity) : LogBackend {
+    private val isRelease: Boolean
+
+    init {
+        // BuildConfig.DEBUG is **NOT** helpful as it always imports com.badlogic.gdx.backends.android.BuildConfig
+
+        /** This is controlled by the buildTypes -> isDebuggable flag in android -> build.gradle.kts */
+        val isDebuggable = (activity.applicationContext.applicationInfo.flags and ApplicationInfo.FLAG_DEBUGGABLE) != 0
+
+        val debugLogging = activity.intent.extras?.getBoolean("debugLogging")
+
+        // Log.d(toAndroidTag(Tag("Log init")), "isDebuggable=$isDebuggable, debugLogging=$debugLogging")
+        this.isRelease = !(debugLogging ?: isDebuggable) // cache as isRelease() is called all the time
+    }
 
     override fun debug(tag: Tag, curThreadName: String, msg: String) {
         Log.d(toAndroidTag(tag), "[$curThreadName] $msg")
@@ -29,7 +49,7 @@ class AndroidLogBackend(private val activity: Activity) : LogBackend {
     }
 
     override fun isRelease(): Boolean {
-        return !BuildConfig.DEBUG
+        return isRelease
     }
 
     /**

core/src/com/unciv/UncivGame.kt

@@ -487,8 +487,7 @@ private class GameStartScreen : BaseScreen() {
     init {
         val logoImage = ImageGetter.getExternalImage("banner.png")
         logoImage.center(stage)
-        logoImage.setOrigin(Align.center)
-        logoImage.color = Color.WHITE.cpy().apply { a = 0f }
+        logoImage.color.a = 0f
         logoImage.addAction(Actions.alpha(1f, 0.3f))
         stage.addActor(logoImage)
     }

core/src/com/unciv/utils/Concurrency.kt

@@ -1,3 +1,5 @@
+@file:Suppress("SpellCheckingInspection")  // shut up about "threadpool"!
+
 package com.unciv.utils
 
 import com.badlogic.gdx.Gdx
@@ -35,7 +37,7 @@ object Concurrency {
     /**
      * See [kotlinx.coroutines.runBlocking]. Runs on a non-daemon thread pool by default.
      *
-     * @return null if an uncaught exception occured
+     * @return null if an uncaught exception occurred
      */
     fun <T> runBlocking(
         name: String? = null,
@@ -72,7 +74,7 @@ object Concurrency {
     ), block)
 
     /** Must only be called in [com.unciv.UncivGame.dispose] to not have any threads running that prevent JVM shutdown. */
-    fun stopThreadPools() = EXECUTORS.forEach(ExecutorService::shutdown)
+    fun stopThreadPools() = Dispatcher.stopThreadPools()
 }
 
 /** See [launch] */
@@ -91,6 +93,8 @@ fun CoroutineScope.launchCrashHandling(
     }
 }
 
+private fun addName(context: CoroutineContext, name: String?) = if (name != null) context + CoroutineName(name) else context
+
 /** See [launch]. Runs on a daemon thread pool. Use this for code that does not necessarily need to finish executing. */
 fun CoroutineScope.launchOnThreadPool(name: String? = null, block: suspend CoroutineScope.() -> Unit) = launchCrashHandling(
     Dispatcher.DAEMON, name, block)
@@ -111,10 +115,40 @@ suspend fun <T> withNonDaemonThreadPoolContext(block: suspend CoroutineScope.()
 suspend fun <T> withGLContext(block: suspend CoroutineScope.() -> T): T = withContext(Dispatcher.GL, block)
 
 
-/**
- * All dispatchers here bring the main game loop to a [com.unciv.CrashScreen] if an exception happens.
+/** Wraps one instance of [Dispatchers], ensuring they are alive when used.
+ *
+ *  [stopThreadPools] is the way to kill them for cleaner app shutdown/pause, but in the Android app lifecycle
+ *  we need to be able to recover from that without the actual OS process terminating.
  */
 object Dispatcher {
+    private var dispatchers = Dispatchers()
+    private fun ensureInitialized() {
+        if (dispatchers.isStopped()) dispatchers = Dispatchers()
+    }
+
+    val DAEMON: CoroutineDispatcher get() {
+        ensureInitialized()
+        return dispatchers.DAEMON
+    }
+    val NON_DAEMON: CoroutineDispatcher get() {
+        ensureInitialized()
+        return dispatchers.NON_DAEMON
+    }
+    val GL: CoroutineDispatcher get() {
+        ensureInitialized()
+        return dispatchers.GL
+    }
+
+    fun stopThreadPools() = dispatchers.stopThreadPools()
+}
+
+/**
+ * All dispatchers here bring the main game loop to a [com.unciv.ui.crashhandling.CrashScreen] if an exception happens.
+ */
+@Suppress("PrivatePropertyName", "PropertyName") // Inherited all-caps names from former dev
+private class Dispatchers {
+    private val EXECUTORS = mutableListOf<ExecutorService>()
+
     /** Runs coroutines on a daemon thread pool. */
     val DAEMON: CoroutineDispatcher = createThreadpoolDispatcher("threadpool-daemon-", isDaemon = true)
 
@@ -122,54 +156,61 @@ object Dispatcher {
     val NON_DAEMON: CoroutineDispatcher = createThreadpoolDispatcher("threadpool-nondaemon-", isDaemon = false)
 
     /** Runs coroutines on the GDX GL thread. */
-    val GL: CoroutineDispatcher = CrashHandlingDispatcher(GLDispatcher())
-}
+    val GL: CoroutineDispatcher = CrashHandlingDispatcher(GLDispatcher(DAEMON))
 
-private fun addName(context: CoroutineContext, name: String?) = if (name != null) context + CoroutineName(name) else context
-
-private val EXECUTORS = mutableListOf<ExecutorService>()
-
-private class GLDispatcher : CoroutineDispatcher(), LifecycleListener {
-    var isDisposed = false
-
-    init {
-        Gdx.app.addLifecycleListener(this)
-    }
-
-    override fun dispatch(context: CoroutineContext, block: Runnable) {
-        if (isDisposed) {
-            context.cancel(CancellationException("GDX GL thread is not handling runnables anymore"))
-            Dispatcher.DAEMON.dispatch(context, block) // dispatch contract states that block has to be invoked
-            return
+    fun stopThreadPools() {
+        val iterator = EXECUTORS.iterator()
+        while (iterator.hasNext()) {
+            val executor = iterator.next()
+            executor.shutdown()
+            iterator.remove()
         }
-        Gdx.app.postRunnable(block)
     }
 
-    override fun dispose() {
-        isDisposed = true
-    }
-    override fun pause() {}
-    override fun resume() {}
-}
+    fun isStopped() = EXECUTORS.isEmpty()
 
-private fun createThreadpoolDispatcher(threadPrefix: String, isDaemon: Boolean): CrashHandlingDispatcher {
-    val executor = Executors.newCachedThreadPool(object : ThreadFactory {
-        var n = 0
-        override fun newThread(r: Runnable): Thread {
-            val thread = Thread(r, "${threadPrefix}${n++}")
-            thread.isDaemon = isDaemon
-            return thread
+    private fun createThreadpoolDispatcher(threadPrefix: String, isDaemon: Boolean): CrashHandlingDispatcher {
+        val executor = Executors.newCachedThreadPool(object : ThreadFactory {
+            var n = 0
+            override fun newThread(r: Runnable): Thread {
+                val thread = Thread(r, "${threadPrefix}${n++}")
+                thread.isDaemon = isDaemon
+                return thread
+            }
+        })
+        EXECUTORS.add(executor)
+        return CrashHandlingDispatcher(executor.asCoroutineDispatcher())
+    }
+
+    private class CrashHandlingDispatcher(
+        private val decoratedDispatcher: CoroutineDispatcher
+    ) : CoroutineDispatcher() {
+
+        override fun dispatch(context: CoroutineContext, block: Runnable) {
+            decoratedDispatcher.dispatch(context, block::run.wrapCrashHandlingUnit())
         }
-    })
-    EXECUTORS.add(executor)
-    return CrashHandlingDispatcher(executor.asCoroutineDispatcher())
-}
+    }
 
-class CrashHandlingDispatcher(
-    private val decoratedDispatcher: CoroutineDispatcher
-) : CoroutineDispatcher() {
+    private class GLDispatcher(private val fallbackDispatcher: CoroutineDispatcher) : CoroutineDispatcher(), LifecycleListener {
+        var isDisposed = false
 
-    override fun dispatch(context: CoroutineContext, block: Runnable) {
-        decoratedDispatcher.dispatch(context, block::run.wrapCrashHandlingUnit())
+        init {
+            Gdx.app.addLifecycleListener(this)
+        }
+
+        override fun dispatch(context: CoroutineContext, block: Runnable) {
+            if (isDisposed) {
+                context.cancel(CancellationException("GDX GL thread is not handling runnables anymore"))
+                fallbackDispatcher.dispatch(context, block) // dispatch contract states that block has to be invoked
+                return
+            }
+            Gdx.app.postRunnable(block)
+        }
+
+        override fun dispose() {
+            isDisposed = true
+        }
+        override fun pause() {}
+        override fun resume() {}
     }
 }

docs/Developers/Building-Locally.md

@@ -82,6 +82,11 @@ After building, the output .JAR file should be in `/desktop/build/libs/Unciv.jar
 
 For actual development, you'll probably need to download Android Studio and build it yourself - see above :)
 
+## Debugging on Android
+
+Sometimes, checking things out on the desktop version is not enough and you need to debug Unciv running on an Android device.
+For an introduction, see [Testing android builds](Testing-Android-Builds.md).
+
 ## Next steps
 
 Congratulations! Unciv should now be running on your computer! Now we can start changing some code, and later we'll see how your changes make it into the main repository!

docs/Developers/Testing-Android-Builds.md

@@ -0,0 +1,70 @@
+# Building for and testing on Android
+
+This is a work in progress - feel free to contribute. Much of this information is not specific to Unciv and publicly available.
+
+## Run configuration
+
+-   In Android Studio, Run > Edit configurations (be sure the Gradle sync is finished successfully first).
+    -   Click "+" to add a new configuration
+    -   Choose "Android App"
+    -   Give the configuration a name, we recommend "Android"
+    -   Set module to `Unciv.android.main`
+    -   On the Miscellaneous tab, we recommend checking both logcat options
+    -   On the Debugger tab, we recommend checking `Automatically attach on Debug.waitForDebugger()`
+    -   That's it, the rest can be left as defaults.
+
+## Physical devices
+
+Debugging on physical devices is actually easiest.
+With Studio running, you will have adb running, and any newly connected device that generally allows debugging will ask to confirm your desktop's fingerprint (use an USB cable for this tutorial, IP is another matter).
+Once adb sees the device and your desktop is authorized from the device, it will be available and preselected on the device select-box to the right of your "android" run configuration and you can start debugging just like the desktop version.
+**Note** A debug session does not end after selecting Exit from Unciv's menus - swipe it out of the recents list to end the debug session. Hitting the stop button in Studio is less recommended. That's an Android feature.
+
+## Building an APK
+
+Android Studio has a menu entry "Build -> Build Bundle(s) / APK(s) -> Build APK(s)."
+This will build a ready-to-install APK, and when it is finished, pop a message that offers to show you the file in your local file manager.
+***Important*** such locally built APK's are debug-signed and not interchangeable with Unciv downloaded from stores. You cannot update one with the other or switch without uninstalling first - losing all data.
+
+## Virtual devices (AVD)
+
+(TODO)
+- Install Emulator
+- Intel HAXM: Deprecated by Intel but still recommended
+- Download system image
+    - Choice: Match host architecture, w/o Google, older is faster...?
+- Configure AVD
+- Debug on AVD
+
+## Unciv's log output
+
+Unciv's log system runs on top of the Android SDK one, and filters and tags the messages before passing them to the system.
+
+Like the desktop variant, it has a 'release' mode where all logging from Unciv code is dropped.
+A release is detected when the actual APK manifest says debuggable=false - all possibilities discussed here are debug builds in that sense.
+Running from Studio it does not matter which button you use - Run or Debug - both deploy a debug build, the difference is only whether it attaches the debugger right away.
+An APK built from Studio is also always a debug build.
+
+Therefore, logging is always enabled unless you run a store version.
+You can override this by providing an intent extra: In your Run configuration, on the "General" Tab, add in the "Launch Flags" field: `--ez debugLogging false`.
+The override can also be controlled without Studio using the activity manager:
+```
+adb shell am start com.unciv.app/com.unciv.app.AndroidLauncher --ez debugLogging true
+```
+(or `am start...` directly from a device terminal) will turn on logging for a release (store) build.
+
+The log system's filtering capabilities that work by providing `-D` options to the Java virtual machine cannot be controlled on Android as far as we know.
+(TODO - document those in the desktop/Studio wiki article)
+
+## Reading the logcat
+
+(TODO)
+- Studio
+    - If the logcat window is missing: View - Tool Windows - Logcat
+- Studio's filtering
+    - When you debug Unciv, a matching filter is pre-applied to the Logcat window, but the tool can actually show the entire system log, including those for other apps.
+    - Using `package:com.unciv.app tag:Unciv` as filter is useful to see only the output of Unciv's own logging system.
+- logcat apps on the device
+    - `com.pluscubed.matloglibre`? Outdated.
+- logcat apps need root or specific authorization
+    - `adb shell pm grant <logcat app's package id> android.permission.READ_LOGS`