Failed ptrcalls: instead of panic, print error + return default #1387
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For inbound (GDScript->Rust) calls done via ptrcall convention, the library used to panic on parameter conversions.
While ptrcalls typically don't fail as their type is already verified at the call site (e.g. in GDScript), there are still some edge cases:
Except for user panics, the new implementation no longer causes a panic, and instead uses
Resultinternally. This should be beneficial inpanic=abortsetups or for platforms which don't always have panics/exceptions enabled (e.g. Wasm).Thanks a lot to @Houtamelo for bringing this to our attention! 👍
Implementation details
Ptrcalls look like this on the FFI level:
Godot pre-initializes the
retvalue: if it is not assigned by the extension binding (godot-rust), the default value for that type is used: null pointer for objects, 0 for numbers, identity for transforms, etc. So failing ptrcalls will not cause UB -- this PR also adds tests for this behavior.