diff --git a/core/variant/method_ptrcall.h b/core/variant/method_ptrcall.h index 569dd892c8e..c108e1a20f7 100644 --- a/core/variant/method_ptrcall.h +++ b/core/variant/method_ptrcall.h @@ -273,7 +273,7 @@ struct PtrToArg { template struct PtrToArg> { - typedef typename RequiredParam::ptr_type EncodeT; + typedef typename RequiredParam::persistent_type EncodeT; _FORCE_INLINE_ static RequiredParam convert(const void *p_ptr) { if (p_ptr == nullptr) { @@ -283,7 +283,7 @@ struct PtrToArg> { } _FORCE_INLINE_ static void encode(const RequiredParam &p_var, void *p_ptr) { - *((typename RequiredParam::ptr_type *)p_ptr) = p_var._internal_ptr_dont_use(); + *((typename RequiredParam::persistent_type *)p_ptr) = p_var._internal_ptr_dont_use(); } }; diff --git a/core/variant/required_ptr.h b/core/variant/required_ptr.h index 3f5154be84f..5efba3c69f6 100644 --- a/core/variant/required_ptr.h +++ b/core/variant/required_ptr.h @@ -153,24 +153,31 @@ class RequiredParam { static_assert(!is_fully_defined_v || std::is_base_of_v, "T must be an Object subtype"); public: + static constexpr bool is_ref = std::is_base_of_v; + using element_type = T; - using ptr_type = std::conditional_t, Ref, T *>; + using extracted_type = std::conditional_t &, T *>; + using persistent_type = std::conditional_t, T *>; private: - ptr_type _value = ptr_type(); + T *_value = nullptr; _FORCE_INLINE_ RequiredParam() = default; public: // These functions should not be called directly, they are only for internal use. - _FORCE_INLINE_ ptr_type _internal_ptr_dont_use() const { return _value; } - _FORCE_INLINE_ bool _is_null_dont_use() const { - if constexpr (std::is_base_of_v) { - return _value.is_null(); + _FORCE_INLINE_ extracted_type _internal_ptr_dont_use() const { + if constexpr (is_ref) { + // Pretend _value is a Ref, for ease of use with existing `const Ref &` accepting APIs. + // This only works as long as Ref is internally T *. + // The double indirection should be optimized away by the compiler. + static_assert(sizeof(Ref) == sizeof(T *)); + return *((const Ref *)&_value); } else { - return _value == nullptr; + return _value; } } + _FORCE_INLINE_ bool _is_null_dont_use() const { return _value == nullptr; } _FORCE_INLINE_ static RequiredParam _err_return_dont_use() { return RequiredParam(); } // Prevent erroneously assigning null values by explicitly removing nullptr constructor/assignment. @@ -202,22 +209,13 @@ public: template , int> = 0> _FORCE_INLINE_ RequiredParam(const Ref &p_ref) : - _value(p_ref) {} + _value(*p_ref) {} template , int> = 0> _FORCE_INLINE_ RequiredParam &operator=(const Ref &p_ref) { - _value = p_ref; + _value = *p_ref; return *this; } - template , int> = 0> - _FORCE_INLINE_ RequiredParam(Ref &&p_ref) : - _value(std::move(p_ref)) {} - template , int> = 0> - _FORCE_INLINE_ RequiredParam &operator=(Ref &&p_ref) { - _value = std::move(p_ref); - return &this; - } - template , int> = 0> _FORCE_INLINE_ RequiredParam(const Variant &p_variant) : _value(static_cast(p_variant.get_validated_object())) {} @@ -242,7 +240,7 @@ public: _err_print_error(FUNCTION_STR, __FILE__, __LINE__, "Required object \"" _STR(m_param) "\" is null.", m_msg, m_editor); \ return m_retval; \ } \ - typename std::decay_t::ptr_type m_name = m_param._internal_ptr_dont_use(); \ + typename std::decay_t::extracted_type m_name = m_param._internal_ptr_dont_use(); \ static_assert(true) // These macros are equivalent to the ERR_FAIL_NULL*() family of macros, only for RequiredParam instead of raw pointers. diff --git a/tests/core/object/test_object.h b/tests/core/object/test_object.h index 1b559a4413d..34941843e15 100644 --- a/tests/core/object/test_object.h +++ b/tests/core/object/test_object.h @@ -596,26 +596,27 @@ TEST_CASE("[Object] Destruction at the end of the call chain is safe") { "Object was tail-deleted without crashes."); } -int required_param_compare(const Ref &p_ref, const RequiredParam &p_required) { - EXTRACT_PARAM_OR_FAIL_V(extract, p_required, false); - ERR_FAIL_COND_V(p_ref->get_reference_count() != extract->get_reference_count(), -1); +int required_param_compare(const Ref &p_ref, const RequiredParam &rp_required) { + EXTRACT_PARAM_OR_FAIL_V(p_required, rp_required, false); + ERR_FAIL_COND_V(p_ref->get_reference_count() != p_required->get_reference_count(), -1); return p_ref->get_reference_count(); } TEST_CASE("[Object] RequiredParam Ref") { Ref ref; ref.instantiate(); + const Ref &ref_ref = ref; RequiredParam required = ref; EXTRACT_PARAM_OR_FAIL(extract, required); - static_assert(std::is_same_v); + static_assert(std::is_same_v); CHECK_EQ(ref->get_reference_count(), extract->get_reference_count()); const int count = required_param_compare(ref, ref); CHECK_NE(count, -1); - CHECK_NE(count, ref->get_reference_count()); + CHECK_EQ(count, ref->get_reference_count()); CHECK_EQ(ref->get_reference_count(), extract->get_reference_count()); } diff --git a/tests/scene/test_instance_placeholder.h b/tests/scene/test_instance_placeholder.h index aa29c22816a..2c06093b37c 100644 --- a/tests/scene/test_instance_placeholder.h +++ b/tests/scene/test_instance_placeholder.h @@ -110,7 +110,7 @@ TEST_CASE("[SceneTree][InstancePlaceholder] Instantiate from placeholder with no scene->set_int_property(12); // Pack the scene. - PackedScene *packed_scene = memnew(PackedScene); + Ref packed_scene = memnew(PackedScene); const Error err = packed_scene->pack(scene); REQUIRE(err == OK); @@ -138,7 +138,7 @@ TEST_CASE("[SceneTree][InstancePlaceholder] Instantiate from placeholder with no referenced->set_owner(scene); scene->set_reference_property(referenced); // Pack the scene. - PackedScene *packed_scene = memnew(PackedScene); + Ref packed_scene = memnew(PackedScene); const Error err = packed_scene->pack(scene); REQUIRE(err == OK); @@ -175,7 +175,7 @@ TEST_CASE("[SceneTree][InstancePlaceholder] Instantiate from placeholder with no node_array.push_back(referenced2); scene->set_reference_array_property(node_array); // Pack the scene. - PackedScene *packed_scene = memnew(PackedScene); + Ref packed_scene = memnew(PackedScene); const Error err = packed_scene->pack(scene); REQUIRE(err == OK); @@ -219,7 +219,7 @@ TEST_CASE("[SceneTree][InstancePlaceholder] Instantiate from placeholder with ov scene->set_int_property(12); // Pack the scene. - PackedScene *packed_scene = memnew(PackedScene); + Ref packed_scene = memnew(PackedScene); packed_scene->pack(scene); // Instantiate the scene. @@ -248,7 +248,7 @@ TEST_CASE("[SceneTree][InstancePlaceholder] Instantiate from placeholder with ov referenced->set_owner(scene); scene->set_reference_property(referenced); // Pack the scene. - PackedScene *packed_scene = memnew(PackedScene); + Ref packed_scene = memnew(PackedScene); const Error err = packed_scene->pack(scene); REQUIRE(err == OK); @@ -303,7 +303,7 @@ TEST_CASE("[SceneTree][InstancePlaceholder] Instantiate from placeholder with ov scene->set_reference_array_property(referenced_array); // Pack the scene. - PackedScene *packed_scene = memnew(PackedScene); + Ref packed_scene = memnew(PackedScene); const Error err = packed_scene->pack(scene); REQUIRE(err == OK); @@ -346,7 +346,7 @@ TEST_CASE("[SceneTree][InstancePlaceholder] Instance a PackedScene containing an internal->set_reference_property(referenced); // Pack the internal scene. - PackedScene *internal_scene = memnew(PackedScene); + Ref internal_scene = memnew(PackedScene); Error err = internal_scene->pack(internal); REQUIRE(err == OK); @@ -375,7 +375,7 @@ TEST_CASE("[SceneTree][InstancePlaceholder] Instance a PackedScene containing an internal_created->set("reference_property", NodePath("OriginalReference")); // Pack the main scene. - PackedScene *main_scene = memnew(PackedScene); + Ref main_scene = memnew(PackedScene); err = main_scene->pack(root); REQUIRE(err == OK); @@ -435,7 +435,7 @@ TEST_CASE("[SceneTree][InstancePlaceholder] Instance a PackedScene containing an internal->set_reference_array_property(referenced_array); // Pack the internal scene. - PackedScene *internal_scene = memnew(PackedScene); + Ref internal_scene = memnew(PackedScene); Error err = internal_scene->pack(internal); REQUIRE(err == OK); @@ -476,7 +476,7 @@ TEST_CASE("[SceneTree][InstancePlaceholder] Instance a PackedScene containing an internal_created->set_reference_array_property(override_array); // Pack the main scene. - PackedScene *main_scene = memnew(PackedScene); + Ref main_scene = memnew(PackedScene); err = main_scene->pack(root); REQUIRE(err == OK);