godotengine / godotengine/godot-cpp

[WIP] Summarise current feature state (especially` Ref`) vs app/branch versions

Open
#417 4 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
C++
Stars
2.7k
Forks
809
Avg merge
1d 3h
Merged PRs (30d)
8

Description

----

**TL;DR:**

* After ~ 9eceb16f0553884094d0f659461649be5d333866 *do* use:

* `Ref ref = Ref(custom_class_instance);`

` `

and *don't* use:

* ~`Ref ref = Ref ref::__internal_constructor(cus_class_inst);`~

` `

If you use `__internal_constructor()` directly--since the "extra reference" bug has been fixed--then there won't be a reference to keep the object/reference alive so it'll be deleted (either immediately, or when it gets passed back to GDScript--I can't remember which).

This may result in a crash or Godot complaining in a manner similar to this:

```
[Object:0]
SCRIPT ERROR: _init: Invalid call. Nonexistent function '' in base 'previously freed instance'.
```

----

### Details / testing

(This would make more sense as a wiki but seems it's not configured for public/logged in editing?

godot-cpp ref vs Godot Version: | 3.0.0 | 3.1.0 | 3.1.2 | 3.2.0 | 3.2.1 | 3.2.2
------------ | -----|-------|----|-----|-----|----
3.0 [ref]() | | | | | | |
3.1 [ref]() | | | | | | |
3.2 [ref]() | | | | | | |
master [ref](https://github.com/godotengine/godot-cpp/tree/9eceb16f0553884094d0f659461649be5d333866) | | [A]* ~[B]~ | [A]* ~[B]~ | [A] [B] | [A] [B] | |

Features to test:

* `Ref`
* `Pool*Array`
* How to use `Ref`
* Lack of memory leaks. :D

*Update:* [A]* It turns out 9eceb16f0553884094d0f659461649be5d333866 *does* work with 3.1.x--the error I was seeing was...because the `test.gd` file couldn't be found because the "current path" CLI calculation has apparently changed... Not that the error message indicated the issue was "file not found". :D (Note that `PoolByteArray` doesn't have a `hex_encode()` method in 3.1 and as it also has no `call()` method I don't know of a backwards-compatible way of conditionally calling it.)

### Example [A] @ 9eceb16f0553884094d0f659461649be5d333866

```cpp
# Foreigner.h

# ...

# ...
```

```cpp
# Foreigner.cpp

# ...

Ref Foreigner::new_buffer(uint32_t size_in_bytes) {

ForeignBuffer *the_buffer = ForeignBuffer::_new();

#...

Ref ref = Ref(the_buffer);
return ref;
}
# ...
```

### Example [B] @ 9eceb16f0553884094d0f659461649be5d333866

Note: This was my initial test code.

Although this approach works with `__internal_constructor()` (because we hold onto the object & the reference ourselves when returning it to GDScript) this approach is now no longer necessary and the simpler approach used in Example [A] can be used.

(There may be some benefit for source code compatibility to staying with this approach until the extra reference bug fix is backported to 3.1 `godot-cpp` branch.)

```cpp
# Foreigner.h

# ...

class Foreigner : public Reference {
GODOT_CLASS(Foreigner, Reference)

ForeignLibrary *library;
Ref ref;

# ...

Ref open(String path);

#...
```

```cpp
# Foreigner.cpp

Ref Foreigner::open(String path) {

#...

library = ForeignLibrary::_new();
ref = Ref::__internal_constructor(library);

# ...

return ref;
}

#...

Foreigner::~Foreigner() {
// ref.free(); // <--- No!
//ref = nullptr; //<--- No!
ref.unref(); // <--- Yes!
////library->free(); // <--- No!
library = nullptr; // <--- Yes!
}

```

* So far [B] with the above implementation approach appears to pass custom class references without crashing and/or leaking memory compiled with current master `godot-cpp` on Mac, test good with 3.2.0-3.2.1 but does *not load* 3.1.0-3.1.2.

### Example [C]

*[WIP: TODO]*

If upgrading to a later `godot-cpp` version is not an option for you, other approaches are:

* Use a `godot-cpp` release which leaks memory but doesn't crash.
* Split the `_new()` call & `Ref<>` creation into two parts & try storing a reference to the `Object` and/or `Reference` in your own class. (As used in Example [B] above.)
* Attempt to back-port the later fixes to earlier `godot-cpp` version.

### Related PRs

* Merged:
* https://github.com/godotengine/godot-cpp/pull/408
* https://github.com/godotengine/godot-cpp/pull/356
* https://github.com/godotengine/godot-cpp/pull/355
* https://github.com/godotengine/godot-cpp/pull/333

* Unmerged:
* https://github.com/godotengine/godot-cpp/pull/401
* https://github.com/godotengine/godot-cpp/pull/307
* https://github.com/godotengine/godot-cpp/pull/350
* https://github.com/godotengine/godot/pull/33532
* https://github.com/GodotNativeTools/godot_headers/pull/60
* https://github.com/godotengine/godot-cpp/pull/346

### Related Issues

* https://github.com/godotengine/godot-cpp/issues/275 https://github.com/godotengine/godot-cpp/issues/276 https://github.com/godotengine/godot-cpp/issues/274 https://github.com/godotengine/godot-cpp/issues/281 https://github.com/godotengine/godot-cpp/issues/322 https://github.com/godotengine/godot-cpp/issues/215 https://github.com/godotengine/godot-cpp/issues/343 https://github.com/godotengine/godot-cpp/issues/253

*[WIP]*

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.