Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added property info for Object's and Ref's and ported the implementation of the check method #950

Merged
merged 1 commit into from
Jan 17, 2023
Merged

Added property info for Object's and Ref's and ported the implementation of the check method #950

merged 1 commit into from
Jan 17, 2023

Conversation

DmitriySalnikov
Copy link
Contributor

@DmitriySalnikov DmitriySalnikov commented Dec 6, 2022

Supersedes #810
Rebased and fixed. Also added PropertyInfo for Ref<T>
#948 is required for a successful build

image

Godot_v4 0-beta7_win64_7OvHJneZs2

Constructor.
Notification: 18
Notification: 31
Notification: 32
Notification: 45
Notification: 10
Notification: 27
Notification: 13
Signal bind

To string
  Example -->  Example:[ GDExtension::Example <--> Instance ID:26541556118 ]
  ExampleMin -->  ExampleMin:[Wrapped:0]
Static method calls
  static (109) 109
  void static
Property list
  property value  (100, 200, 300)
Instance method calls
  Simple func called.
  Simple const func called.
  typed_ptr_parameter called: Example:[ GDExtension::Example <--> Instance ID:26541556118 ] <----------------------
  typed_const_ptr_parameter called: Example:[ GDExtension::Example <--> Instance ID:26541556118 ] <----------------
  Return something called.
...

@akien-mga akien-mga added bug This has been identified as a bug enhancement This is an enhancement on the current functionality labels Dec 6, 2022
@akien-mga akien-mga requested a review from a team December 6, 2022 09:14
@DmitriySalnikov
Copy link
Contributor Author

Rebased to master. Now it should build without problems.

@DmitriySalnikov
Copy link
Contributor Author

Do I need to do something else to continue the review?

@WildRackoon
Copy link
Contributor

This is spot on, but I am unsure how we can further reduce code duplication within some of those bindings files that are not generated.

@DmitriySalnikov
Copy link
Contributor Author

DmitriySalnikov commented Dec 18, 2022

Macros can be used here. All 3 VariantCasters can be replaced with such a macro:

#define VARIANT_OBJECT_CAST(impl)                                      \
	template <class T>                                                 \
	struct VariantCaster impl {                                        \
		static _FORCE_INLINE_ T cast(const Variant &p_variant) {       \
			using TStripped = std::remove_pointer_t<T>;                \
			if constexpr (std::is_base_of<Object, TStripped>::value) { \
				return Object::cast_to<TStripped>(p_variant);          \
			} else {                                                   \
				return p_variant;                                      \
			}                                                          \
		}                                                              \
	}

// Will produce a warning. Error when building for Windows with godot-cpp's CI.
VARIANT_OBJECT_CAST();
VARIANT_OBJECT_CAST(<T &>);
VARIANT_OBJECT_CAST(<const T &>);

But there will still be a bunch of duplicated code nearby. I'll leave my changes here as is.
It may be worth creating a separate PR to remove all duplicates.

@DmitriySalnikov DmitriySalnikov changed the title Added support for typed ptr calls and added property info for ptr and ref Added property info for Object's and Ref's and ported the implementation of the check method Jan 11, 2023
@DmitriySalnikov
Copy link
Contributor Author

DmitriySalnikov commented Jan 11, 2023

After the rebase, I removed the examples here, but I think this is not a problem because one of them has already been merged in another PR

Faless
Faless previously requested changes Jan 13, 2023
Copy link
Contributor

@Faless Faless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the changes to use appropriate hint string (as in upstream godot).
The class_name is then copied automatically from the hint_string in make_property_info.

include/godot_cpp/classes/ref.hpp Outdated Show resolved Hide resolved
include/godot_cpp/classes/ref.hpp Outdated Show resolved Hide resolved
@DmitriySalnikov
Copy link
Contributor Author

Done.

@DmitriySalnikov
Copy link
Contributor Author

Do I need to do something else?

@WildRackoon
Copy link
Contributor

I would also really like to see those kind of convenience fixes land in godot-cpp soon

@akien-mga akien-mga merged commit b21026e into godotengine:master Jan 17, 2023
@akien-mga
Copy link
Member

Thanks!

@DmitriySalnikov DmitriySalnikov deleted the typed-ptr-method-call-support-and-ref-class-name branch January 20, 2023 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug enhancement This is an enhancement on the current functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants