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

Fix properties still using a slash delimiter #44876

Closed
wants to merge 2 commits into from

Conversation

madmiraal
Copy link
Contributor

@madmiraal madmiraal commented Jan 2, 2021

As identified in #17558, there are a number of properties that are still using a slash / as a delimiter (Godot 2 style). Many of these properties appear in the inspector, but are difficult to access via scripts, and, in general, are not documented. However, most importantly, it's not even currently possible to create standard getters and setters for them.

This PR converts all(?) of the remaining properties that still use the old slash delimiter to use the underscore delimiter _, and includes updating some of the relevant documentation. It has been divided into two commits:

  1. Properties that already use the ADD_PROPERTY macros: see List of properties still using a slash as delimiter (Godot 2 style) #17558 (comment)
  2. Properties that are built via code: see List of properties still using a slash as delimiter (Godot 2 style) #17558 (comment)

They are best reviewed separately, because they use different approaches. The first renames properties, but also sometimes renames the getters and setters and includes the necessary documentation updates. The second also involves creating the necessary groupings for the inspector and also updating the old style getters and setters.

Closes #17558.
Prerequisite for godotengine/godot-proposals#2000 and for creating standard getters and setters for the other properties identified in #17558 (comment).

@madmiraal
Copy link
Contributor Author

Rebased following merge of #37547, #45549 and #45564.

@madmiraal
Copy link
Contributor Author

Rebased following merge of #45845.

@reduz
Copy link
Member

reduz commented Feb 13, 2021

I honestly don't mind the slash being used in a lot of places, specially when the properties exposed are dynamic or when accessed via index, and it's not a problem that GDScript can't access those via properties (and have to use functions instead). I find it myself to be a lot cleaner this way.

That said a few of those could be converted to properties (when not being dynamically exported or not indexed), but IMO this is not a priority right now, will check this PR again when closer to beta and see which parts could be of use.

@madmiraal
Copy link
Contributor Author

Rebased following merge of #42974 and #47277.

@madmiraal madmiraal requested a review from a team as a code owner March 27, 2021 11:31
@madmiraal
Copy link
Contributor Author

Rebased following merge of #47166.

@madmiraal
Copy link
Contributor Author

Rebased following merge of #47252.

@madmiraal
Copy link
Contributor Author

Rebased following merge of #47642.

@madmiraal
Copy link
Contributor Author

Rebased following merge of #47878 and #47915.

@madmiraal
Copy link
Contributor Author

Rebased following merge of #34823 and #47485.

@akien-mga
Copy link
Member

While reviewing #64428 I decided to have a go at solving some of #17558 and omitted to re-review this PR.

So I made #64719 which partially supersedes this PR for the case of "normal" properties which were just wrongly using a slash.
Since the engine evolved I could also solve some issues more elegantly (like using the radians hint instead of all those intermediate methods for Joint3D angular limits).

For properties which are constructed (typically using an int section, or using a set_param method) and not meant to be access from script directly, the consensus for now seems to be to leave those as is.

Then this PR still had relevant changes to:

@akien-mga
Copy link
Member

Closing as superseded by #64719, and this is not in a mergeable state.

Some changes in this PR could still be worth redoing in a new PR, as pointed out in my previous comment.

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.

List of properties still using a slash as delimiter (Godot 2 style)
3 participants