search

boku-ilen / geodot-plugin

Godot plugin for loading geospatial data
GNU General Public License v3.0
109 stars 19 forks source link

geodot kills godot if requested file does not exist #26

Open chrisgraf opened 4 years ago

chrisgraf commented 4 years ago

see discord and remarks in https://github.com/boku-ilen/geodot-plugin/commit/34a3c4956d9e89b12c17b98638b445f11a9197c1

kb173 commented 4 years ago

Thanks for the remarks! Should we just return null and leave the error handling to the GDScript code? (+ output a proper Godot Error) That would be consistent with GDScript's load and preload functions, for example.

chrisgraf commented 4 years ago

Yes, the error should be handled where we have our "proper" logging and the code has enough information to estimate the impact of the failed call. So handling this in GDScript seems correct to me.

kb173 commented 4 years ago

The crash should be prevented with https://github.com/boku-ilen/geodot-plugin/commit/c7c60647e8f1f139b519f1e9d28415ef73f3e1b6 - an empty list is returned if the dataset is invalid. Currently, an error is printed to the console saying that the dataset couldn't be opened, but we'd want to get this error into Godot. We might need to use Exceptions for this. They're not commonly used in game development, so I'm not sure whether we want to bring them into Geodot?

kb173 commented 1 year ago

Seems like with GDExtension there are ways to properly propagate errors to Godot like ERR_FAIL_V_MSG (see the Godot XR reference project https://github.com/GodotVR/godot_xr_reference/blob/master/src/xr_interface_reference.cpp)

kb173 commented 1 year ago

Blocked by https://github.com/godotengine/godot-cpp/issues/521