Refactor GUI code, remove bad SwingWorker junk

src/main/kotlin/link/infra/packwiz/installer/DownloadTask.kt

@@ -6,8 +6,8 @@ import link.infra.packwiz.installer.metadata.SpaceSafeURI
 import link.infra.packwiz.installer.metadata.hash.Hash
 import link.infra.packwiz.installer.metadata.hash.HashUtils.getHash
 import link.infra.packwiz.installer.metadata.hash.HashUtils.getHasher
-import link.infra.packwiz.installer.ui.ExceptionDetails
+import link.infra.packwiz.installer.ui.data.ExceptionDetails
-import link.infra.packwiz.installer.ui.IOptionDetails
+import link.infra.packwiz.installer.ui.data.IOptionDetails
 import okio.Buffer
 import okio.HashingSink
 import okio.buffer

src/main/kotlin/link/infra/packwiz/installer/Main.kt

@@ -3,9 +3,8 @@
 package link.infra.packwiz.installer
 
 import link.infra.packwiz.installer.metadata.SpaceSafeURI
-import link.infra.packwiz.installer.ui.CLIHandler
+import link.infra.packwiz.installer.ui.cli.CLIHandler
-import link.infra.packwiz.installer.ui.InputStateHandler
+import link.infra.packwiz.installer.ui.gui.InstallWindow
-import link.infra.packwiz.installer.ui.InstallWindow
 import org.apache.commons.cli.DefaultParser
 import org.apache.commons.cli.Options
 import org.apache.commons.cli.ParseException
@@ -19,7 +18,7 @@ import kotlin.system.exitProcess
 @Suppress("unused")
 class Main(args: Array<String>) {
 	// Don't attempt to start a GUI if we are headless
-	var guiEnabled = !GraphicsEnvironment.isHeadless()
+	private var guiEnabled = !GraphicsEnvironment.isHeadless()
 
 	private fun startup(args: Array<String>) {
 		val options = Options()
@@ -59,8 +58,7 @@ class Main(args: Array<String>) {
 
 		cmd.getOptionValue("title")?.also(ui::setTitle)
 
-		val inputStateHandler = InputStateHandler()
+		ui.show()
-		ui.show(inputStateHandler)
 
 		val uOptions = UpdateManager.Options().apply {
 			side = cmd.getOptionValue("side")?.let((UpdateManager.Options.Side)::from) ?: side
@@ -76,18 +74,13 @@ class Main(args: Array<String>) {
 		}
 
 		// Start update process!
-		// TODO: start in SwingWorker?
 		try {
-			ui.executeManager {
+			UpdateManager(uOptions, ui)
-				try {
-					UpdateManager(uOptions, ui, inputStateHandler)
-				} catch (e: Exception) { // TODO: better error message?
-					ui.handleExceptionAndExit(e)
-				}
-			}
 		} catch (e: Exception) { // TODO: better error message?
 			ui.handleExceptionAndExit(e)
 		}
+		println("Finished successfully!")
+		ui.dispose()
 	}
 
 	companion object {

src/main/kotlin/link/infra/packwiz/installer/UpdateManager.kt

@@ -18,8 +18,7 @@ import link.infra.packwiz.installer.request.HandlerManager.getNewLoc
 import link.infra.packwiz.installer.ui.IUserInterface
 import link.infra.packwiz.installer.ui.IUserInterface.CancellationResult
 import link.infra.packwiz.installer.ui.IUserInterface.ExceptionListResult
-import link.infra.packwiz.installer.ui.InputStateHandler
+import link.infra.packwiz.installer.ui.data.InstallProgress
-import link.infra.packwiz.installer.ui.InstallProgress
 import okio.buffer
 import java.io.FileNotFoundException
 import java.io.FileReader
@@ -34,7 +33,7 @@ import java.util.concurrent.ExecutorCompletionService
 import java.util.concurrent.Executors
 import kotlin.system.exitProcess
 
-class UpdateManager internal constructor(private val opts: Options, val ui: IUserInterface, private val stateHandler: InputStateHandler) {
+class UpdateManager internal constructor(private val opts: Options, val ui: IUserInterface) {
 	private var cancelled = false
 	private var cancelledStartGame = false
 	private var errorsOccurred = false
@@ -119,7 +118,7 @@ class UpdateManager internal constructor(private val opts: Options, val ui: IUse
 			return
 		}
 
-		if (stateHandler.cancelButton) {
+		if (ui.cancelButtonPressed) {
 			showCancellationDialog()
 			handleCancellation()
 		}
@@ -142,7 +141,7 @@ class UpdateManager internal constructor(private val opts: Options, val ui: IUse
 			}
 		}
 
-		if (stateHandler.cancelButton) {
+		if (ui.cancelButtonPressed) {
 			showCancellationDialog()
 			handleCancellation()
 		}
@@ -178,14 +177,14 @@ class UpdateManager internal constructor(private val opts: Options, val ui: IUse
 		if (manifest.packFileHash?.let { packFileSource.hashIsEqual(it) } == true && invalidatedUris.isEmpty()) {
 			println("Modpack is already up to date!")
 			// todo: --force?
-			if (!stateHandler.optionsButton) {
+			if (!ui.optionsButtonPressed) {
 				return
 			}
 		}
 
 		println("Modpack name: " + pf.name)
 
-		if (stateHandler.cancelButton) {
+		if (ui.cancelButtonPressed) {
 			showCancellationDialog()
 			handleCancellation()
 		}
@@ -234,7 +233,7 @@ class UpdateManager internal constructor(private val opts: Options, val ui: IUse
 	private fun processIndex(indexUri: SpaceSafeURI, indexHash: Hash, hashFormat: String, manifest: ManifestFile, invalidatedUris: List<SpaceSafeURI>) {
 		if (manifest.indexFileHash == indexHash && invalidatedUris.isEmpty()) {
 			println("Modpack files are already up to date!")
-			if (!stateHandler.optionsButton) {
+			if (!ui.optionsButtonPressed) {
 				return
 			}
 		}
@@ -258,7 +257,7 @@ class UpdateManager internal constructor(private val opts: Options, val ui: IUse
 			ui.handleExceptionAndExit(RuntimeException("Your index hash is invalid! Please run packwiz refresh on the pack again"))
 			return
 		}
-		if (stateHandler.cancelButton) {
+		if (ui.cancelButtonPressed) {
 			showCancellationDialog()
 			return
 		}
@@ -295,7 +294,7 @@ class UpdateManager internal constructor(private val opts: Options, val ui: IUse
 			}
 		}
 
-		if (stateHandler.cancelButton) {
+		if (ui.cancelButtonPressed) {
 			showCancellationDialog()
 			return
 		}
@@ -326,7 +325,7 @@ class UpdateManager internal constructor(private val opts: Options, val ui: IUse
 			f.updateFromCache(file)
 		}
 
-		if (stateHandler.cancelButton) {
+		if (ui.cancelButtonPressed) {
 			showCancellationDialog()
 			return
 		}
@@ -360,7 +359,7 @@ class UpdateManager internal constructor(private val opts: Options, val ui: IUse
 			}
 		}
 
-		if (stateHandler.cancelButton) {
+		if (ui.cancelButtonPressed) {
 			showCancellationDialog()
 			return
 		}
@@ -369,7 +368,7 @@ class UpdateManager internal constructor(private val opts: Options, val ui: IUse
 		val nonFailedFirstTasks = tasks.filter { t -> !t.failed() }.toList()
 		val optionTasks = nonFailedFirstTasks.filter(DownloadTask::correctSide).filter(DownloadTask::isOptional).toList()
 		// If options changed, present all options again
-		if (stateHandler.optionsButton || optionTasks.any(DownloadTask::isNewOptional)) {
+		if (ui.optionsButtonPressed || optionTasks.any(DownloadTask::isNewOptional)) {
 			// new ArrayList is required so it's an IOptionDetails rather than a DownloadTask list
 			val cancelledResult = ui.showOptions(ArrayList(optionTasks))
 			try {
@@ -433,7 +432,7 @@ class UpdateManager internal constructor(private val opts: Options, val ui: IUse
 			}
 			ui.submitProgress(InstallProgress(progress, i + 1, tasks.size))
 
-			if (stateHandler.cancelButton) { // Stop all tasks, don't launch the game (it's in an invalid state!)
+			if (ui.cancelButtonPressed) { // Stop all tasks, don't launch the game (it's in an invalid state!)
 				threadPool.shutdown()
 				cancelled = true
 				return
@@ -483,6 +482,7 @@ class UpdateManager internal constructor(private val opts: Options, val ui: IUse
 		}
 	}
 
+	// TODO: move to UI?
 	private fun handleCancellation() {
 		if (cancelled) {
 			println("Update cancelled by user!")

src/main/kotlin/link/infra/packwiz/installer/metadata/hash/HashingSourceHasher.kt

@@ -5,7 +5,7 @@ import okio.Source
 
 class HashingSourceHasher internal constructor(private val type: String) : IHasher {
 	// i love naming things
-	private inner class HashingSourceGeneralHashingSource internal constructor(val delegateHashing: HashingSource) : GeneralHashingSource(delegateHashing) {
+	private inner class HashingSourceGeneralHashingSource(val delegateHashing: HashingSource) : GeneralHashingSource(delegateHashing) {
 		override val hash: Hash by lazy(LazyThreadSafetyMode.NONE) {
 			HashingSourceHash(delegateHashing.hash.hex())
 		}

src/main/kotlin/link/infra/packwiz/installer/request/handlers/RequestHandlerZip.kt

@@ -25,7 +25,7 @@ abstract class RequestHandlerZip(private val modeHasFolder: Boolean) : RequestHa
 		}
 	}
 
-	private inner class ZipReader internal constructor(zip: Source) {
+	private inner class ZipReader(zip: Source) {
 		private val zis = ZipInputStream(zip.buffer().inputStream())
 		private val readFiles: MutableMap<SpaceSafeURI, Buffer> = HashMap()
 		// Write lock implies access to ZipInputStream - only 1 thread must read at a time!

src/main/kotlin/link/infra/packwiz/installer/ui/IUserInterface.kt

@@ -1,30 +1,29 @@
 package link.infra.packwiz.installer.ui
 
+import link.infra.packwiz.installer.ui.data.ExceptionDetails
+import link.infra.packwiz.installer.ui.data.IOptionDetails
+import link.infra.packwiz.installer.ui.data.InstallProgress
 import java.util.concurrent.CompletableFuture
 import java.util.concurrent.Future
 import kotlin.system.exitProcess
 
 interface IUserInterface {
-	fun show(handler: InputStateHandler)
+	fun show()
+	fun dispose()
 	fun handleException(e: Exception)
-	@JvmDefault
 	fun handleExceptionAndExit(e: Exception) {
 		handleException(e)
 		exitProcess(1)
 	}
 
-	@JvmDefault
 	fun setTitle(title: String) {}
 	fun submitProgress(progress: InstallProgress)
-	fun executeManager(task: () -> Unit)
 	// Return true if the installation was cancelled!
 	fun showOptions(options: List<IOptionDetails>): Future<Boolean>
 
 	fun showExceptions(exceptions: List<ExceptionDetails>, numTotal: Int, allowsIgnore: Boolean): Future<ExceptionListResult>
-	@JvmDefault
 	fun disableOptionsButton() {}
 
-	@JvmDefault
 	fun showCancellationDialog(): Future<CancellationResult> {
 		return CompletableFuture<CancellationResult>().apply {
 			complete(CancellationResult.QUIT)
@@ -38,4 +37,7 @@ interface IUserInterface {
 	enum class CancellationResult {
 		QUIT, CONTINUE
 	}
+
+	var optionsButtonPressed: Boolean
+	var cancelButtonPressed: Boolean
 }

src/main/kotlin/link/infra/packwiz/installer/ui/InputStateHandler.kt

@@ -1,21 +0,0 @@
-package link.infra.packwiz.installer.ui
-
-class InputStateHandler {
-	// TODO: convert to coroutines/locks?
-	@get:Synchronized
-	var optionsButton = false
-		private set
-	@get:Synchronized
-	var cancelButton = false
-		private set
-
-	@Synchronized
-	fun pressCancelButton() {
-		cancelButton = true
-	}
-
-	@Synchronized
-	fun pressOptionsButton() {
-		optionsButton = true
-	}
-}

src/main/kotlin/link/infra/packwiz/installer/ui/SwingWorkerButWithPublicPublish.kt

@@ -1,13 +0,0 @@
-package link.infra.packwiz.installer.ui
-
-import javax.swing.SwingWorker
-
-// Q: AAA WHAT HAVE YOU DONE THIS IS DISGUSTING
-// A: it just makes things easier, so i can easily have one interface for CLI/GUI
-//    if someone has a better way to do this please PR it
-abstract class SwingWorkerButWithPublicPublish<T, V> : SwingWorker<T, V>() {
-	@SafeVarargs
-	fun publishPublic(vararg chunks: V) {
-		publish(*chunks)
-	}
-}

src/main/kotlin/link/infra/packwiz/installer/ui/cli/CLIHandler.kt

@@ -1,16 +1,26 @@
-package link.infra.packwiz.installer.ui
+package link.infra.packwiz.installer.ui.cli
 
+import link.infra.packwiz.installer.ui.IUserInterface
 import link.infra.packwiz.installer.ui.IUserInterface.ExceptionListResult
+import link.infra.packwiz.installer.ui.data.ExceptionDetails
+import link.infra.packwiz.installer.ui.data.IOptionDetails
+import link.infra.packwiz.installer.ui.data.InstallProgress
 import java.util.concurrent.CompletableFuture
 import java.util.concurrent.Future
 import kotlin.system.exitProcess
 
 class CLIHandler : IUserInterface {
+	@Volatile
+	override var optionsButtonPressed = false
+	@Volatile
+	override var cancelButtonPressed = false
+
 	override fun handleException(e: Exception) {
 		e.printStackTrace()
 	}
 
-	override fun show(handler: InputStateHandler) {}
+	override fun show() {}
+	override fun dispose() {}
 	override fun submitProgress(progress: InstallProgress) {
 		val sb = StringBuilder()
 		if (progress.hasProgress) {
@@ -24,11 +34,6 @@ class CLIHandler : IUserInterface {
 		println(sb.toString())
 	}
 
-	override fun executeManager(task: () -> Unit) {
-		task()
-		println("Finished successfully!")
-	}
-
 	override fun showOptions(options: List<IOptionDetails>): Future<Boolean> {
 		for (opt in options) {
 			opt.optionValue = true

src/main/kotlin/link/infra/packwiz/installer/ui/data/ExceptionDetails.kt

@@ -1,4 +1,4 @@
-package link.infra.packwiz.installer.ui
+package link.infra.packwiz.installer.ui.data
 
 data class ExceptionDetails(
 		val name: String,

src/main/kotlin/link/infra/packwiz/installer/ui/data/IOptionDetails.kt

@@ -1,4 +1,4 @@
-package link.infra.packwiz.installer.ui
+package link.infra.packwiz.installer.ui.data
 
 interface IOptionDetails {
 	val name: String

src/main/kotlin/link/infra/packwiz/installer/ui/data/InstallProgress.kt

@@ -1,4 +1,4 @@
-package link.infra.packwiz.installer.ui
+package link.infra.packwiz.installer.ui.data
 
 data class InstallProgress(
 		val message: String,

src/main/kotlin/link/infra/packwiz/installer/ui/gui/ExceptionListWindow.kt

@@ -1,5 +1,7 @@
-package link.infra.packwiz.installer.ui
+package link.infra.packwiz.installer.ui.gui
 
+import link.infra.packwiz.installer.ui.IUserInterface
+import link.infra.packwiz.installer.ui.data.ExceptionDetails
 import java.awt.BorderLayout
 import java.awt.Desktop
 import java.awt.event.WindowAdapter
@@ -16,7 +18,7 @@ import javax.swing.border.EmptyBorder
 class ExceptionListWindow(eList: List<ExceptionDetails>, future: CompletableFuture<IUserInterface.ExceptionListResult>, numTotal: Int, allowsIgnore: Boolean, parentWindow: JFrame?) : JDialog(parentWindow, "Failed file downloads", true) {
 	private val lblExceptionStacktrace: JTextArea
 
-	private class ExceptionListModel internal constructor(private val details: List<ExceptionDetails>) : AbstractListModel<String>() {
+	private class ExceptionListModel(private val details: List<ExceptionDetails>) : AbstractListModel<String>() {
 		override fun getSize() = details.size
 		override fun getElementAt(index: Int) = details[index].name
 		fun getExceptionAt(index: Int) = details[index].exception

src/main/kotlin/link/infra/packwiz/installer/ui/gui/InstallWindow.kt

@@ -1,10 +1,13 @@
-package link.infra.packwiz.installer.ui
+package link.infra.packwiz.installer.ui.gui
 
+import link.infra.packwiz.installer.ui.IUserInterface
 import link.infra.packwiz.installer.ui.IUserInterface.ExceptionListResult
+import link.infra.packwiz.installer.ui.data.ExceptionDetails
+import link.infra.packwiz.installer.ui.data.IOptionDetails
+import link.infra.packwiz.installer.ui.data.InstallProgress
 import java.awt.*
 import java.util.concurrent.CompletableFuture
 import java.util.concurrent.Future
-import java.util.concurrent.atomic.AtomicBoolean
 import javax.swing.*
 import javax.swing.border.EmptyBorder
 import kotlin.system.exitProcess
@@ -15,10 +18,12 @@ class InstallWindow : IUserInterface {
 	private lateinit var progressBar: JProgressBar
 	private lateinit var btnOptions: JButton
 
-	private var inputStateHandler: InputStateHandler? = null
+	@Volatile
+	override var optionsButtonPressed = false
+	@Volatile
+	override var cancelButtonPressed = false
+
 	private var title = "Updating modpack..."
-	private var worker: SwingWorkerButWithPublicPublish<Unit, InstallProgress>? = null
-	private val aboutToCrash = AtomicBoolean()
 
 	// TODO: separate JFrame junk from IUserInterface junk?
 
@@ -55,7 +60,7 @@ class InstallWindow : IUserInterface {
 						addActionListener {
 							text = "Loading..."
 							isEnabled = false
-							inputStateHandler?.pressOptionsButton()
+							optionsButtonPressed = true
 						}
 					}
 					add(btnOptions, GridBagConstraints().apply {
@@ -66,7 +71,7 @@ class InstallWindow : IUserInterface {
 					add(JButton("Cancel").apply {
 						addActionListener {
 							isEnabled = false
-							inputStateHandler?.pressCancelButton()
+							cancelButtonPressed = true
 						}
 					}, GridBagConstraints().apply {
 						gridx = 0
@@ -77,10 +82,10 @@ class InstallWindow : IUserInterface {
 		}
 	}
 
-	override fun show(handler: InputStateHandler) {
+	override fun show() {
-		inputStateHandler = handler
 		EventQueue.invokeLater {
 			try {
+				// TODO: shouldn't we do this before everything else?
 				UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName())
 				frmPackwizlauncher.isVisible = true
 			} catch (e: Exception) {
@@ -89,9 +94,15 @@ class InstallWindow : IUserInterface {
 		}
 	}
 
+	override fun dispose() {
+		EventQueue.invokeAndWait {
+			frmPackwizlauncher.dispose()
+		}
+	}
+
 	override fun handleException(e: Exception) {
 		e.printStackTrace()
-		EventQueue.invokeLater {
+		EventQueue.invokeAndWait {
 			JOptionPane.showMessageDialog(null,
 					"An error occurred: \n" + e.javaClass.canonicalName + ": " + e.message,
 					title, JOptionPane.ERROR_MESSAGE)
@@ -100,27 +111,17 @@ class InstallWindow : IUserInterface {
 
 	override fun handleExceptionAndExit(e: Exception) {
 		e.printStackTrace()
-		// TODO: Fix this mess
+		EventQueue.invokeAndWait {
-		// Used to prevent the done() handler of SwingWorker executing if the invokeLater hasn't happened yet
-		aboutToCrash.set(true)
-		EventQueue.invokeLater {
 			JOptionPane.showMessageDialog(null,
 					"A fatal error occurred: \n" + e.javaClass.canonicalName + ": " + e.message,
 					title, JOptionPane.ERROR_MESSAGE)
 			exitProcess(1)
 		}
-		// Pause forever, so it blocks while we wait for System.exit to take effect
-		try {
-			Thread.currentThread().join()
-		} catch (ex: InterruptedException) { // no u
-		}
 	}
 
 	override fun setTitle(title: String) {
 		this.title = title
-		frmPackwizlauncher.let { frame ->
+		EventQueue.invokeLater { frmPackwizlauncher.title = title }
-			EventQueue.invokeLater { frame.title = title }
-		}
 	}
 
 	override fun submitProgress(progress: InstallProgress) {
@@ -135,45 +136,16 @@ class InstallWindow : IUserInterface {
 		sb.append(progress.message)
 		// TODO: better logging library?
 		println(sb.toString())
-		worker?.publishPublic(progress)
-	}
-
-	override fun executeManager(task: Function0<Unit>) {
 		EventQueue.invokeLater {
-			// TODO: rewrite this stupidity to use channels??!!!
+			if (progress.hasProgress) {
-			worker = object : SwingWorkerButWithPublicPublish<Unit, InstallProgress>() {
+				progressBar.isIndeterminate = false
-				override fun doInBackground() {
+				progressBar.value = progress.progress
-					task.invoke()
+				progressBar.maximum = progress.progressTotal
-				}
+			} else {
-
+				progressBar.isIndeterminate = true
-				override fun process(chunks: List<InstallProgress>) {
+				progressBar.value = 0
-					// Only process last chunk
-					if (chunks.isNotEmpty()) {
-						val (message, hasProgress, progress, progressTotal) = chunks[chunks.size - 1]
-						if (hasProgress) {
-							progressBar.isIndeterminate = false
-							progressBar.value = progress
-							progressBar.maximum = progressTotal
-						} else {
-							progressBar.isIndeterminate = true
-							progressBar.value = 0
-						}
-						lblProgresslabel.text = message
-					}
-				}
-
-				override fun done() {
-					if (aboutToCrash.get()) {
-						return
-					}
-					// TODO: a better way to do this?
-					frmPackwizlauncher.dispose()
-					println("Finished successfully!")
-					exitProcess(0)
-				}
-			}.also {
-				it.execute()
 			}
+			lblProgresslabel.text = progress.message
 		}
 	}
 
@@ -207,9 +179,11 @@ class InstallWindow : IUserInterface {
 	}
 
 	override fun disableOptionsButton() {
-		btnOptions.apply {
+		EventQueue.invokeLater {
-			text = "No optional mods"
+			btnOptions.apply {
-			isEnabled = false
+				text = "No optional mods"
+				isEnabled = false
+			}
 		}
 	}
 

src/main/kotlin/link/infra/packwiz/installer/ui/gui/OptionTempHandler.kt

@@ -1,4 +1,6 @@
-package link.infra.packwiz.installer.ui
+package link.infra.packwiz.installer.ui.gui
+
+import link.infra.packwiz.installer.ui.data.IOptionDetails
 
 // Serves as a proxy for IOptionDetails, so that setOptionValue isn't called until OK is clicked
 internal class OptionTempHandler(private val opt: IOptionDetails) : IOptionDetails {

src/main/kotlin/link/infra/packwiz/installer/ui/gui/OptionsSelectWindow.kt

@@ -1,5 +1,6 @@
-package link.infra.packwiz.installer.ui
+package link.infra.packwiz.installer.ui.gui
 
+import link.infra.packwiz.installer.ui.data.IOptionDetails
 import java.awt.BorderLayout
 import java.awt.FlowLayout
 import java.awt.event.ActionEvent
@@ -18,7 +19,7 @@ class OptionsSelectWindow internal constructor(optList: List<IOptionDetails>, fu
 	private val tableModel: OptionTableModel
 	private val future: CompletableFuture<Boolean>
 
-	private class OptionTableModel internal constructor(givenOpts: List<IOptionDetails>) : TableModel {
+	private class OptionTableModel(givenOpts: List<IOptionDetails>) : TableModel {
 		private val opts: List<OptionTempHandler>
 
 		init {