diff --git a/core/src/com/unciv/UncivGame.kt b/core/src/com/unciv/UncivGame.kt index 6a7f0e7ff2..265bcbb458 100644 --- a/core/src/com/unciv/UncivGame.kt +++ b/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() { diff --git a/core/src/com/unciv/ui/components/AudioExceptionHelper.kt b/core/src/com/unciv/ui/components/AudioExceptionHelper.kt deleted file mode 100644 index 0833291a33..0000000000 --- a/core/src/com/unciv/ui/components/AudioExceptionHelper.kt +++ /dev/null @@ -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)? - ) -} diff --git a/desktop/src/com/unciv/app/desktop/DesktopGame.kt b/desktop/src/com/unciv/app/desktop/DesktopGame.kt index 8a88b2b75b..f6b764a6bc 100644 --- a/desktop/src/com/unciv/app/desktop/DesktopGame.kt +++ b/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() ) diff --git a/desktop/src/com/unciv/app/desktop/DesktopLauncher.kt b/desktop/src/com/unciv/app/desktop/DesktopLauncher.kt index bc11454f3e..8cee48dc88 100644 --- a/desktop/src/com/unciv/app/desktop/DesktopLauncher.kt +++ b/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) - Lwjgl3Application(game, config) + // HardenGdxAudio extends Lwjgl3Application, and the Lwjgl3Application constructor runs as long as the game runs + HardenGdxAudio(DesktopGame(config), config) + exitProcess(0) } } diff --git a/desktop/src/com/unciv/app/desktop/HardenGdxAudio.kt b/desktop/src/com/unciv/app/desktop/HardenGdxAudio.kt index b6011e1125..f4b7501cf2 100644 --- a/desktop/src/com/unciv/app/desktop/HardenGdxAudio.kt +++ b/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 - * is that OpenALMusic isn't thread-safe on play, load, dispose or any other methods touching any AL10.al*Buffers* call. + * 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. * 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 - * - 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 + * - Get the framework to use the subclassed Audio by overriding Lwjgl3ApplicationBase.createAudio * * ### 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 - // (may be OpenALLwjgl3Audio or MockAudio at this point) - 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() - - // Create replacement - val newAudio = HardenedGdxAudio(updateCallback, exceptionHandler) - - // Store in Gdx fields used throughout the app (Gdx.app.audio by Gdx internally, Gdx.audio by us) - audioField?.set(Gdx.app, newAudio) - Gdx.audio = newAudio + this.updateCallback = updateCallback + this.exceptionHandler = exceptionHandler } - class HardenedGdxAudio( - private val updateCallback: (()->Unit)?, - private val exceptionHandler: ((Throwable, Music)->Unit)? - ) : OpenALLwjgl3Audio() { + /** + * 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. + * + * Notes: + * - Uses reflection to avoid reimplementing the entire thing. + * - Accesses the super private field noDevice - once. Yes their constructor runs before ours. + * - Accesses super.music and copies a reference - thus size and element access are "live". + * - 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). + */ + inner class HardenedGdxAudio : OpenALLwjgl3Audio() { private val noDevice: Boolean private val music: Array @@ -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 } - // 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