kool icon indicating copy to clipboard operation
kool copied to clipboard

[Kool Editor] Mesh Editor: Setting to 'Model' causes a crash, attempting to set an existing Model from a mesh to 'None' also causes a crash.

Open 0megaD opened this issue 2 months ago • 0 comments

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.

https://github.com/kool-engine/kool/blob/792cf44ed9e35161b0b780d85b9032bc2690be38/kool-core/src/commonMain/kotlin/de/fabmax/kool/scene/Model.kt#L7

https://github.com/kool-engine/kool/blob/792cf44ed9e35161b0b780d85b9032bc2690be38/kool-editor-model/src/commonMain/kotlin/de/fabmax/kool/editor/api/SceneNodes.kt#L319


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

https://github.com/kool-engine/kool/blob/792cf44ed9e35161b0b780d85b9032bc2690be38/kool-editor/src/commonMain/kotlin/de/fabmax/kool/editor/ui/componenteditors/MeshEditor.kt#L247

Which is then used by SceneNodes to attempt to load a model (since there is no model for 'empty path', it crashes)

https://github.com/kool-engine/kool/blob/792cf44ed9e35161b0b780d85b9032bc2690be38/kool-editor-model/src/commonMain/kotlin/de/fabmax/kool/editor/api/SceneNodes.kt#L319-L321


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.

0megaD avatar Dec 15 '25 23:12 0megaD