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

GDScript: Rework GDScriptUtilityFunctions macros #84368

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

dalexeev
Copy link
Member

@dalexeev dalexeev commented Nov 2, 2023

  1. Rework macros for validating count and types of arguments. This makes the code easier to read and reduces the likelihood of accidental bugs. This also fixes a bug with implicit conversions don't work for some utility functions.
  2. Rework function registration macros. Instead of several REGISTER_FUNC_* macros for different signature variations, there is now one REGISTER_FUNC macro and several shorthands for PropertyInfo/MethodInfo.

Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

The side-effect of an actual enum for convert's argument is nice, but that's all of the input I can give.

Copy link
Member

@adamscott adamscott left a comment

Choose a reason for hiding this comment

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

Seems ok, and seems to remove a lot of boilerplate code. I think that if the tests pass, it's a good sign to merge.

@dalexeev dalexeev force-pushed the gds-rework-util-funcs-macros branch from 827c915 to 4dc5688 Compare October 25, 2024 14:46
@Repiteo Repiteo merged commit 955c056 into godotengine:master Oct 25, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Oct 25, 2024

Thanks!

@dalexeev dalexeev deleted the gds-rework-util-funcs-macros branch October 25, 2024 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants