Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

geodot kills godot if requested file does not exist #26

Open
chrisgraf opened this issue May 24, 2020 · 5 comments
Open

geodot kills godot if requested file does not exist #26

chrisgraf opened this issue May 24, 2020 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@chrisgraf
Copy link
Member

see discord and remarks in 34a3c49

@chrisgraf chrisgraf added the bug Something isn't working label May 24, 2020
@kb173
Copy link
Member

kb173 commented Jun 21, 2020

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
Copy link
Member Author

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 added a commit that referenced this issue Jul 15, 2020
An empty list is now returned instead of doing something which kills Godot. However, this solution is not final since we only output the error to the console at the moment. It's not trivial to let the GDNative layer know what went wrong, we might have to go with an Exception? Though Exceptions are unpopular in game programming...
@kb173
Copy link
Member

kb173 commented Jul 15, 2020

The crash should be prevented with c7c6064 - 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
Copy link
Member

kb173 commented Oct 19, 2022

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
Copy link
Member

kb173 commented Nov 3, 2022

Blocked by godotengine/godot-cpp#521

kb173 added a commit that referenced this issue Nov 4, 2022
Prevents crashes and insteads prints Errors to Godot's Debugger.

Fixes part of #26, but this type of error handling should be added everywhere.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants