godot-cpp
godot-cpp copied to clipboard
Global godot-type variable cause crash and load gdextension failed
Godot version
4.3.dev(11d3768)
godot-cpp version
4.3.dev(e23b117)
System information
win11
Issue description
This issue have existed for a long time, The reproduction steps are the same as this comment: https://github.com/godotengine/godot-cpp/issues/358#issuecomment-566168170.
String s; // crashes
class SomeClass {
static Dictionary d; // crashes
static Node *singleton; // fine, pointer doesn't call a constructor
};
This issue is quite tricky, as it is caused by godot load gdextension .dll and initializate GDExtensionInterface order. Do need to solve this problem?
Steps to reproduce
// world.h
const String level0 = "res://level/level0.tscn"; // load gdextension.dll will call String constructor, but this time gdextension function didnt initialized
class World : public Node {
GDCLASS(World, Node)
static void _bind_methods() {}
};
Define a global variable String, open godot.exe timeline:
- start open godot.exe
- godot.exe: Load gdextension.dll -> dll: global variable Construct -> dll: call
Stringconstruct GDExtensionInterface(not initialzed and will Crash) - godot.exe: initialize gdextension.dll GDExtensionInterface
Minimal reproduction project
N/A
Well, we can't make Godot types work if they are constructed before the GDExtension has been initialized.
It would be nice to not crash, although, I'm not sure how feasible that is. We could wrap everything in a check to see if it's been initialized yet, and then output a message, I guess?
There maybe two types that need to be checked:
- function pointer type declare in
gdextension_interface.h(define in godot.cpp) _method_bindingsin the Variant type(for example in string_name.cpp).
Checking if they are nullptr (or assignment placeholder) requires writing a big logic and the final outcome still cannot declare global variables. I think it's better to not solved it (global variable not many usage scenarios and it's not necessary).
Doing some experimentation, one type of static storage does work: static variables inside of functions.
int Foo::get_increment()
{
static int counter {0};
++counter;
return counter;
}
one type of static storage does work: static variables inside of functions
Yes, if the function is called after the GDExtension has been initialized, that should work.
From my comment on #1557:
It would be nice to not crash, however, that would probably mean adding a check to see if GDExtension has been initialized yet to all constructors, and the only things we could do is (1) construct an invalid object that wouldn't actually work if you tried to use it later, or (2) crash but with a better error message :-)
I'm starting to think that maybe we should try to implement nr 2? Using unlikely() we could probably make the check speedy enough, and we'd just be adding this to constructors. (It's possible to call other functions too early as well, but I feel like that's much easier for developers to figure out, and hasn't really been a frequent support issue in the way that constructing objects too early has been.)
This limitation is also something that could use to be documented somewhere.
I think we can initialize the gdextension-interface function pointer as a FuncA instead of nullptr, FuncA output an error: "Call constructor too early, earlier than GDExtendenInterface initialization, please check if global godot variables are declared" (similar error output and crash).
After the initialization of the existing GDExtendeInterface, it will overwrite our FuncA, so there is no need to call the unlike() function to check.
I think we can initialize the gdextension-interface function pointer as a
FuncAinstead of nullptr,FuncAoutput an error: "Call constructor too early, earlier than GDExtendenInterface initialization, please check if global godot variables are declared" (similar error output and crash).
Ooo, that's an interesting idea! However, it would be nice to say in the error message the name of the object that's being constructed, and I think the only way to do that would be to have a FuncA for each class. I think that's doable, though.