Skip to content

Commit fd91042

Browse files
BastiaanOlijBastiaanOlij
committed
Fixing reference return
1 parent f58a2f2 commit fd91042

File tree

5 files changed

+91
-16
lines changed

5 files changed

+91
-16
lines changed

‎include/godot_cpp/classes/ref.hpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,12 @@ class Ref {
112112
}
113113

114114
operator Variant() const {
115+
if (reference != nullptr) {
116+
// When assigning a variant this way it's not increasing the refcount like it should.
117+
// Discussion currently going on if this shouldn't be handle by VariantInternal::set in godot itself.
118+
reference->reference();
119+
}
120+
115121
return Variant(reference);
116122
}
117123

@@ -186,6 +192,14 @@ class Ref {
186192
r.reference = nullptr;
187193
}
188194

195+
template <class T_Other>
196+
Ref(Ref<T_Other> &&p_other) {
197+
// Should just be able to swap these...
198+
T *swap = (T *)p_other.reference;
199+
p_other.reference = (T_Other *)reference;
200+
reference = swap;
201+
}
202+
189203
Ref(T *p_reference) {
190204
if (p_reference) {
191205
ref_pointer(p_reference);

‎test/demo/main.gd‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,12 @@ func _ready():
99
($Example as Example).simple_const_func() # Force use of ptrcall
1010
prints("returned", $Example.return_something("some string"))
1111
prints("returned const", $Example.return_something_const())
12-
prints("returned ref", $Example.return_extended_ref())
12+
13+
prints("returned example ref object: ", $Example.return_extended_ref())
14+
1315
var ref = ExampleRef.new()
1416
prints("sending ref: ", ref.get_instance_id(), "returned ref: ", $Example.extended_ref_checks(ref).get_instance_id())
17+
1518
prints("vararg args", $Example.varargs_func("some", "arguments", "to", "test"))
1619

1720
prints("test array", $Example.test_array())

‎test/demo/main.tscn‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,25 @@
1-
[gd_scene load_steps=2 format=3 uid="uid://dmx2xuigcpvt4"]
1+
[gd_scene load_steps=3 format=3 uid="uid://dmx2xuigcpvt4"]
22

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

5+
[sub_resource type="ExampleRef" id="ExampleRef_gveeo"]
6+
value = 5
7+
58
[node name="Node" type="Node"]
69
script = ExtResource( "1_c326s" )
710

811
[node name="Example" type="Example" parent="."]
12+
ref_obj = SubResource( "ExampleRef_gveeo" )
13+
__meta__ = {
14+
"_edit_use_anchors_": false
15+
}
916

1017
[node name="Label" type="Label" parent="Example"]
1118
offset_left = 194.0
1219
offset_top = -2.0
1320
offset_right = 234.0
1421
offset_bottom = 21.0
22+
theme_override_font_sizes/font_size = 16
1523
__meta__ = {
1624
"_edit_use_anchors_": false
1725
}

‎test/src/example.cpp‎

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,22 +38,43 @@
3838

3939
using namespace godot;
4040

41+
uint64_t ExampleRef::instance_count = 0;
42+
43+
void ExampleRef::_bind_methods() {
44+
ClassDB::bind_method(D_METHOD("get_value"), &ExampleRef::get_value);
45+
ClassDB::bind_method(D_METHOD("set_value", "value"), &ExampleRef::set_value);
46+
ADD_PROPERTY(PropertyInfo(Variant::INT, "value"), "set_value", "get_value");
47+
}
48+
4149
ExampleRef::ExampleRef() {
42-
UtilityFunctions::print("ExampleRef created.");
50+
instance_count++;
51+
UtilityFunctions::print("ExampleRef created. Our total instance count is now: ", instance_count);
52+
53+
// default this
54+
value = 1;
4355
}
4456

4557
ExampleRef::~ExampleRef() {
46-
UtilityFunctions::print("ExampleRef destroyed.");
58+
instance_count--;
59+
UtilityFunctions::print("ExampleRef destroyed. Our total instance count is now: ", instance_count);
60+
}
61+
62+
void ExampleRef::set_value(int64_t p_value) {
63+
value = p_value;
64+
}
65+
66+
int64_t ExampleRef::get_value() const {
67+
return value;
4768
}
4869

4970
void Example::_bind_methods() {
5071
// Methods.
5172
ClassDB::bind_method(D_METHOD("simple_func"), &Example::simple_func);
5273
ClassDB::bind_method(D_METHOD("simple_const_func"), &Example::simple_const_func);
53-
ClassDB::bind_method(D_METHOD("return_something"), &Example::return_something);
74+
ClassDB::bind_method(D_METHOD("return_something", "base"), &Example::return_something);
5475
ClassDB::bind_method(D_METHOD("return_something_const"), &Example::return_something_const);
5576
ClassDB::bind_method(D_METHOD("return_extended_ref"), &Example::return_extended_ref);
56-
ClassDB::bind_method(D_METHOD("extended_ref_checks"), &Example::extended_ref_checks);
77+
ClassDB::bind_method(D_METHOD("extended_ref_checks", "ref"), &Example::extended_ref_checks);
5778

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

97+
ClassDB::bind_method(D_METHOD("get_ref_obj"), &Example::get_ref_obj);
98+
ClassDB::bind_method(D_METHOD("set_ref_obj", "ref_obj"), &Example::set_ref_obj);
99+
ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "ref_obj", PROPERTY_HINT_RESOURCE_TYPE, "ExampleRef"), "set_ref_obj", "get_ref_obj");
100+
76101
// Signals.
77102
ADD_SIGNAL(MethodInfo("custom_signal", PropertyInfo(Variant::STRING, "name"), PropertyInfo(Variant::INT, "value")));
78103
ClassDB::bind_method(D_METHOD("emit_custom_signal", "name", "value"), &Example::emit_custom_signal);
@@ -85,11 +110,11 @@ void Example::_bind_methods() {
85110
}
86111

87112
Example::Example() {
88-
UtilityFunctions::print("Constructor.");
113+
UtilityFunctions::print("Example Constructor.");
89114
}
90115

91116
Example::~Example() {
92-
UtilityFunctions::print("Destructor.");
117+
UtilityFunctions::print("Example Destructor.");
93118
}
94119

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

118-
ExampleRef *Example::return_extended_ref() const {
119-
return memnew(ExampleRef());
143+
Ref<ExampleRef> Example::return_extended_ref() const {
144+
// 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.
145+
// We should never instantiate the object directly such as this:
146+
// return memnew(ExampleRef());
147+
148+
Ref<ExampleRef> ref;
149+
ref.instantiate();
150+
return ref;
120151
}
121152

122153
Ref<ExampleRef> Example::extended_ref_checks(Ref<ExampleRef> p_ref) const {
123154
Ref<ExampleRef> ref;
124155
ref.instantiate();
125-
// TODO the returned value gets dereferenced too early and return a null object otherwise.
126-
ref->reference();
127156
UtilityFunctions::print("Example ref checks called with value: ", p_ref->get_instance_id(), ", returning value: ", ref->get_instance_id());
128157
return ref;
129158
}
@@ -165,6 +194,14 @@ Vector2 Example::get_custom_position() const {
165194
return custom_position;
166195
}
167196

197+
void Example::set_ref_obj(const Ref<ExampleRef> p_ref) {
198+
ref_obj = p_ref;
199+
}
200+
201+
Ref<ExampleRef> Example::get_ref_obj() const {
202+
return ref_obj;
203+
}
204+
168205
// Virtual function override.
169206
bool Example::_has_point(const Vector2 &point) const {
170207
Label *label = get_node<Label>("Label");

‎test/src/example.h‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,21 +39,30 @@
3939

4040
#include <godot_cpp/classes/control.hpp>
4141
#include <godot_cpp/classes/global_constants.hpp>
42+
#include <godot_cpp/classes/resource.hpp>
4243
#include <godot_cpp/classes/viewport.hpp>
4344

4445
#include <godot_cpp/core/binder_common.hpp>
4546

4647
using namespace godot;
4748

48-
class ExampleRef : public RefCounted {
49-
GDCLASS(ExampleRef, RefCounted);
49+
class ExampleRef : public Resource {
50+
GDCLASS(ExampleRef, Resource);
51+
52+
private:
53+
static uint64_t instance_count; // just so we can test if this comes back to 0 at the end of our test
54+
55+
int64_t value;
5056

5157
protected:
52-
static void _bind_methods() {}
58+
static void _bind_methods();
5359

5460
public:
5561
ExampleRef();
5662
~ExampleRef();
63+
64+
void set_value(int64_t p_value);
65+
int64_t get_value() const;
5766
};
5867

5968
class Example : public Control {
@@ -64,6 +73,7 @@ class Example : public Control {
6473

6574
private:
6675
Vector2 custom_position;
76+
Ref<ExampleRef> ref_obj;
6777

6878
public:
6979
// Constants.
@@ -84,7 +94,7 @@ class Example : public Control {
8494
void simple_const_func() const;
8595
String return_something(const String &base);
8696
Viewport *return_something_const() const;
87-
ExampleRef *return_extended_ref() const;
97+
Ref<ExampleRef> return_extended_ref() const;
8898
Ref<ExampleRef> extended_ref_checks(Ref<ExampleRef> p_ref) const;
8999
Variant varargs_func(const Variant **args, GDNativeInt arg_count, GDNativeCallError &error);
90100
void emit_custom_signal(const String &name, int value);
@@ -96,6 +106,9 @@ class Example : public Control {
96106
void set_custom_position(const Vector2 &pos);
97107
Vector2 get_custom_position() const;
98108

109+
void set_ref_obj(const Ref<ExampleRef> p_ref);
110+
Ref<ExampleRef> get_ref_obj() const;
111+
99112
// Virtual function override (no need to bind manually).
100113
virtual bool _has_point(const Vector2 &point) const override;
101114
};

0 commit comments

Comments
 (0)