Skip to content

Commit 6ecfe99

Browse files
tmgasttmgast
committed
Refactor codebase for cleanliness and fix downloads screen bugs
Code cleanliness: - Remove unnecessary comments (section headers, obvious descriptions) - Remove verbose/sensitive logging from InputDispatcher, HapticFeedback, UserPreferencesRepository, DownloadManager, RomMRepository - Extract magic numbers to named constants - Consolidate duplicate ViewModel methods into GameActionsDelegate Downloads screen fixes: - Fix cancel button mapping (now uses X/ContextMenu to match footer hint) - Fix UI hang when canceling by wrapping file deletion in Dispatchers.IO - Add DOWNLOAD_CANCEL sound type with dismiss_fail.ogg
1 parent 86b76e1 commit 6ecfe99

20 files changed

Lines changed: 203 additions & 332 deletions

‎app/src/main/kotlin/com/nendo/argosy/data/download/DownloadManager.kt‎

Lines changed: 41 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package com.nendo.argosy.data.download
22

33
import android.content.Context
44
import android.os.StatFs
5-
import android.util.Log
65
import com.nendo.argosy.data.local.dao.DownloadQueueDao
76
import com.nendo.argosy.data.local.dao.GameDao
87
import com.nendo.argosy.data.local.entity.DownloadQueueEntity
@@ -32,8 +31,10 @@ import java.time.Instant
3231
import javax.inject.Inject
3332
import javax.inject.Singleton
3433

35-
private const val TAG = "DownloadManager"
3634
private const val STORAGE_BUFFER_BYTES = 50 * 1024 * 1024L
35+
private const val DOWNLOAD_BUFFER_SIZE = 64 * 1024
36+
private const val UI_UPDATE_INTERVAL_MS = 500L
37+
private const val DB_UPDATE_INTERVAL_MS = 5000L
3738

3839
data class DownloadProgress(
3940
val id: Long = 0,
@@ -114,8 +115,6 @@ class DownloadManager @Inject constructor(
114115
return
115116
}
116117

117-
Log.d(TAG, "restoreQueueFromDatabase: restoring ${pending.size} downloads")
118-
119118
val restored = pending.map { it.toDownloadProgress() }
120119
_state.value = DownloadQueueState(
121120
queue = restored,
@@ -141,8 +140,7 @@ class DownloadManager @Inject constructor(
141140
val downloadDir = getDownloadDir()
142141
val stat = StatFs(downloadDir.absolutePath)
143142
stat.availableBytes
144-
} catch (e: Exception) {
145-
Log.e(TAG, "getAvailableStorageBytes: error", e)
143+
} catch (_: Exception) {
146144
0L
147145
}
148146
}
@@ -167,22 +165,11 @@ class DownloadManager @Inject constructor(
167165
expectedSizeBytes: Long = 0
168166
) {
169167
val currentState = _state.value
170-
if (currentState.activeDownloads.any { it.gameId == gameId }) {
171-
Log.d(TAG, "enqueueDownload: game $gameId already downloading, skipping")
172-
return
173-
}
174-
if (currentState.queue.any { it.gameId == gameId }) {
175-
Log.d(TAG, "enqueueDownload: game $gameId already queued, skipping")
176-
return
177-
}
168+
if (currentState.activeDownloads.any { it.gameId == gameId }) return
169+
if (currentState.queue.any { it.gameId == gameId }) return
178170

179171
val existing = downloadQueueDao.getByGameId(gameId)
180-
if (existing != null) {
181-
Log.d(TAG, "enqueueDownload: game $gameId exists in database, skipping")
182-
return
183-
}
184-
185-
Log.d(TAG, "enqueueDownload: $gameTitle ($platformSlug), expectedSize=$expectedSizeBytes")
172+
if (existing != null) return
186173

187174
val downloadDir = getDownloadDir()
188175
val platformDir = File(downloadDir, platformSlug)
@@ -246,7 +233,6 @@ class DownloadManager @Inject constructor(
246233
val requiredBytes = next.totalBytes - next.bytesDownloaded
247234

248235
if (!hasEnoughStorage(requiredBytes, availableStorage)) {
249-
Log.w(TAG, "processQueue: insufficient storage for ${next.gameTitle}")
250236
downloadQueueDao.updateState(next.id, DownloadState.WAITING_FOR_STORAGE.name)
251237
_state.value = _state.value.copy(
252238
queue = _state.value.queue.map {
@@ -257,7 +243,6 @@ class DownloadManager @Inject constructor(
257243
continue
258244
}
259245

260-
Log.d(TAG, "processQueue: starting ${next.gameTitle}")
261246
soundManager.play(SoundType.DOWNLOAD_START)
262247

263248
_state.value = _state.value.copy(
@@ -309,8 +294,6 @@ class DownloadManager @Inject constructor(
309294
private suspend fun downloadRom(progress: DownloadProgress): DownloadResult =
310295
withContext(Dispatchers.IO) {
311296
try {
312-
Log.d(TAG, "downloadRom: starting download for ${progress.fileName}")
313-
314297
val downloadDir = getDownloadDir()
315298
val platformDir = File(downloadDir, progress.platformSlug).also { it.mkdirs() }
316299
val tempFile = File(platformDir, "${progress.fileName}.tmp")
@@ -319,24 +302,18 @@ class DownloadManager @Inject constructor(
319302
val existingBytes = if (tempFile.exists()) tempFile.length() else 0L
320303
val rangeHeader = if (existingBytes > 0) "bytes=$existingBytes-" else null
321304

322-
Log.d(TAG, "downloadRom: existingBytes=$existingBytes, rangeHeader=$rangeHeader")
323-
324305
when (val result = romMRepository.downloadRom(progress.rommId, progress.fileName, rangeHeader)) {
325306
is RomMResult.Success -> {
326307
val response = result.data
327308
val body = response.body
328309
val contentType = body.contentType()?.toString() ?: ""
329310
val contentLength = body.contentLength()
330311

331-
Log.d(TAG, "downloadRom: contentType=$contentType, contentLength=$contentLength, partial=${response.isPartialContent}")
332-
333312
if (INVALID_CONTENT_TYPES.any { contentType.startsWith(it) }) {
334-
Log.e(TAG, "downloadRom: invalid content type: $contentType")
335313
return@withContext DownloadResult.Failure("Invalid file type: $contentType")
336314
}
337315

338316
if (!response.isPartialContent && existingBytes > 0) {
339-
Log.w(TAG, "downloadRom: server doesn't support Range, starting fresh")
340317
tempFile.delete()
341318
}
342319

@@ -348,7 +325,6 @@ class DownloadManager @Inject constructor(
348325
}
349326

350327
if (totalSize > 0 && totalSize < MIN_ROM_SIZE_BYTES) {
351-
Log.w(TAG, "downloadRom: file too small ($totalSize bytes)")
352328
return@withContext DownloadResult.Failure("File too small to be a ROM")
353329
}
354330

@@ -360,22 +336,8 @@ class DownloadManager @Inject constructor(
360336
val startOffset = if (response.isPartialContent) existingBytes else 0L
361337

362338
body.byteStream().use { input ->
363-
val outputStream = if (response.isPartialContent && tempFile.exists()) {
364-
RandomAccessFile(tempFile, "rw").apply {
365-
seek(tempFile.length())
366-
}.let { raf ->
367-
object : java.io.OutputStream() {
368-
override fun write(b: Int) = raf.write(b)
369-
override fun write(b: ByteArray, off: Int, len: Int) = raf.write(b, off, len)
370-
override fun close() = raf.close()
371-
}
372-
}
373-
} else {
374-
FileOutputStream(tempFile)
375-
}
376-
377-
outputStream.use { output ->
378-
val buffer = ByteArray(65536)
339+
createOutputStream(tempFile, response.isPartialContent).use { output ->
340+
val buffer = ByteArray(DOWNLOAD_BUFFER_SIZE)
379341
var bytesRead: Long = startOffset
380342
var lastUpdateTime = System.currentTimeMillis()
381343
var lastDbUpdateTime = System.currentTimeMillis()
@@ -389,8 +351,7 @@ class DownloadManager @Inject constructor(
389351
bytesRead += read
390352

391353
val now = System.currentTimeMillis()
392-
if (now - lastUpdateTime > 500) {
393-
Log.d(TAG, "downloadRom: progress $bytesRead / $totalSize")
354+
if (now - lastUpdateTime > UI_UPDATE_INTERVAL_MS) {
394355
updateProgress(
395356
progress.copy(
396357
bytesDownloaded = bytesRead,
@@ -400,7 +361,7 @@ class DownloadManager @Inject constructor(
400361
lastUpdateTime = now
401362
}
402363

403-
if (now - lastDbUpdateTime > 5000) {
364+
if (now - lastDbUpdateTime > DB_UPDATE_INTERVAL_MS) {
404365
downloadQueueDao.updateProgress(progress.id, bytesRead)
405366
lastDbUpdateTime = now
406367
}
@@ -414,15 +375,11 @@ class DownloadManager @Inject constructor(
414375
)
415376
downloadQueueDao.updateProgress(progress.id, bytesRead)
416377

417-
if (tempFile.renameTo(targetFile)) {
418-
Log.d(TAG, "downloadRom: renamed temp file to ${targetFile.absolutePath}")
419-
} else {
378+
if (!tempFile.renameTo(targetFile)) {
420379
tempFile.copyTo(targetFile, overwrite = true)
421380
tempFile.delete()
422-
Log.d(TAG, "downloadRom: copied temp file to ${targetFile.absolutePath}")
423381
}
424382

425-
Log.d(TAG, "downloadRom: download complete, updating database")
426383
gameDao.updateLocalPath(
427384
progress.gameId,
428385
targetFile.absolutePath,
@@ -434,19 +391,32 @@ class DownloadManager @Inject constructor(
434391
}
435392
}
436393
is RomMResult.Error -> {
437-
Log.e(TAG, "downloadRom: failed - ${result.message}")
438394
DownloadResult.Failure(result.message)
439395
}
440396
}
441-
} catch (e: CancellationException) {
442-
Log.d(TAG, "downloadRom: cancelled")
397+
} catch (_: CancellationException) {
443398
DownloadResult.Cancelled
444399
} catch (e: Exception) {
445-
Log.e(TAG, "downloadRom: exception", e)
446400
DownloadResult.Failure(e.message ?: "Unknown error")
447401
}
448402
}
449403

404+
private fun createOutputStream(tempFile: File, isResume: Boolean): java.io.OutputStream {
405+
return if (isResume && tempFile.exists()) {
406+
RandomAccessFile(tempFile, "rw").apply {
407+
seek(tempFile.length())
408+
}.let { raf ->
409+
object : java.io.OutputStream() {
410+
override fun write(b: Int) = raf.write(b)
411+
override fun write(b: ByteArray, off: Int, len: Int) = raf.write(b, off, len)
412+
override fun close() = raf.close()
413+
}
414+
}
415+
} else {
416+
FileOutputStream(tempFile)
417+
}
418+
}
419+
450420
private fun updateProgress(progress: DownloadProgress) {
451421
_state.value = _state.value.copy(
452422
activeDownloads = _state.value.activeDownloads.map {
@@ -458,7 +428,6 @@ class DownloadManager @Inject constructor(
458428
fun pauseDownload(rommId: Long) {
459429
val active = _state.value.activeDownloads.find { it.rommId == rommId }
460430
if (active != null) {
461-
Log.d(TAG, "pauseDownload: pausing active download for rommId=$rommId")
462431
downloadJobs[active.id]?.cancel()
463432
downloadJobs.remove(active.id)
464433

@@ -488,7 +457,6 @@ class DownloadManager @Inject constructor(
488457
it.gameId == gameId && (it.state == DownloadState.PAUSED || it.state == DownloadState.WAITING_FOR_STORAGE)
489458
}
490459
if (paused != null) {
491-
Log.d(TAG, "resumeDownload: resuming download for gameId=$gameId")
492460
scope.launch {
493461
downloadQueueDao.updateState(paused.id, DownloadState.QUEUED.name)
494462
}
@@ -513,12 +481,7 @@ class DownloadManager @Inject constructor(
513481
!reason.contains("HTTP 404", ignoreCase = true)
514482
}
515483

516-
if (retryable.isEmpty()) {
517-
Log.d(TAG, "retryFailedDownloads: ${failed.size} failed but none retryable (permanent errors)")
518-
return
519-
}
520-
521-
Log.d(TAG, "retryFailedDownloads: retrying ${retryable.size} of ${failed.size} failed downloads")
484+
if (retryable.isEmpty()) return
522485

523486
for (entity in retryable) {
524487
downloadQueueDao.updateState(entity.id, DownloadState.QUEUED.name, null)
@@ -535,13 +498,11 @@ class DownloadManager @Inject constructor(
535498
}
536499

537500
val availableStorage = getAvailableStorageBytes()
538-
Log.d(TAG, "recheckStorageAndResume: checking ${waiting.size} items, available=$availableStorage")
539501

540502
var anyResumed = false
541503
for (item in waiting) {
542504
val requiredBytes = item.totalBytes - item.bytesDownloaded
543505
if (hasEnoughStorage(requiredBytes, availableStorage)) {
544-
Log.d(TAG, "recheckStorageAndResume: resuming ${item.gameTitle}")
545506
downloadQueueDao.updateState(item.id, DownloadState.QUEUED.name)
546507
anyResumed = true
547508
}
@@ -555,15 +516,18 @@ class DownloadManager @Inject constructor(
555516
}
556517

557518
fun cancelDownload(rommId: Long) {
519+
soundManager.play(SoundType.DOWNLOAD_CANCEL)
558520
val active = _state.value.activeDownloads.find { it.rommId == rommId }
559521
if (active != null) {
560522
downloadJobs[active.id]?.cancel()
561523
downloadJobs.remove(active.id)
562524
scope.launch {
563525
downloadQueueDao.deleteById(active.id)
564-
val downloadDir = getDownloadDir()
565-
val tempFile = File(downloadDir, "${active.platformSlug}/${active.fileName}.tmp")
566-
if (tempFile.exists()) tempFile.delete()
526+
withContext(Dispatchers.IO) {
527+
val downloadDir = getDownloadDir()
528+
val tempFile = File(downloadDir, "${active.platformSlug}/${active.fileName}.tmp")
529+
if (tempFile.exists()) tempFile.delete()
530+
}
567531
}
568532
_state.value = _state.value.copy(
569533
activeDownloads = _state.value.activeDownloads.filter { it.id != active.id }
@@ -574,9 +538,11 @@ class DownloadManager @Inject constructor(
574538
if (queued != null) {
575539
scope.launch {
576540
downloadQueueDao.deleteById(queued.id)
577-
val downloadDir = getDownloadDir()
578-
val tempFile = File(downloadDir, "${queued.platformSlug}/${queued.fileName}.tmp")
579-
if (tempFile.exists()) tempFile.delete()
541+
withContext(Dispatchers.IO) {
542+
val downloadDir = getDownloadDir()
543+
val tempFile = File(downloadDir, "${queued.platformSlug}/${queued.fileName}.tmp")
544+
if (tempFile.exists()) tempFile.delete()
545+
}
580546
}
581547
_state.value = _state.value.copy(
582548
queue = _state.value.queue.filter { it.rommId != rommId }

‎app/src/main/kotlin/com/nendo/argosy/data/emulator/EmulatorDetector.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,7 @@ class EmulatorDetector @Inject constructor(
4646
versionCode = versionCode
4747
)
4848
)
49-
} catch (e: PackageManager.NameNotFoundException) {
50-
// Emulator not installed
49+
} catch (_: PackageManager.NameNotFoundException) {
5150
}
5251
}
5352

‎app/src/main/kotlin/com/nendo/argosy/data/emulator/EmulatorRegistry.kt‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ enum class LaunchType {
5454
object EmulatorRegistry {
5555

5656
private val emulators = listOf(
57-
// RetroArch - Universal
5857
EmulatorDef(
5958
id = "retroarch",
6059
packageName = "com.retroarch",
@@ -94,7 +93,6 @@ object EmulatorRegistry {
9493
downloadUrl = "https://play.google.com/store/apps/details?id=com.retroarch.aarch64"
9594
),
9695

97-
// Nintendo - Standalone
9896
EmulatorDef(
9997
id = "mupen64plus_fz",
10098
packageName = "org.mupen64plusae.v3.fzurita",
@@ -216,7 +214,6 @@ object EmulatorRegistry {
216214
downloadUrl = "https://play.google.com/store/apps/details?id=com.swordfish.lemuroid"
217215
),
218216

219-
// Sony
220217
EmulatorDef(
221218
id = "duckstation",
222219
packageName = "com.github.stenzek.duckstation",
@@ -260,7 +257,6 @@ object EmulatorRegistry {
260257
downloadUrl = "https://github.com/Vita3K/Vita3K-Android/releases"
261258
),
262259

263-
// Sega
264260
// NOTE: Redream has known Android 13+ issues - explicit activity launches fail
265261
// https://github.com/TapiocaFox/Daijishou/issues/487
266262
// https://github.com/TapiocaFox/Daijishou/issues/579
@@ -300,7 +296,6 @@ object EmulatorRegistry {
300296
downloadUrl = "https://play.google.com/store/apps/details?id=com.explusalpha.MdEmu"
301297
),
302298

303-
// Arcade
304299
EmulatorDef(
305300
id = "mame4droid",
306301
packageName = "com.seleuco.mame4droid",
@@ -316,7 +311,6 @@ object EmulatorRegistry {
316311
downloadUrl = "https://play.google.com/store/apps/details?id=com.bangkokfusion.finalburn"
317312
),
318313

319-
// Other
320314
EmulatorDef(
321315
id = "scummvm",
322316
packageName = "org.scummvm.scummvm",

0 commit comments

Comments
 (0)