Skip to content

Fixing reference return #662

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Fixing reference return #662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions include/godot_cpp/classes/ref.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,14 @@ class Ref {
r.reference = nullptr;
}

template <class T_Other>
Ref(Ref<T_Other> &&p_other) {
// Should just be able to swap these...
T *swap = (T *)p_other.reference;
p_other.reference = (T_Other *)reference;
reference = swap;
}

Ref(T *p_reference) {
if (p_reference) {
ref_pointer(p_reference);
Expand Down
5 changes: 4 additions & 1 deletion test/demo/main.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ func _ready():
($Example as Example).simple_const_func() # Force use of ptrcall
prints("returned", $Example.return_something("some string"))
prints("returned const", $Example.return_something_const())
prints("returned ref", $Example.return_extended_ref())

prints("returned example ref object: ", $Example.return_extended_ref())
Copy link
Member

@akien-mga akien-mga Feb 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this mean to say "extended" instead of "example"?


var ref = ExampleRef.new()
prints("sending ref: ", ref.get_instance_id(), "returned ref: ", $Example.extended_ref_checks(ref).get_instance_id())

prints("vararg args", $Example.varargs_func("some", "arguments", "to", "test"))

prints("test array", $Example.test_array())
Expand Down
10 changes: 9 additions & 1 deletion test/demo/main.tscn
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,25 @@
[gd_scene load_steps=2 format=3 uid="uid://dmx2xuigcpvt4"]
[gd_scene load_steps=3 format=3 uid="uid://dmx2xuigcpvt4"]

[ext_resource type="Script" path="res://main.gd" id="1_c326s"]

[sub_resource type="ExampleRef" id="ExampleRef_gveeo"]
value = 5

[node name="Node" type="Node"]
script = ExtResource( "1_c326s" )

[node name="Example" type="Example" parent="."]
ref_obj = SubResource( "ExampleRef_gveeo" )
__meta__ = {
"_edit_use_anchors_": false
}

[node name="Label" type="Label" parent="Example"]
offset_left = 194.0
offset_top = -2.0
offset_right = 234.0
offset_bottom = 21.0
theme_override_font_sizes/font_size = 16
__meta__ = {
"_edit_use_anchors_": false
}
Expand Down
57 changes: 47 additions & 10 deletions test/src/example.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,22 +38,43 @@

using namespace godot;

uint64_t ExampleRef::instance_count = 0;

void ExampleRef::_bind_methods() {
ClassDB::bind_method(D_METHOD("get_value"), &ExampleRef::get_value);
ClassDB::bind_method(D_METHOD("set_value", "value"), &ExampleRef::set_value);
ADD_PROPERTY(PropertyInfo(Variant::INT, "value"), "set_value", "get_value");
}

ExampleRef::ExampleRef() {
UtilityFunctions::print("ExampleRef created.");
instance_count++;
UtilityFunctions::print("ExampleRef created. Our total instance count is now: ", instance_count);

// default this
value = 1;
}

ExampleRef::~ExampleRef() {
UtilityFunctions::print("ExampleRef destroyed.");
instance_count--;
UtilityFunctions::print("ExampleRef destroyed. Our total instance count is now: ", instance_count);
}

void ExampleRef::set_value(int64_t p_value) {
value = p_value;
}

int64_t ExampleRef::get_value() const {
return value;
}

void Example::_bind_methods() {
// Methods.
ClassDB::bind_method(D_METHOD("simple_func"), &Example::simple_func);
ClassDB::bind_method(D_METHOD("simple_const_func"), &Example::simple_const_func);
ClassDB::bind_method(D_METHOD("return_something"), &Example::return_something);
ClassDB::bind_method(D_METHOD("return_something", "base"), &Example::return_something);
ClassDB::bind_method(D_METHOD("return_something_const"), &Example::return_something_const);
ClassDB::bind_method(D_METHOD("return_extended_ref"), &Example::return_extended_ref);
ClassDB::bind_method(D_METHOD("extended_ref_checks"), &Example::extended_ref_checks);
ClassDB::bind_method(D_METHOD("extended_ref_checks", "ref"), &Example::extended_ref_checks);

ClassDB::bind_method(D_METHOD("test_array"), &Example::test_array);
ClassDB::bind_method(D_METHOD("test_dictionary"), &Example::test_dictionary);
Expand All @@ -73,6 +94,10 @@ void Example::_bind_methods() {
ClassDB::bind_method(D_METHOD("set_custom_position", "position"), &Example::set_custom_position);
ADD_PROPERTY(PropertyInfo(Variant::VECTOR2, "group_subgroup_custom_position"), "set_custom_position", "get_custom_position");

ClassDB::bind_method(D_METHOD("get_ref_obj"), &Example::get_ref_obj);
ClassDB::bind_method(D_METHOD("set_ref_obj", "ref_obj"), &Example::set_ref_obj);
ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "ref_obj", PROPERTY_HINT_RESOURCE_TYPE, "ExampleRef"), "set_ref_obj", "get_ref_obj");

// Signals.
ADD_SIGNAL(MethodInfo("custom_signal", PropertyInfo(Variant::STRING, "name"), PropertyInfo(Variant::INT, "value")));
ClassDB::bind_method(D_METHOD("emit_custom_signal", "name", "value"), &Example::emit_custom_signal);
Expand All @@ -85,11 +110,11 @@ void Example::_bind_methods() {
}

Example::Example() {
UtilityFunctions::print("Constructor.");
UtilityFunctions::print("Example Constructor.");
}

Example::~Example() {
UtilityFunctions::print("Destructor.");
UtilityFunctions::print("Example Destructor.");
}

// Methods.
Expand All @@ -115,15 +140,19 @@ Viewport *Example::return_something_const() const {
return nullptr;
}

ExampleRef *Example::return_extended_ref() const {
return memnew(ExampleRef());
Ref<ExampleRef> Example::return_extended_ref() const {
// When subclassing RefCounted we should ALWAYS use Ref<..> or Godot will start doing confusing things as it will start using reference counting to manage the object.
// We should never instantiate the object directly such as this:
// return memnew(ExampleRef());

Ref<ExampleRef> ref;
ref.instantiate();
return ref;
}

Ref<ExampleRef> Example::extended_ref_checks(Ref<ExampleRef> p_ref) const {
Ref<ExampleRef> ref;
ref.instantiate();
// TODO the returned value gets dereferenced too early and return a null object otherwise.
ref->reference();
UtilityFunctions::print("Example ref checks called with value: ", p_ref->get_instance_id(), ", returning value: ", ref->get_instance_id());
return ref;
}
Expand Down Expand Up @@ -165,6 +194,14 @@ Vector2 Example::get_custom_position() const {
return custom_position;
}

void Example::set_ref_obj(const Ref<ExampleRef> p_ref) {
ref_obj = p_ref;
}

Ref<ExampleRef> Example::get_ref_obj() const {
return ref_obj;
}

// Virtual function override.
bool Example::_has_point(const Vector2 &point) const {
Label *label = get_node<Label>("Label");
Expand Down
21 changes: 17 additions & 4 deletions test/src/example.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,30 @@

#include <godot_cpp/classes/control.hpp>
#include <godot_cpp/classes/global_constants.hpp>
#include <godot_cpp/classes/resource.hpp>
#include <godot_cpp/classes/viewport.hpp>

#include <godot_cpp/core/binder_common.hpp>

using namespace godot;

class ExampleRef : public RefCounted {
GDCLASS(ExampleRef, RefCounted);
class ExampleRef : public Resource {
GDCLASS(ExampleRef, Resource);

private:
static uint64_t instance_count; // just so we can test if this comes back to 0 at the end of our test

int64_t value;

protected:
static void _bind_methods() {}
static void _bind_methods();

public:
ExampleRef();
~ExampleRef();

void set_value(int64_t p_value);
int64_t get_value() const;
};

class Example : public Control {
Expand All @@ -64,6 +73,7 @@ class Example : public Control {

private:
Vector2 custom_position;
Ref<ExampleRef> ref_obj;

public:
// Constants.
Expand All @@ -84,7 +94,7 @@ class Example : public Control {
void simple_const_func() const;
String return_something(const String &base);
Viewport *return_something_const() const;
ExampleRef *return_extended_ref() const;
Ref<ExampleRef> return_extended_ref() const;
Ref<ExampleRef> extended_ref_checks(Ref<ExampleRef> p_ref) const;
Variant varargs_func(const Variant **args, GDNativeInt arg_count, GDNativeCallError &error);
void emit_custom_signal(const String &name, int value);
Expand All @@ -96,6 +106,9 @@ class Example : public Control {
void set_custom_position(const Vector2 &pos);
Vector2 get_custom_position() const;

void set_ref_obj(const Ref<ExampleRef> p_ref);
Ref<ExampleRef> get_ref_obj() const;

// Virtual function override (no need to bind manually).
virtual bool _has_point(const Vector2 &point) const override;
};
Expand Down