Fix OpenAL error Windows Events after application ends (#10551)

* Move exitProcess to a better place

* Fix OpenAL error Windows Events after application ends

* Extensive commenting

core/src/com/unciv/UncivGame.kt

@@ -51,7 +51,6 @@ import java.io.PrintWriter
 import java.util.EnumSet
 import java.util.UUID
 import kotlinx.coroutines.CancellationException
-import kotlin.system.exitProcess
 
 open class UncivGame(val isConsoleMode: Boolean = false) : Game(), PlatformSpecific {
 
@@ -412,7 +411,7 @@ open class UncivGame(val isConsoleMode: Boolean = false) : Game(), PlatformSpeci
         // On desktop this should only be this one and "DestroyJavaVM"
         logRunningThreads()
 
-        exitProcess(0)
+        // DO NOT `exitProcess(0)` - bypasses all Gdx and GLFW cleanup
     }
 
     private fun logRunningThreads() {

core/src/com/unciv/ui/components/AudioExceptionHelper.kt

@@ -1,10 +0,0 @@
-package com.unciv.ui.components
-
-import com.badlogic.gdx.audio.Music
-
-interface AudioExceptionHelper {
-    fun installHooks(
-        updateCallback: (()->Unit)?,
-        exceptionHandler: ((Throwable, Music)->Unit)?
-    )
-}

desktop/src/com/unciv/app/desktop/DesktopGame.kt

@@ -1,11 +1,11 @@
 package com.unciv.app.desktop
 
+import com.badlogic.gdx.Gdx
 import com.badlogic.gdx.backends.lwjgl3.Lwjgl3ApplicationConfiguration
 import com.unciv.UncivGame
 
 class DesktopGame(config: Lwjgl3ApplicationConfiguration) : UncivGame() {
 
-    private val audio = HardenGdxAudio()
     private var discordUpdater = DiscordUpdater()
     private val windowListener = UncivWindowListener()
 
@@ -34,7 +34,7 @@ class DesktopGame(config: Lwjgl3ApplicationConfiguration) : UncivGame() {
     }
 
     override fun installAudioHooks() {
-        audio.installHooks(
+        (Gdx.app as HardenGdxAudio).installHooks(
             musicController.getAudioLoopCallback(),
             musicController.getAudioExceptionHandler()
         )

desktop/src/com/unciv/app/desktop/DesktopLauncher.kt

@@ -1,6 +1,5 @@
 package com.unciv.app.desktop
 
-import com.badlogic.gdx.backends.lwjgl3.Lwjgl3Application
 import com.badlogic.gdx.backends.lwjgl3.Lwjgl3ApplicationConfiguration
 import com.badlogic.gdx.files.FileHandle
 import com.badlogic.gdx.graphics.glutils.HdpiMode
@@ -14,6 +13,7 @@ import com.unciv.ui.components.fonts.Fonts
 import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.utils.Display
 import com.unciv.utils.Log
+import kotlin.system.exitProcess
 
 internal object DesktopLauncher {
 
@@ -50,10 +50,6 @@ internal object DesktopLauncher {
         config.setHdpiMode(HdpiMode.Logical)
         config.setWindowSizeLimits(WindowState.minimumWidth, WindowState.minimumHeight, -1, -1)
 
-        // We don't need the initial Audio created in Lwjgl3Application, HardenGdxAudio will replace it anyway.
-        // Note that means config.setAudioConfig() would be ignored too, those would need to go into the HardenedGdxAudio constructor.
-        config.disableAudio(true)
-
         // LibGDX not yet configured, use regular java class
         val maximumWindowBounds = getMaximumWindowBounds()
 
@@ -76,7 +72,8 @@ internal object DesktopLauncher {
             UiElementDocsWriter().write()
         }
 
-        val game = DesktopGame(config)
+        // HardenGdxAudio extends Lwjgl3Application, and the Lwjgl3Application constructor runs as long as the game runs
-        Lwjgl3Application(game, config)
+        HardenGdxAudio(DesktopGame(config), config)
+        exitProcess(0)
     }
 }

desktop/src/com/unciv/app/desktop/HardenGdxAudio.kt

@@ -1,17 +1,16 @@
 package com.unciv.app.desktop
 
-import com.badlogic.gdx.Gdx
 import com.badlogic.gdx.audio.Music
+import com.badlogic.gdx.backends.lwjgl3.Lwjgl3Application
+import com.badlogic.gdx.backends.lwjgl3.Lwjgl3ApplicationConfiguration
 import com.badlogic.gdx.backends.lwjgl3.audio.OpenALLwjgl3Audio
 import com.badlogic.gdx.backends.lwjgl3.audio.OpenALMusic
-import com.badlogic.gdx.backends.lwjgl3.audio.mock.MockAudio
 import com.badlogic.gdx.utils.Array
-import com.unciv.ui.components.AudioExceptionHelper
+import com.unciv.ui.audio.MusicController
 
 /**
  *  Problem: Not all exceptions playing Music can be caught on the desktop platform using a try-catch around the play method.
- *  Unciv 3.17.13 to 4.0.5 all exacerbated the problem due to using Music from various threads - my current interpretation
+ *  Unciv 3.17.13 to 4.0.5 all exacerbated the problem due to using Music from various threads - and Gdx documents it isn't threadsafe.
- *  is that OpenALMusic isn't thread-safe on play, load, dispose or any other methods touching any AL10.al*Buffers* call.
  *  But even with that fixed, music streams can have codec failures _after_ the first buffer's worth of data, so the problem is only mitigated.
  *
  *  Sooner or later some Exception will be thrown from the code under `Lwjgl3Application.loop` -> `OpenALLwjgl3Audio.update` ->
@@ -22,10 +21,7 @@ import com.unciv.ui.components.AudioExceptionHelper
  * #
  *  ### Approach:
  *  - Subclass [OpenALLwjgl3Audio] overriding [update][OpenALLwjgl3Audio.update] with equivalent code catching any Exceptions and Errors
- *  - Replicate original update (a 2-liner) accessing underlying fields through reflection to avoid rewriting the _whole_ thing
+ *  - Get the framework to use the subclassed Audio by overriding Lwjgl3ApplicationBase.createAudio
- *  - Not super.update so failed music can be immediately stopped, disposed and removed
- *  - Replace the object running inside Gdx - Gdx.app.audio - through reflection
- *  - Replace the object Unciv talks to - overwriting Gdx.audio is surprisingly allowed
  *
  *  ### Some exceptions so far seen:
  *  * Cannot store to object array because "this.mode_param" is null
@@ -44,44 +40,47 @@ import com.unciv.ui.components.AudioExceptionHelper
  *  * javazoom.jl.decoder.BitstreamException: Bitstream errorcode 102
  *  * NullPointerException: Cannot invoke "javazoom.jl.decoder.Bitstream.closeFrame()" because "this.bitstream" is null
  */
-class HardenGdxAudio : AudioExceptionHelper {
+class HardenGdxAudio(
+    game: DesktopGame,
+    config: Lwjgl3ApplicationConfiguration
+) : Lwjgl3Application(game, config) {
+    private var updateCallback: (()->Unit)? = null
+    private var exceptionHandler: ((Throwable, Music)->Unit)? = null
 
-    override fun installHooks(
+    /** Hooks part 1
+     *
+     *  This installs our extended version of OpenALLwjgl3Audio into Gdx from within the Lwjgl3Application constructor
+     *  Afterwards, Music/Sound decoder crashes are handled silently - nothing, not even a console log entry.
+     */
+    override fun createAudio(config: Lwjgl3ApplicationConfiguration?) =
+        HardenedGdxAudio()
+
+    /** Hooks part 2
+     *
+     *  This installs callbacks into [MusicController] that
+     *  - allow handling the exceptions - see [MusicController.getAudioExceptionHandler].
+     *  - and also allow MusicController to use the loop as timing source instead of a [Timer][java.util.Timer] - see [MusicController.getAudioLoopCallback].
+     */
+    fun installHooks(
         updateCallback: (()->Unit)?,
         exceptionHandler: ((Throwable, Music)->Unit)?
     ) {
-        // Get already instantiated Audio implementation for cleanup
+        this.updateCallback = updateCallback
-        // (may be OpenALLwjgl3Audio or MockAudio at this point)
+        this.exceptionHandler = exceptionHandler
-        val badAudio = Gdx.audio
-
-        val noAudio = MockAudio()
-
-        Gdx.audio = noAudio  // It's a miracle this is allowed.
-        // If it breaks in a Gdx update, go reflection instead as done below for Gdx.app.audio:
-
-        // Access the reference stored in Gdx.app.audio via reflection
-        val appClass = Gdx.app::class.java
-        val audioField = appClass.declaredFields.firstOrNull { it.name == "audio" }
-        if (audioField != null) {
-            audioField.isAccessible = true
-            audioField.set(Gdx.app, noAudio)  // kill it for a microsecond safely
     }
 
-        // Clean up allocated resources
+    /**
-        (badAudio as? OpenALLwjgl3Audio)?.dispose()
+     *  Desktop implementation of the [Audio][com.badlogic.gdx.Audio] interface that
-
+     *  unlike its superclass catches exceptions and allows passing them into application code for handling.
-        // Create replacement
+     *
-        val newAudio = HardenedGdxAudio(updateCallback, exceptionHandler)
+     *  Notes:
-
+     *  - Uses reflection to avoid reimplementing the entire thing.
-        // Store in Gdx fields used throughout the app (Gdx.app.audio by Gdx internally, Gdx.audio by us)
+     *  - Accesses the super private field noDevice - once. Yes their constructor runs before ours.
-        audioField?.set(Gdx.app, newAudio)
+     *  - Accesses super.music and copies a reference - thus size and element access are "live".
-        Gdx.audio = newAudio
+     *  - An alternative might be to try-catch in an OpenALMusic wrapper, that would be reflection-free
-    }
+     *    but quite a few lines more (override OpenALLwjgl3Audio.newMusic).
-
+     */
-    class HardenedGdxAudio(
+    inner class HardenedGdxAudio : OpenALLwjgl3Audio() {
-        private val updateCallback: (()->Unit)?,
-        private val exceptionHandler: ((Throwable, Music)->Unit)?
-    ) : OpenALLwjgl3Audio() {
         private val noDevice: Boolean
         private val music: Array<OpenALMusic>
 
@@ -92,11 +91,27 @@ class HardenGdxAudio : AudioExceptionHelper {
             noDevice = noDeviceField.getBoolean(this)
             val musicField = myClass.superclass.declaredFields.first { it.name == "music" }
             musicField.isAccessible = true
-            @Suppress("UNCHECKED_CAST")
+            @Suppress("UNCHECKED_CAST") // See OpenALLwjgl3Audio line 86 - it's a Gdx Array of OpenALMusic alright.
             music = musicField.get(this) as Array<OpenALMusic>
         }
 
-        // This is just a kotlin translation of the original `update` with added try-catch and cleanup
+        /**
+         *  This is just a kotlin translation of the original [update][OpenALLwjgl3Audio.update] with added try-catch and cleanup
+         *
+         *  ## Important: This _must_ stay in sync if ever upstream Gdx changes that function.
+         *  This current version corresponds to this upstream source:
+         *  ```java
+         *  	public void update () {
+         * 		    if (noDevice) return;
+         * 		    for (int i = 0; i < music.size; i++)
+         * 			    music.items[i].update();
+         *      }
+         *  ```
+         *  Note you ***cannot*** use Studio's automatic java-to-kotlin translation: In this case, it goes awry.
+         *
+         *  "```for (i in 0 until music.size) music.items[i].update()```" is ***not*** equivalent!
+         *  It tests the end each loop iteration against the size at the moment the loop was entered, not the current size.
+         */
         override fun update() {
             if (noDevice) return
             var i = 0  // No for loop as the array might be changed