Fix Gdx.files being garbage collected in MultiplayerTurnCheckWorker (#6817)

* Add logging to MultiplayerTurnCheckWorker

* Fix NullPointerException in turn check worker: Gdx.files is null
This commit is contained in:
Timo T
2022-05-18 06:35:00 +02:00
committed by GitHub
parent 4986505363
commit 5353f3337c
5 changed files with 62 additions and 21 deletions

View File

@ -70,11 +70,11 @@ open class AndroidLauncher : AndroidApplication() {
}
override fun onPause() {
if (UncivGame.Companion.isCurrentInitialized()
if (UncivGame.isCurrentInitialized()
&& UncivGame.Current.isGameInfoInitialized()
&& UncivGame.Current.settings.multiplayerTurnCheckerEnabled
&& GameSaver.getSaves(true).any()) {
MultiplayerTurnCheckWorker.startTurnChecker(applicationContext, UncivGame.Current.gameInfo, UncivGame.Current.settings)
MultiplayerTurnCheckWorker.startTurnChecker(applicationContext, GameSaver, UncivGame.Current.gameInfo, UncivGame.Current.settings)
}
super.onPause()
}

View File

@ -4,15 +4,18 @@ import android.app.NotificationChannel
import android.app.NotificationManager
import android.app.PendingIntent
import android.content.Context
import android.content.ContextWrapper
import android.content.Intent
import android.graphics.Color
import android.net.Uri
import android.os.Build
import android.util.Log
import androidx.core.app.NotificationCompat
import androidx.core.app.NotificationCompat.DEFAULT_VIBRATE
import androidx.core.app.NotificationManagerCompat
import androidx.work.*
import com.badlogic.gdx.backends.android.AndroidApplication
import com.badlogic.gdx.backends.android.DefaultAndroidFiles
import com.unciv.logic.GameInfo
import com.unciv.logic.GameSaver
import com.unciv.logic.multiplayer.storage.FileStorageRateLimitReached
@ -32,6 +35,7 @@ class MultiplayerTurnCheckWorker(appContext: Context, workerParams: WorkerParame
companion object {
const val WORK_TAG = "UNCIV_MULTIPLAYER_TURN_CHECKER_WORKER"
const val LOG_TAG = "Unciv turn checker"
const val CLIPBOARD_EXTRA = "CLIPBOARD_STRING"
const val NOTIFICATION_ID_SERVICE = 1
const val NOTIFICATION_ID_INFO = 2
@ -143,6 +147,7 @@ class MultiplayerTurnCheckWorker(appContext: Context, workerParams: WorkerParame
}
fun notifyUserAboutTurn(applicationContext: Context, game: Pair<String, String>) {
Log.i(LOG_TAG, "notifyUserAboutTurn ${game.first}")
val intent = Intent(applicationContext, AndroidLauncher::class.java).apply {
action = Intent.ACTION_VIEW
data = Uri.parse("https://unciv.app/multiplayer?id=${game.second}")
@ -168,15 +173,16 @@ class MultiplayerTurnCheckWorker(appContext: Context, workerParams: WorkerParame
}
}
fun startTurnChecker(applicationContext: Context, currentGameInfo: GameInfo, settings: GameSettings) {
val gameFiles = GameSaver.getSaves(true)
fun startTurnChecker(applicationContext: Context, gameSaver: GameSaver, currentGameInfo: GameInfo, settings: GameSettings) {
Log.i(LOG_TAG, "startTurnChecker")
val gameFiles = gameSaver.getSaves(true)
val gameIds = Array(gameFiles.count()) {""}
val gameNames = Array(gameFiles.count()) {""}
var count = 0
for (gameFile in gameFiles) {
try {
val gamePreview = GameSaver.loadGamePreviewFromFile(gameFile)
val gamePreview = gameSaver.loadGamePreviewFromFile(gameFile)
if (gamePreview.turnNotification) {
gameIds[count] = gamePreview.gameId
gameNames[count] = gameFile.name()
@ -189,13 +195,16 @@ class MultiplayerTurnCheckWorker(appContext: Context, workerParams: WorkerParame
}
}
Log.d(LOG_TAG, "start gameNames: ${Arrays.toString(gameNames)}")
if (currentGameInfo.currentPlayerCiv.playerId == settings.userId) {
// May be useful to remind a player that he forgot to complete his turn.
val gameIndex = gameIds.indexOf(currentGameInfo.gameId)
// Of the turnNotification is OFF, this will be -1 since we never saved this game in the array
// Or possibly reading the preview file returned an exception
if (gameIndex!=-1)
if (gameIndex!=-1) {
notifyUserAboutTurn(applicationContext, Pair(gameNames[gameIndex], gameIds[gameIndex]))
}
} else {
val inputData = workDataOf(Pair(FAIL_COUNT, 0), Pair(GAME_ID, gameIds), Pair(GAME_NAME, gameNames),
Pair(USER_ID, settings.userId), Pair(CONFIGURED_DELAY, settings.multiplayerTurnCheckerDelayInMinutes),
@ -206,6 +215,7 @@ class MultiplayerTurnCheckWorker(appContext: Context, workerParams: WorkerParame
showPersistentNotification(applicationContext,
"", settings.multiplayerTurnCheckerDelayInMinutes.toString())
}
Log.d(LOG_TAG, "startTurnChecker enqueue")
// Initial check always happens after a minute, ignoring delay config. Better user experience this way.
enqueue(applicationContext, 1, inputData)
}
@ -235,7 +245,16 @@ class MultiplayerTurnCheckWorker(appContext: Context, workerParams: WorkerParame
}
}
private val gameSaver = GameSaver
init {
// We can't use Gdx.files since that is only initialized within a com.badlogic.gdx.backends.android.AndroidApplication.
// Worker instances may be stopped & recreated by the Android WorkManager, so no AndroidApplication and thus no Gdx.files available
val files = DefaultAndroidFiles(applicationContext.assets, ContextWrapper(applicationContext), false)
gameSaver.init(files, null)
}
override fun doWork(): Result = runBlocking {
Log.i(LOG_TAG, "doWork")
val showPersistNotific = inputData.getBoolean(PERSISTENT_NOTIFICATION_ENABLED, true)
val configuredDelay = inputData.getInt(CONFIGURED_DELAY, 5)
val fileStorage = inputData.getString(FILE_STORAGE)
@ -244,6 +263,7 @@ class MultiplayerTurnCheckWorker(appContext: Context, workerParams: WorkerParame
val gameIds = inputData.getStringArray(GAME_ID)!!
val gameNames = inputData.getStringArray(GAME_NAME)!!
var arrayIndex = 0
Log.d(LOG_TAG, "doWork gameNames: ${Arrays.toString(gameNames)}")
// We only want to notify the user or update persisted notification once but still want
// to download all games to update the files so we save the first one we find
var foundGame: Pair<String, String>? = null
@ -254,7 +274,9 @@ class MultiplayerTurnCheckWorker(appContext: Context, workerParams: WorkerParame
continue
try {
Log.d(LOG_TAG, "doWork download ${gameId}")
val gamePreview = OnlineMultiplayerGameSaver(fileStorage).tryDownloadGamePreview(gameId)
Log.d(LOG_TAG, "doWork download ${gameId} done")
val currentTurnPlayer = gamePreview.getCivilization(gamePreview.currentPlayer)
//Save game so MultiplayerScreen gets updated
@ -265,22 +287,26 @@ class MultiplayerTurnCheckWorker(appContext: Context, workerParams: WorkerParame
while saves are getting saved right here.
Lets hope it works with gamePreview as they are a lot smaller and faster to save
*/
GameSaver.saveGame(gamePreview, gameNames[arrayIndex])
Log.i(LOG_TAG, "doWork save gameName: ${gameNames[arrayIndex]}")
gameSaver.saveGame(gamePreview, gameNames[arrayIndex])
Log.i(LOG_TAG, "doWork save ${gameNames[arrayIndex]} done")
if (currentTurnPlayer.playerId == inputData.getString(USER_ID)!! && foundGame == null) {
foundGame = Pair(gameNames[arrayIndex], gameIds[arrayIndex])
}
arrayIndex++
} catch (ex: FileStorageRateLimitReached) {
Log.i(LOG_TAG, "doWork FileStorageRateLimitReached ${ex.message}")
// We just break here as configuredDelay is probably enough to wait for the rate limit anyway
break
} catch (ex: FileNotFoundException){
Log.i(LOG_TAG, "doWork FileNotFoundException ${ex.message}")
// FileNotFoundException is thrown by OnlineMultiplayer().tryDownloadGamePreview(gameId)
// and indicates that there is no game preview present for this game
// in the dropbox so we should not check for this game in the future anymore
val currentGamePreview = GameSaver.loadGamePreviewByName(gameNames[arrayIndex])
val currentGamePreview = gameSaver.loadGamePreviewByName(gameNames[arrayIndex])
currentGamePreview.turnNotification = false
GameSaver.saveGame(currentGamePreview, gameNames[arrayIndex])
gameSaver.saveGame(currentGamePreview, gameNames[arrayIndex])
}
}
@ -293,10 +319,12 @@ class MultiplayerTurnCheckWorker(appContext: Context, workerParams: WorkerParame
if (showPersistNotific) { updatePersistentNotification(inputData) }
// We have to reset the fail counter since no exception appeared
val inputDataFailReset = Data.Builder().putAll(inputData).putInt(FAIL_COUNT, 0).build()
Log.d(LOG_TAG, "doWork enqueue")
enqueue(applicationContext, configuredDelay, inputDataFailReset)
}
} catch (ex: Exception) {
Log.e(LOG_TAG, "doWork ${ex::class.simpleName}: ${ex.message}")
val failCount = inputData.getInt(FAIL_COUNT, 0)
if (failCount > 3) {
showErrorNotification(getStackTraceString(ex))

View File

@ -85,7 +85,7 @@ class UncivGame(parameters: UncivGameParameters) : Game() {
viewEntireMapForDebug = false
}
Current = this
GameSaver.customSaveLocationHelper = customSaveLocationHelper
GameSaver.init(Gdx.files, customSaveLocationHelper)
// If this takes too long players, especially with older phones, get ANR problems.
// Whatever needs graphics needs to be done on the main thread,

View File

@ -1,5 +1,6 @@
package com.unciv.logic
import com.badlogic.gdx.Files
import com.badlogic.gdx.Gdx
import com.badlogic.gdx.files.FileHandle
import com.unciv.UncivGame
@ -20,30 +21,41 @@ object GameSaver {
const val settingsFileName = "GameSettings.json"
var saveZipped = false
@Volatile
var customSaveLocationHelper: CustomSaveLocationHelper? = null
/**
* This is necessary because the Android turn check background worker does not hold any reference to the actual [com.badlogic.gdx.Application],
* which is normally responsible for keeping the [Gdx] static variables from being garbage collected.
*/
private lateinit var files: Files
private var customSaveLocationHelper: CustomSaveLocationHelper? = null
/** When set, we know we're on Android and can save to the app's personal external file directory
* See https://developer.android.com/training/data-storage/app-specific#external-access-files */
var externalFilesDirForAndroid = ""
/** Needs to be called before the class can be used */
fun init(files: Files, customSaveLocationHelper: CustomSaveLocationHelper?) {
this.files = files
this.customSaveLocationHelper = customSaveLocationHelper
}
//endregion
//region Helpers
private fun getSavefolder(multiplayer: Boolean = false) = if (multiplayer) multiplayerFilesFolder else saveFilesFolder
fun getSave(GameName: String, multiplayer: Boolean = false): FileHandle {
val localFile = Gdx.files.local("${getSavefolder(multiplayer)}/$GameName")
if (externalFilesDirForAndroid == "" || !Gdx.files.isExternalStorageAvailable) return localFile
val externalFile = Gdx.files.absolute(externalFilesDirForAndroid + "/${getSavefolder(multiplayer)}/$GameName")
val localFile = files.local("${getSavefolder(multiplayer)}/$GameName")
if (externalFilesDirForAndroid == "" || !files.isExternalStorageAvailable) return localFile
val externalFile = files.absolute(externalFilesDirForAndroid + "/${getSavefolder(multiplayer)}/$GameName")
if (localFile.exists() && !externalFile.exists()) return localFile
return externalFile
}
fun getSaves(multiplayer: Boolean = false): Sequence<FileHandle> {
val localSaves = Gdx.files.local(getSavefolder(multiplayer)).list().asSequence()
if (externalFilesDirForAndroid == "" || !Gdx.files.isExternalStorageAvailable) return localSaves
return localSaves + Gdx.files.absolute(externalFilesDirForAndroid + "/${getSavefolder(multiplayer)}").list().asSequence()
val localSaves = files.local(getSavefolder(multiplayer)).list().asSequence()
if (externalFilesDirForAndroid == "" || !files.isExternalStorageAvailable) return localSaves
return localSaves + files.absolute(externalFilesDirForAndroid + "/${getSavefolder(multiplayer)}").list().asSequence()
}
fun canLoadFromCustomSaveLocation() = customSaveLocationHelper != null
@ -162,7 +174,7 @@ object GameSaver {
private fun getGeneralSettingsFile(): FileHandle {
return if (UncivGame.Current.consoleMode) FileHandle(settingsFileName)
else Gdx.files.local(settingsFileName)
else files.local(settingsFileName)
}
fun getGeneralSettings(): GameSettings {
@ -218,7 +230,7 @@ object GameSaver {
// keep auto-saves for the last 10 turns for debugging purposes
val newAutosaveFilename =
saveFilesFolder + File.separator + autoSaveFileName + "-${gameInfo.currentPlayer}-${gameInfo.turns}"
getSave(autoSaveFileName).copyTo(Gdx.files.local(newAutosaveFilename))
getSave(autoSaveFileName).copyTo(files.local(newAutosaveFilename))
fun getAutosaves(): Sequence<FileHandle> {
return getSaves().filter { it.name().startsWith(autoSaveFileName) }

View File

@ -21,7 +21,7 @@ import com.badlogic.gdx.Gdx;
import com.badlogic.gdx.backends.headless.HeadlessApplication;
import com.badlogic.gdx.backends.headless.HeadlessApplicationConfiguration;
import com.badlogic.gdx.graphics.GL20;
import com.unciv.logic.GameSaver;
import org.junit.runner.notification.RunNotifier;
import org.junit.runners.BlockJUnit4ClassRunner;
import org.junit.runners.model.FrameworkMethod;
@ -46,6 +46,7 @@ public class GdxTestRunner extends BlockJUnit4ClassRunner implements Application
@Override
public void create() {
GameSaver.INSTANCE.init(Gdx.files, null);
}
@Override