Skip to content

Conversation

@BastiaanOlij
Copy link
Collaborator

@BastiaanOlij BastiaanOlij commented Nov 18, 2021

Requires godotengine/godot#55247

Implement accessing dictionary through index. The test case in our test project works but fails on returning the dictionary which is a different issue. The problem here is that return dict will do a swap with a temporary dictionary object. This object does not instance a Godot object and the error happens on trying to free this non-existent object. We can see in the log that the dictionary was returned correctly:

USER ERROR: Condition "!_p" is true.
   at: Dictionary::_unref (core\variant\dictionary.cpp:244) - Condition "!_p" is true.
test dictionary {foo:bar, hello:world}

CI will fail until godot-headers is updated.

@BastiaanOlij BastiaanOlij added the enhancement This is an enhancement on the current functionality label Nov 18, 2021
@BastiaanOlij BastiaanOlij added this to the 4.0 milestone Nov 18, 2021
@BastiaanOlij BastiaanOlij self-assigned this Nov 18, 2021
@BastiaanOlij
Copy link
Collaborator Author

BastiaanOlij commented Nov 18, 2021

Currently crashes on attempting to update the variant in the dictionary.

libgdexample.windows.debug.64.dll!std::swap<unsigned char,0>(unsigned char & _Left, unsigned char & _Right) Line 104 (c:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\utility:104)
libgdexample.windows.debug.64.dll!std::iter_swap<unsigned char *,unsigned char *>(unsigned char * _Left, unsigned char * _Right) Line 82 (c:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\utility:82)
libgdexample.windows.debug.64.dll!std::swap<unsigned char,24,0>(unsigned char[24] & _Left, unsigned char[24] & _Right) Line 93 (c:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\utility:93)
libgdexample.windows.debug.64.dll!godot::Variant::operator=(godot::Variant && other) Line 464 (d:\Development\gdnative\godot-cpp.4.0\src\variant\variant.cpp:464)
libgdexample.windows.debug.64.dll!Example::test_dictionary() Line 152 (d:\Development\gdnative\godot-cpp.4.0\test\src\example.cpp:152)
libgdexample.windows.debug.64.dll!godot::call_with_variant_args_retc_helper<Example,godot::Dictionary>(Example * p_instance, godot::Dictionary(const Example::*)() p_method, const godot::Variant * * p_args, godot::Variant & r_ret, GDNativeCallError & r_error, IndexSequence<> __formal) Line 230 (d:\Development\gdnative\godot-cpp.4.0\include\godot_cpp\core\binder_common.hpp:230)
libgdexample.windows.debug.64.dll!godot::call_with_variant_args_retc_dv<Example,godot::Dictionary>(Example * p_instance, godot::Dictionary(const Example::*)() p_method, void * const * p_args, int p_argcount, godot::Variant & r_ret, GDNativeCallError & r_error, const std::vector<godot::Variant,std::allocator<godot::Variant>> & default_values) Line 374 (d:\Development\gdnative\godot-cpp.4.0\include\godot_cpp\core\binder_common.hpp:374)
libgdexample.windows.debug.64.dll!godot::MethodBindTRC<Example,godot::Dictionary>::call(void * p_instance, void * const * p_args, const __int64 p_argument_count, GDNativeCallError & r_error) Line 506 (d:\Development\gdnative\godot-cpp.4.0\include\godot_cpp\core\method_bind.hpp:506)
libgdexample.windows.debug.64.dll!godot::MethodBind::bind_call(void * p_method_userdata, void * p_instance, void * const * p_args, const __int64 p_argument_count, void * r_return, GDNativeCallError * r_error) Line 103 (d:\Development\gdnative\godot-cpp.4.0\src\core\method_bind.cpp:103)
godot.windows.tools.64.exe!NativeExtensionMethodBind::call(Object * p_object, const Variant * * p_args, int p_arg_count, Callable::CallError & r_error) Line 78 (d:\Development\godot4-git\core\extension\native_extension.cpp:78)
godot.windows.tools.64.exe!Object::call(const StringName & p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error) Line 837 (d:\Development\godot4-git\core\object\object.cpp:837)
godot.windows.tools.64.exe!Variant::call(const StringName & p_method, const Variant * * p_args, int p_argcount, Variant & r_ret, Callable::CallError & r_error) Line 1021 (d:\Development\godot4-git\core\variant\variant_call.cpp:1021)
godot.windows.tools.64.exe!GDScriptFunction::call(GDScriptInstance * p_instance, const Variant * * p_args, int p_argcount, Callable::CallError & r_err, GDScriptFunction::CallState * p_state) Line 1489 (d:\Development\godot4-git\modules\gdscript\gdscript_vm.cpp:1489)
godot.windows.tools.64.exe!GDScriptInstance::call(const StringName & p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error) Line 1511 (d:\Development\godot4-git\modules\gdscript\gdscript.cpp:1511)
godot.windows.tools.64.exe!Node::_gdvirtual__ready_call() Line 236 (d:\Development\godot4-git\scene\main\node.h:236)
godot.windows.tools.64.exe!Node::_notification(int p_notification) Line 145 (d:\Development\godot4-git\scene\main\node.cpp:145)
godot.windows.tools.64.exe!Node::_notificationv(int p_notification, bool p_reversed) Line 48 (d:\Development\godot4-git\scene\main\node.h:48)
godot.windows.tools.64.exe!Object::notification(int p_notification, bool p_reversed) Line 848 (d:\Development\godot4-git\core\object\object.cpp:848)
godot.windows.tools.64.exe!Node::_propagate_ready() Line 189 (d:\Development\godot4-git\scene\main\node.cpp:189)
godot.windows.tools.64.exe!Node::_propagate_ready() Line 181 (d:\Development\godot4-git\scene\main\node.cpp:181)

@BastiaanOlij
Copy link
Collaborator Author

Changing over to using operator[] on the Godot side as @groud suggested (see bruvzg/godot#1), modifying the dictionary now works. This actually does enough for what I need it to do :P

However, the return dict at the end of the test case in our test example results in:

USER ERROR: Condition "!_p" is true.
   at: Dictionary::_unref (core\variant\dictionary.cpp:244) - Condition "!_p" is true.

Looks like it unreferences the dictionary held within when the variable goes out of scope.

@akien-mga
Copy link
Member

I guess you need to update godot-headers to latest upstream to fix the build.

@BastiaanOlij
Copy link
Collaborator Author

I guess you need to update godot-headers to latest upstream to fix the build.

Yup, all done now

@akien-mga akien-mga merged commit adbc0bf into godotengine:master Nov 23, 2021
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement This is an enhancement on the current functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants