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 remaining animation-related class reference docs #82713

Closed
wants to merge 1 commit into from

Conversation

Anutrix
Copy link
Contributor

@Anutrix Anutrix commented Oct 3, 2023

Added remaining animation-related class reference docs.

@Anutrix Anutrix requested a review from a team as a code owner October 3, 2023 07:05
Comment on lines 58 to 60
<member name="_data" type="Dictionary" setter="_set_data" getter="_get_data" default="{}">
Contains a [Dictionary] of [Animation] resources.
</member>
Copy link
Member

@akien-mga akien-mga Oct 3, 2023

Choose a reason for hiding this comment

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

Seems like a bug that a member starting with an underscore is exposed.

Sounds like this was just meant to serialization, not public access?

CC @godotengine/animation

But well now that it's been exposed I'm not sure we can take it back without breaking compat.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be fine. It needs to have its usage changed to PROPERTY_USAGE_NO_EDITOR | PROPERTY_USAGE_INTERNAL. If it needs to be exposed, it needs to be renamed anyway. So we'd be breaking compatibility.

If we do neither, we risk it being accidentally unexposed if we, say, fix the fact that an underscored member is added to the docs (it should be stripped from XMLs by the doctool, the fact that it's not is a bug).

Copy link
Contributor

Choose a reason for hiding this comment

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

I made #84376

@YuriSizov
Copy link
Contributor

Superseded by #84376. Thanks for your contribution nevertheless!

@YuriSizov YuriSizov closed this Nov 6, 2023
@YuriSizov YuriSizov removed this from the 4.2 milestone Nov 6, 2023
@YuriSizov
Copy link
Contributor

Oh wait, this also added a description to AnimationNodeStateMachine::replace_node. Could you open a PR for that?

@Anutrix
Copy link
Contributor Author

Anutrix commented Nov 6, 2023

Oh wait, this also added a description to AnimationNodeStateMachine::replace_node. Could you open a PR for that?

Sure

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.

3 participants