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

Catch null object pointers in Variant::is_nil #1002

Merged
merged 3 commits into from
Jan 8, 2023

Conversation

chitoyuu
Copy link
Contributor

@chitoyuu chitoyuu commented Jan 8, 2023

With varcall, Godot sometimes return null objects as Variants with VariantType::Object but a null pointer, instead of the usually expected/assumed VariantType::Nil. This caused Option::from_variant to panic in case of null objects returned by Godot in this way.

Fix #1001

With varcall, Godot sometimes return null objects as Variants with
`VariantType::Object` but a null pointer, instead of the usually
expected/assumed `VariantType::Nil`. This caused `Option::from_variant`
to panic in case of null objects returned by Godot in this way.

Fix godot-rust#1001
@chitoyuu chitoyuu added bug c: core Component: core (mod core_types, object, log, init, ...) labels Jan 8, 2023
@chitoyuu chitoyuu added this to the v0.11.x milestone Jan 8, 2023
@chitoyuu
Copy link
Contributor Author

chitoyuu commented Jan 8, 2023

bors try

bors bot added a commit that referenced this pull request Jan 8, 2023
@bors
Copy link
Contributor

bors bot commented Jan 8, 2023

try

Build succeeded:

@chitoyuu
Copy link
Contributor Author

chitoyuu commented Jan 8, 2023

bors r+

@bors
Copy link
Contributor

bors bot commented Jan 8, 2023

Build succeeded:

@bors bors bot merged commit 040fdf0 into godot-rust:master Jan 8, 2023
@chitoyuu chitoyuu deleted the fix/varcall-null-object-return branch January 8, 2023 17:12
chitoyuu added a commit to chitoyuu/godot-rust that referenced this pull request Jan 21, 2023
As of godot-rust#1002, it has been discovered that more than one representations
for Nil can exist, and such tests should be carried out using `is_nil()`
instead.
bors bot added a commit that referenced this pull request Jan 21, 2023
1012: Fix FromVariant impls using get_type to test for Nils r=chitoyuu a=chitoyuu

As of #1002, it has been discovered that more than one representations for Nil can exist, and such tests should be carried out using `is_nil()` instead.

Co-authored-by: Chitose Yuuzaki <chitoyuu@potatoes.gay>
chitoyuu added a commit to chitoyuu/godot-rust that referenced this pull request Jan 23, 2023
Without the flag, Godot will not emit "Leaked instance" messages we're
looking for in CI.

The leak was introduced in godot-rust#1002, in the `test_nil_object_return_value`
test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: core Component: core (mod core_types, object, log, init, ...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Node::get_node unexpected panic when given a path to a non-existent node
1 participant