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

Add BoneAttachment3D.get_external_skeleton_path, deprecate get_external_skeleton #96649

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Flynsarmy
Copy link
Contributor

@Flynsarmy Flynsarmy requested review from a team as code owners September 6, 2024 13:15
@AThousandShips AThousandShips changed the title Add BoneAttachment3D get_external_skeleton_path(), deprecate get_exte… Add BoneAttachment3D.get_external_skeleton_path, deprecate get_external_skeleton Sep 6, 2024
@AThousandShips AThousandShips added this to the 4.x milestone Sep 6, 2024
@AThousandShips
Copy link
Member

Why only the get? Why not set_external_skeleton_path? This makes things more confusing

@Flynsarmy
Copy link
Contributor Author

You're absolutely right! I'll update the setter too.

@Flynsarmy Flynsarmy force-pushed the get_external_skeleton_path branch 2 times, most recently from d72e59b to c9c7d67 Compare September 6, 2024 14:10
@fire fire requested a review from a team September 6, 2024 19:31
@fire
Copy link
Member

fire commented Sep 6, 2024

@TokageItLab I think this is ok, but I may have missed a conceptual problem. Can you review?

scene/3d/bone_attachment_3d.cpp Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Pending, No consensus
Development

Successfully merging this pull request may close these issues.

Rename BoneAttachment3D::get_external_skeleton() to get_external_skeleton_path()
4 participants