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

Expose get_skeleton() from SkeletonModifier3D #92896

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

TokageItLab
Copy link
Member

Follow up #91507.

To create a custom SkeletonModifier, a method to safely retrieve the Skeleton was lacked for gdscript and extention.

@TokageItLab TokageItLab force-pushed the skeleton-mod-get-skeleton branch from d81609c to e92a453 Compare June 9, 2024 14:58
Copy link
Contributor

@lyuma lyuma 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 technically.

Get parent [Skeleton3D] node if found.

Based on this documentation, I do not understand why this method is different from get_parent() as Skeleton3D - I don't like having two methods that do the same thing.

Is if we eventually allow descendants of Skeleton3D to be modifiers then this could be useful separate from get_parent

@TokageItLab
Copy link
Member Author

TokageItLab commented Jun 10, 2024

This method is a kind of remnant from the old api when SkeletonModifier3D could not be a child of a Skeleton, but it is useful for safety and readability.

Based on this documentation, I do not understand why this method is different from get_parent() as Skeleton3D - I don't like having two methods that do the same thing.

In practice, it is not equivalent. It retrieve skeleton from cached ObjectID which is found/stored when parenting. But indeed, for performance reasons, it may be better to cache the pointer or make a wrapper function of get_parent() with some flag of the availavility like:

bool is_child_of_skeleton = true;

Skeleton3D *SkeletonModifier3D::get_skeleton() const {
  if (is_child_of_skeleton) {
    return get_parent();
  }
  return nullptr;
}

Well, for now, I would like to expose it as an API to simplify the code during the development of SkeletonModifier3D. I think we can send a follow-up later to improve the performance.

@akien-mga akien-mga merged commit 76b1a1f into godotengine:master Jun 10, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@TokageItLab TokageItLab deleted the skeleton-mod-get-skeleton branch June 29, 2024 11:44
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