-
Notifications
You must be signed in to change notification settings - Fork 41
Description
Version: Github snapshot - Main Branch (Commit 792cf44ed9e35161b0b780d85b9032bc2690be38)
Creating a mesh of type 'model' causes a crash. Creating a mesh of a primitive type and changing it to Model also causes a crash. (IllegalArgumentException 'Required Value was Null')
This is because SceneNodes.kt requires the model path to be non-null, however the Model class allows a null value, and it appears that the default value when creating a Model mesh is a Model with a null path.
| class Model(name: String? = null) : Node(name) { |
kool/kool-editor-model/src/commonMain/kotlin/de/fabmax/kool/editor/api/SceneNodes.kt
Line 319 in 792cf44
| val modelRef = requireNotNull(modelShape.toAssetRef()) |
If you create a GLTF mesh, it gets the type Model. If you then try to set it to None, this also causes a crash (FileNotFound exception)
The FileNotFound exception is caused by the 'None' option creating a Model with an empty string as path
kool/kool-editor/src/commonMain/kotlin/de/fabmax/kool/editor/ui/componenteditors/MeshEditor.kt
Line 247 in 792cf44
| valueSetter = { _, newValue -> ShapeData.Model(newValue.item?.path ?: "") }, |
Which is then used by SceneNodes to attempt to load a model (since there is no model for 'empty path', it crashes)
kool/kool-editor-model/src/commonMain/kotlin/de/fabmax/kool/editor/api/SceneNodes.kt
Lines 319 to 321 in 792cf44
| val modelRef = requireNotNull(modelShape.toAssetRef()) | |
| val gltf = AppAssets.requireModel(modelRef) |
Now I do not know whether you have a strong opinion on the 'correct' solution, but I think the engine should either stick with utilizing null values (and checking for these) or should allow for empty strings (and should also check for these accordingly if used as defaultvalues), but not this 'hybrid'.
In my personal fix for this issue I stuck with null values and attempting to handle them gracefully.