Question regarding proper memory management of CollisonShape3D's Shape3D resource in godot-cpp GDExtension

[NonGreasyRelief]

Godot Version

4.6.1

Question

I’ve been creating a character controller in godot-cpp GDExtension that extends CharacterBody3D. This section of code for creating my collision shape has been giving me trouble. I was under the impression anything that extends resource/refcounted should be instantiated as a Ref like this:

Ref<Class> objectname = memnew(Class);

However, when I try and instantiate my CylinderShape3D for my CollisionShape3D this way, my crouch function later segfaults when I try and access the get_height(); function. However, everything works correctly when I instantiate the CylinderShape3D like this:

CylinderShape3D* objectname = memnew(Class);

Here’s a snip from my _ready() function on my character controller where I am creating my collision shape. I commented out the line that I thought was “correct” but was causing a segfault.

    _PlayerCollision = memnew ( CollisionShape3D );
    _PlayerCollision -> set_name ( "PlayerCollision" );
    add_child ( _PlayerCollision );
    _PlayerCollision -> set_owner(get_tree()->get_edited_scene_root());

    //working instantiation (Defined as CylinderShape3D* pointer in .h)
    _PlayerCylinder = memnew ( CylinderShape3D );

    //not working instantiation (defined as Ref<CylinderShape3D> in .h)
    //Ref<CylinderShape3D> _PlayerCylinder = memnew ( CylinderShape3D );

    _PlayerCylinder -> set_height ( PLAYER_COLLISION_HEIGHT );
    _PlayerCylinder -> set_radius ( PLAYER_COLLISION_RADIUS );
    _PlayerCollision -> set_shape ( _PlayerCylinder );

And here’s the part of the crouch function causing a segfault when instantiated as a Ref<> but not as a pointer like a node, the crouch function is called once per _physics_process().

	_PlayerCylinder->set_height(m_player_cylinder_height); 

I think way I am doing it is bypassing the reference count and instantiating it like a node. Am I creating a small memory leak when the CharacterBody3D leaves the scene tree? Should I manually call memfree(); in the destructor if I am doing this?

Why could my Ref<> be producing a nullptr when I call memnew() this way and assign the shape in _ready()? Is it getting garbage collected? I used to call get _PlayerCylinder->get_shape() in _ready() to initialize the Ref when I had the resource as a tscn file, and now I’m trying to instantiate it from code.

EDIT:
After much screwing around, I came to the conclusion that the Ref<> was going out of scope at the end of _Ready(), as it should. Here’s how to initialize in _Ready()

	Ref<CylinderShape3D> _PlayerCylinder = memnew ( CylinderShape3D );
	_PlayerCylinder -> set_height ( PLAYER_COLLISION_HEIGHT );
	_PlayerCylinder -> set_radius ( PLAYER_COLLISION_RADIUS );
	_PlayerCollision -> set_shape ( _PlayerCylinder );

and then just make a new Ref in the duck function to reference it

	Ref<CylinderShape3D> m_CylinderShape3D = _PlayerCollision -> get_shape ();
	m_CylinderShape3D -> set_height ( m_player_cylinder_height );

Feel free to close this thread.

[silnarm]

Sounds like you have things figured out now anyway, but just for the sake of clarity, this one:

@NonGreasyRelief

//Ref<CylinderShape3D> _PlayerCylinder = memnew ( CylinderShape3D );

did not work (and indeed went out of scope) because you declared the Ref as a local variable here, shadowing the one you declared in the header. The member variable _PlayerCyclinder was never assigned anything, so _PlayerCylinder->set_height(m_player_cylinder_height); later was always going to crash you out.

In _ready() you should been doing either:
_PlayerCylinder.instantiate(); – ok for CylinderShape3D, and anything else registered in ClassDB
or,
_PlayerCylinder = Ref<CylinderShape3D>(memnew(CylinderShape3D)); – ok with any RefCounted

Which would be the member variable you declared in the header, and would therefore have kept the referenced CyclinderShape3D alive as long as the CharacterBody.

[NonGreasyRelief]

Oh that was my problem! I see so I was shadowing the member variable by putting the type before it, thereby creating a new one in local scope instead of assigning the one I created. I don’t know how I didn’t realize that was what was happening, but that makes perfect sense. I am going to go with your second suggestion and go back to creating the member variable in my .h, and assigning it with the proper syntax instead of creating a local one with the same name. Thank you so much for explaining this.