Add DISABLE_OBJECT_BINDING_GUARD compiler flag #1585
Conversation
The guard in `Wrapped::Wrapped(const StringName)` detects the partial initialization of `godot::Object` subclasses. This is needed because using `new` to allocate a `godot::Object` subclass will result in unexpected crashes because the bindings for the instance have not been set. The side effect of this guard is that it also prevents any use of a `godot::Object` subclass on the stack. There are godot native types such as `godot::RegEx` that are safe to use on the stack, and are used that way throughout the godot source. Similarly, custom classes that subclass `godot::Object` may also be used safely on the stack[^1]. In this commit, I add a compiler flag to disable this guard. This will allow godot-cpp users who understand the safety implications to use `godot::Object` subclasses on the stack. [^1]: godotengine#1446 (comment)
|
Could this be applied surgically so that it isn't globally enabled but per concrete type? Perhaps as a specialization of a new My main concern with a compiler flag is that this disables this check globally. I think that it's better to be explicit for which classes can be used in this fashion versus those that cannot, allowing those that shouldn't to continue to throw the |
If you used a This isn't the case within Godot itself - the implications of not using As I've said over on #1446, I'm of the opinion that the cases where Godot is creating classes that descend from |
Have two approches, C++ class and GDScript wrapper class `HexMapCellId` and `HexMapCellIdRef`, or unified class in `HexMapIterAxial`. The unified approach requires an upstream change[^1] to allow stack allocation of `godot::Object` classes. `HexMapIterAxial` being an Object means all the doctest tests are now broken, as it cannot find the `godot::String` implementation pointers. The tests crash currently because of this. We do have tests in the godot demo project, using GUT. They confirm that `HexMapCellId.get_neighbors()` is properly working from GDScript. I'm not sure which direction I'll go in the long term. I'd like to be able to unit test in C++ and GDScript, but I don't want to have to maintain a pile of pass-through proxy methods in the wrapper class. I made one suggestion[^2] in godot-cpp to allow binding methods to class instance methods, but I don't even know if that's possible. [^1]: godotengine/godot-cpp#1585 [^2]: godotengine/godot-cpp#1446 (comment)
|
Per this note:
... I'm going to close this PR. |
The guard in
Wrapped::Wrapped(const StringName)detects the partial initialization ofgodot::Objectsubclasses. This is needed because usingnewto allocate agodot::Objectsubclass will result in unexpected crashes because the bindings for the instance have not been set.The side effect of this guard is that it also prevents any use of a
godot::Objectsubclass on the stack.There are godot native types such as
godot::RegExthat are safe to use on the stack, and are used that way throughout the godot source. Similarly, custom classes that subclassgodot::Objectmay also be used safely on the stack1.In this commit, I add a compiler flag to disable this guard. This will allow godot-cpp users who understand the safety implications to use
godot::Objectsubclasses on the stack.Footnotes
https://github.com/godotengine/godot-cpp/pull/1446#issuecomment-2351331480 ↩