Merge pull request #37 from DanielSWolf/bugfix/mutable-set

Use mutable set for animation names

VERSION.md

@@ -1,5 +1,9 @@
 # Version history
 
+## Unreleased
+
+* Fixed bug in Rhubarb for Spine where processing failed depending on the number of existing animations. See [issue #34](https://github.com/DanielSWolf/rhubarb-lip-sync/issues/34#issuecomment-378198776).
+
 ## Version 1.7.1
 
 * Fixed [issue #34](https://github.com/DanielSWolf/rhubarb-lip-sync/issues/34): Generic error message in Rhubarb for Spine

extras/EsotericSoftwareSpine/src/main/kotlin/com/rhubarb_lip_sync/rhubarb_for_spine/AnimationFileModel.kt

@@ -16,41 +16,41 @@ class AnimationFileModel(val parentModel: MainModel, animationFilePath: Path, pr
 	val spineJson = SpineJson(animationFilePath)
 
 	val slotsProperty = SimpleObjectProperty<ObservableList<String>>()
-	var slots by slotsProperty
+	private var slots: ObservableList<String> by slotsProperty
-		private set
 
 	val mouthSlotProperty: SimpleStringProperty = SimpleStringProperty().alsoListen {
-		mouthNaming = if (mouthSlot != null)
+		val mouthSlot = this.mouthSlot
+		val mouthNaming = if (mouthSlot != null)
 			MouthNaming.guess(spineJson.getSlotAttachmentNames(mouthSlot))
 		else null
+		this.mouthNaming = mouthNaming
 
-		mouthShapes = if (mouthSlot != null) {
+		mouthShapes = if (mouthSlot != null && mouthNaming != null) {
 			val mouthNames = spineJson.getSlotAttachmentNames(mouthSlot)
 			MouthShape.values().filter { mouthNames.contains(mouthNaming.getName(it)) }
 		} else listOf()
 
-		mouthSlotError = if (mouthSlot != null) null
+		mouthSlotError = if (mouthSlot != null)
-		else "No slot with mouth drawings specified."
+			null
+		else
+			"No slot with mouth drawings specified."
 	}
-	var mouthSlot by mouthSlotProperty
+	private var mouthSlot: String? by mouthSlotProperty
 
 	val mouthSlotErrorProperty = SimpleStringProperty()
-	var mouthSlotError by mouthSlotErrorProperty
+	private var mouthSlotError: String? by mouthSlotErrorProperty
-		private set
 
 	val mouthNamingProperty = SimpleObjectProperty<MouthNaming>()
-	var mouthNaming by mouthNamingProperty
+	private var mouthNaming: MouthNaming? by mouthNamingProperty
-		private set
 
 	val mouthShapesProperty = SimpleObjectProperty<List<MouthShape>>().alsoListen {
 		mouthShapesError = getMouthShapesErrorString()
 	}
-	var mouthShapes by mouthShapesProperty
+	var mouthShapes: List<MouthShape> by mouthShapesProperty
 		private set
 
 	val mouthShapesErrorProperty = SimpleStringProperty()
-	var mouthShapesError by mouthShapesErrorProperty
+	private var mouthShapesError: String? by mouthShapesErrorProperty
-		private set
 
 	val audioFileModelsProperty = SimpleListProperty<AudioFileModel>(
 		spineJson.audioEvents
@@ -63,7 +63,7 @@ class AnimationFileModel(val parentModel: MainModel, animationFilePath: Path, pr
 			}
 			.observable()
 	)
-	val audioFileModels by audioFileModelsProperty
+	val audioFileModels: ObservableList<AudioFileModel> by audioFileModelsProperty
 
 	val busyProperty = SimpleBooleanProperty().apply {
 		bind(object : BooleanBinding() {
@@ -90,10 +90,9 @@ class AnimationFileModel(val parentModel: MainModel, animationFilePath: Path, pr
 			}
 		})
 	}
-	val valid by validProperty
 
 	private fun saveAnimation(animationName: String, audioEventName: String, mouthCues: List<MouthCue>) {
-		spineJson.createOrUpdateAnimation(mouthCues, audioEventName, animationName, mouthSlot, mouthNaming)
+		spineJson.createOrUpdateAnimation(mouthCues, audioEventName, animationName, mouthSlot!!, mouthNaming!!)
 		spineJson.save()
 	}
 

extras/EsotericSoftwareSpine/src/main/kotlin/com/rhubarb_lip_sync/rhubarb_for_spine/AudioFileModel.kt

@@ -11,6 +11,7 @@ import javafx.scene.control.Alert
 import javafx.scene.control.ButtonType
 import tornadofx.getValue
 import tornadofx.setValue
+import java.nio.file.Path
 import java.util.concurrent.ExecutorService
 import java.util.concurrent.Future
 
@@ -20,15 +21,14 @@ class AudioFileModel(
 	private val executor: ExecutorService,
 	private val reportResult: (List<MouthCue>) -> Unit
 ) {
-	val spineJson = parentModel.spineJson
+	private val spineJson = parentModel.spineJson
 
-	val audioFilePath = spineJson.audioDirectoryPath.resolve(audioEvent.relativeAudioFilePath)
+	private val audioFilePath: Path = spineJson.audioDirectoryPath.resolve(audioEvent.relativeAudioFilePath)
 
 	val eventNameProperty = SimpleStringProperty(audioEvent.name)
-	val eventName by eventNameProperty
+	val eventName: String by eventNameProperty
 
 	val displayFilePathProperty = SimpleStringProperty(audioEvent.relativeAudioFilePath)
-	val displayFilePath by displayFilePathProperty
 
 	val animationNameProperty = SimpleStringProperty().apply {
 		val mainModel = parentModel.parentModel
@@ -45,13 +45,13 @@ class AudioFileModel(
 			}
 		})
 	}
-	val animationName by animationNameProperty
+	val animationName: String by animationNameProperty
 
 	val dialogProperty = SimpleStringProperty(audioEvent.dialog)
-	val dialog: String? by dialogProperty
+	private val dialog: String? by dialogProperty
 
 	val animationProgressProperty = SimpleObjectProperty<Double?>(null)
-	var animationProgress by animationProgressProperty
+	var animationProgress: Double? by animationProgressProperty
 		private set
 
 	private val animatedProperty = SimpleBooleanProperty().apply {
@@ -92,7 +92,6 @@ class AudioFileModel(
 			}
 		})
 	}
-	val audioFileState by audioFileStateProperty
 
 	val busyProperty = SimpleBooleanProperty().apply {
 		bind(object : BooleanBinding() {
@@ -120,7 +119,6 @@ class AudioFileModel(
 			}
 		})
 	}
-	val actionLabel by actionLabelProperty
 
 	fun performAction() {
 		if (future == null) {
@@ -162,21 +160,21 @@ class AudioFileModel(
 				}
 			} catch (e: InterruptedException) {
 			} catch (e: Exception) {
-				e.printStackTrace(System.err);
+				e.printStackTrace(System.err)
 
 				Platform.runLater {
 					Alert(Alert.AlertType.ERROR).apply {
 						headerText = "Error performing lip sync for event '$eventName'."
-						contentText = if (e.message.isNullOrEmpty())
+						contentText = if (e is EndUserException)
-							// Some exceptions don't have a message
-							"An internal error of type ${e.javaClass.name} occurred."
-						else
 							e.message
+						else
+							("An internal error occurred.\n"
+								+ "Please report an issue, including the following information.\n"
+								+ getStackTrace(e))
 						show()
 					}
 				}
 			}
-
 		}
 		future = executor.submit(wrapperTask)
 	}

extras/EsotericSoftwareSpine/src/main/kotlin/com/rhubarb_lip_sync/rhubarb_for_spine/EndUserException.kt

@@ -0,0 +1,4 @@
+package com.rhubarb_lip_sync.rhubarb_for_spine
+
+// An exception with a human-readable message that can be shown to the end user
+class EndUserException(message: String): Exception(message)

extras/EsotericSoftwareSpine/src/main/kotlin/com/rhubarb_lip_sync/rhubarb_for_spine/MainModel.kt

@@ -15,38 +15,36 @@ class MainModel(private val executor: ExecutorService) {
 		filePathError = getExceptionMessage {
 			animationFileModel = null
 			if (value.isNullOrBlank()) {
-				throw Exception("No input file specified.")
+				throw EndUserException("No input file specified.")
 			}
 
 			val path = try {
 				val trimmed = value.removeSurrounding("\"")
 				Paths.get(trimmed)
 			} catch (e: InvalidPathException) {
-				throw Exception("Not a valid file path.")
+				throw EndUserException("Not a valid file path.")
 			}
 
 			if (!Files.exists(path)) {
-				throw Exception("File does not exist.")
+				throw EndUserException("File does not exist.")
 			}
 
 			animationFileModel = AnimationFileModel(this, path, executor)
 		}
 	}
-	var filePathString by filePathStringProperty
 
 	val filePathErrorProperty = SimpleStringProperty()
-	var filePathError by filePathErrorProperty
+	private var filePathError: String? by filePathErrorProperty
-		private set
 
 	val animationFileModelProperty = SimpleObjectProperty<AnimationFileModel?>()
 	var animationFileModel by animationFileModelProperty
 		private set
 
 	val animationPrefixProperty = SimpleStringProperty("say_")
-	var animationPrefix by animationPrefixProperty
+	var animationPrefix: String by animationPrefixProperty
 
 	val animationSuffixProperty = SimpleStringProperty("")
-	var animationSuffix by animationSuffixProperty
+	var animationSuffix: String by animationSuffixProperty
 
 	private fun getDefaultPathString() = FX.application.parameters.raw.firstOrNull()
 }

extras/EsotericSoftwareSpine/src/main/kotlin/com/rhubarb_lip_sync/rhubarb_for_spine/MouthNaming.kt

@@ -2,7 +2,7 @@ package com.rhubarb_lip_sync.rhubarb_for_spine
 
 import java.util.*
 
-class MouthNaming(val prefix: String, val suffix: String, val mouthShapeCasing: MouthShapeCasing) {
+class MouthNaming(private val prefix: String, private val suffix: String, private val mouthShapeCasing: MouthShapeCasing) {
 
 	companion object {
 		fun guess(mouthNames: List<String>): MouthNaming {

extras/EsotericSoftwareSpine/src/main/kotlin/com/rhubarb_lip_sync/rhubarb_for_spine/MouthShape.kt

@@ -10,7 +10,7 @@ enum class MouthShape {
 		get() = !this.isBasic
 
 	companion object {
-		val basicShapeCount = 6
+		const val basicShapeCount = 6
 
 		val basicShapes = MouthShape.values().take(basicShapeCount)
 

extras/EsotericSoftwareSpine/src/main/kotlin/com/rhubarb_lip_sync/rhubarb_for_spine/RhubarbTask.kt

@@ -24,7 +24,7 @@ class RhubarbTask(
 			throw InterruptedException()
 		}
 		if (!Files.exists(audioFilePath)) {
-			throw IllegalArgumentException("File '$audioFilePath' does not exist.");
+			throw EndUserException("File '$audioFilePath' does not exist.")
 		}
 
 		val dialogFile = if (dialog != null) TemporaryTextFile(dialog) else null
@@ -50,7 +50,7 @@ class RhubarbTask(
 							return parseRhubarbResult(resultString)
 						}
 						"failure" -> {
-							throw Exception(message.string("reason"))
+							throw EndUserException(message.string("reason") ?: "Rhubarb failed without reason.")
 						}
 					}
 				}
@@ -58,13 +58,13 @@ class RhubarbTask(
 				process.destroyForcibly()
 				throw e
 			} catch (e: EOFException) {
-				throw Exception("Rhubarb terminated unexpectedly.")
+				throw EndUserException("Rhubarb terminated unexpectedly.")
 			} finally {
-				process.waitFor();
+				process.waitFor()
 			}
 		}}
 
-		throw Exception("An unexpected error occurred.")
+		throw EndUserException("Audio file processing terminated in an unexpected way.")
 	}
 
 	private fun parseRhubarbResult(jsonString: String): List<MouthCue> {
@@ -118,7 +118,7 @@ class RhubarbTask(
 			}
 			currentDirectory = currentDirectory.parent
 		}
-		throw Exception("Could not find Rhubarb Lip Sync executable '$rhubarbBinName'."
+		throw EndUserException("Could not find Rhubarb Lip Sync executable '$rhubarbBinName'."
 			+ " Expected to find it in '$guiBinDirectory' or any directory above.")
 	}
 

extras/EsotericSoftwareSpine/src/main/kotlin/com/rhubarb_lip_sync/rhubarb_for_spine/SpineJson.kt

@@ -6,24 +6,24 @@ import java.nio.charset.StandardCharsets
 import java.nio.file.Files
 import java.nio.file.Path
 
-class SpineJson(val filePath: Path) {
+class SpineJson(private val filePath: Path) {
-	val fileDirectoryPath: Path = filePath.parent
+	private val fileDirectoryPath: Path = filePath.parent
-	val json: JsonObject
+	private val json: JsonObject
 	private val skeleton: JsonObject
 	private val defaultSkin: JsonObject
 
 	init {
 		if (!Files.exists(filePath)) {
-			throw Exception("File '$filePath' does not exist.")
+			throw EndUserException("File '$filePath' does not exist.")
 		}
 		try {
 			json = Parser().parse(filePath.toString()) as JsonObject
 		} catch (e: Exception) {
-			throw Exception("Wrong file format. This is not a valid JSON file.")
+			throw EndUserException("Wrong file format. This is not a valid JSON file.")
 		}
-		skeleton = json.obj("skeleton") ?: throw Exception("JSON file is corrupted.")
+		skeleton = json.obj("skeleton") ?: throw EndUserException("JSON file is corrupted.")
-		val skins = json.obj("skins") ?: throw Exception("JSON file doesn't contain skins.")
+		val skins = json.obj("skins") ?: throw EndUserException("JSON file doesn't contain skins.")
-		defaultSkin = skins.obj("default") ?: throw Exception("JSON file doesn't have a default skin.")
+		defaultSkin = skins.obj("default") ?: throw EndUserException("JSON file doesn't have a default skin.")
 		validateProperties()
 	}
 
@@ -32,14 +32,14 @@ class SpineJson(val filePath: Path) {
 		audioDirectoryPath
 	}
 
-	val imagesDirectoryPath: Path get() {
+	private val imagesDirectoryPath: Path get() {
 		val relativeImagesDirectory = skeleton.string("images")
-			?: throw Exception("JSON file is incomplete: Images path is missing."
+			?: throw EndUserException("JSON file is incomplete: Images path is missing."
 				+ "Make sure to check 'Nonessential data' when exporting.")
 
 		val imagesDirectoryPath = fileDirectoryPath.resolve(relativeImagesDirectory).normalize()
 		if (!Files.exists(imagesDirectoryPath)) {
-			throw Exception("Could not find images directory relative to the JSON file."
+			throw EndUserException("Could not find images directory relative to the JSON file."
 				+ " Make sure the JSON file is in the same directory as the original Spine file.")
 		}
 
@@ -48,12 +48,12 @@ class SpineJson(val filePath: Path) {
 
 	val audioDirectoryPath: Path get() {
 		val relativeAudioDirectory = skeleton.string("audio")
-			?: throw Exception("JSON file is incomplete: Audio path is missing."
+			?: throw EndUserException("JSON file is incomplete: Audio path is missing."
 			+ "Make sure to check 'Nonessential data' when exporting.")
 
 		val audioDirectoryPath = fileDirectoryPath.resolve(relativeAudioDirectory).normalize()
 		if (!Files.exists(audioDirectoryPath)) {
-			throw Exception("Could not find audio directory relative to the JSON file."
+			throw EndUserException("Could not find audio directory relative to the JSON file."
 				+ " Make sure the JSON file is in the same directory as the original Spine file.")
 		}
 
@@ -80,7 +80,7 @@ class SpineJson(val filePath: Path) {
 		val events = json.obj("events") ?: JsonObject()
 		val result = mutableListOf<AudioEvent>()
 		for ((name, value) in events) {
-			if (value !is JsonObject) throw Exception("Invalid event found.")
+			if (value !is JsonObject) throw EndUserException("Invalid event found.")
 
 			val relativeAudioFilePath = value.string("audio") ?: continue
 
@@ -96,7 +96,7 @@ class SpineJson(val filePath: Path) {
 	}
 
 	val animationNames = observableSet<String>(
-		json.obj("animations")?.map{ it.key }?.toSet() ?: setOf()
+		json.obj("animations")?.map{ it.key }?.toMutableSet() ?: mutableSetOf()
 	)
 
 	fun createOrUpdateAnimation(mouthCues: List<MouthCue>, eventName: String, animationName: String,

extras/EsotericSoftwareSpine/src/main/kotlin/com/rhubarb_lip_sync/rhubarb_for_spine/tools.kt

@@ -2,11 +2,10 @@ package com.rhubarb_lip_sync.rhubarb_for_spine
 
 import javafx.application.Platform
 import javafx.beans.property.Property
-import java.util.concurrent.ExecutionException
-import java.util.concurrent.locks.Condition
 import java.util.concurrent.locks.ReentrantLock
 import kotlin.concurrent.withLock
-
+import java.io.PrintWriter
+import java.io.StringWriter
 
 val List<String>.commonPrefix: String get() {
 	return if (isEmpty()) "" else this.reduce { result, string -> result.commonPrefixWith(string) }
@@ -29,14 +28,13 @@ fun <TValue, TProperty : Property<TValue>> TProperty.alsoListen(listener: (TValu
 
 fun getExceptionMessage(action: () -> Unit): String? {
 	try {
-		action();
+		action()
 	} catch (e: Exception) {
 		return e.message
 	}
 	return null
 }
 
-
 /**
  * Invokes a Runnable on the JFX thread and waits until it's finished.
  * Similar to SwingUtilities.invokeAndWait.
@@ -68,4 +66,10 @@ fun runAndWait(action: () -> Unit) {
 			throwable?.let { throw it }
 		}
 	}
+}
+
+fun getStackTrace(e: Exception): String {
+	val stringWriter = StringWriter()
+	e.printStackTrace(PrintWriter(stringWriter))
+	return stringWriter.toString()
 }