godot-cpp icon indicating copy to clipboard operation
godot-cpp copied to clipboard

Passing `null` for `RefCounted` argument from GDScript prints error (though the function runs without problems)

Open Maximilian-Seitz opened this issue 1 year ago • 1 comments

Godot version

v4.4.dev3.official (f4af820)

godot-cpp version

4.4.dev (6facde3)

System information

Ubuntu 22.04.5 LTS 22.04 - X11 - Vulkan (Forward+)

Issue description

A GDExtension function with a RefCounted parameter (using Ref), which may be null, works fine from within GDExtension, but sometimes prints an error when passing null from GDScript.

The functionality remains in all cases, except for the printed error in one corner-case.

The error is: Parameter "p_ptr" is null. from godot-cpp/include/godot_cpp/classes/ref.hpp:233 @ convert()

Steps to reproduce

The exact problem occurs when a variable typed as some RefCounted type is passed and this variable holds null. When the variable is un-typed, null is passed directly, or a valid object is passed, no error is printed.

GDScript example:

extends Node

enum Mode {
	TYPED_INPUT,
	UNTYPED_INPUT,
	LITERAL_INPUT,
	OBJECT_INPUT
}

@export var mode: Mode = Mode.TYPED_INPUT

func _ready() -> void:
	var typed_variable: Texture2D = null
	
	match mode:
		Mode.TYPED_INPUT:
			# Prints error, then 'true'
			print(Test.is_null(typed_variable))
		
		Mode.UNTYPED_INPUT:
			# Prints 'true'
			var untyped_variable = typed_variable
			print(Test.is_null(untyped_variable))
		
		Mode.LITERAL_INPUT:
			# Prints 'true'
			print(Test.is_null(null))
		
		Mode.OBJECT_INPUT:
			# Prints 'false'
			typed_variable = ImageTexture.new()
			print(Test.is_null(typed_variable))

GDExtension example:

#include <godot_cpp/core/class_db.hpp>
#include <godot_cpp/classes/ref_counted.hpp>
#include <godot_cpp/classes/texture2d.hpp>

namespace godot {

class Test : public RefCounted {
	GDCLASS(Test, RefCounted)

	protected:

	static void _bind_methods() {
		ClassDB::bind_static_method("Test", D_METHOD("is_null", "value"), &Test::is_null);
	}

	public:

	Test() = default;

	static bool is_null(Ref<Texture2D> value) {
		return value.is_null();
	}

};

}

Minimal reproduction project

Note: godot-cpp folder is empty in minimal reproduction project, and needs to be fetched (as to not upload too much junk)

Project.zip

Maximilian-Seitz avatar Oct 06 '24 12:10 Maximilian-Seitz

Hm, yeah, I agree we shouldn't print an error message for this - it's a valid case.

I just pushed PR https://github.com/godotengine/godot-cpp/pull/1616 which I think should fix this (I haven't had a chance to test it yet).

dsnopek avatar Oct 07 '24 16:10 dsnopek