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

[3.x] SkeletonEditorPlugin backport #39796

Conversation

TwistedTwigleg
Copy link
Contributor

@TwistedTwigleg TwistedTwigleg commented Jun 24, 2020

This PR is a backport of the SkeletonEditorPlugin that @fire created, with the modifications I added for Godot 4.0. I tested and everything seems to be working like it does in the Godot 4.0/master version.

Closes #35652 and supersedes #39090
Edit: Fixed the linked issue. I accidentally had the wrong one.

@fire
Copy link
Member

fire commented Jun 24, 2020

	// Create Top Menu Bar
	options = memnew(MenuButton);
	SpatialEditor::get_singleton()->add_control_to_menu_panel(options);

	options->set_text(TTR("Skeleton"));
	options->set_icon(EditorNode::get_singleton()->get_gui_base()->get_icon("Skeleton", "EditorIcons"));

	options->get_popup()->add_item(TTR("Create physical skeleton"), MENU_OPTION_CREATE_PHYSICAL_SKELETON);

	options->get_popup()->connect("id_pressed", this, "_on_click_option");
	options->hide();

Removing the hide was patched in #39543

@TwistedTwigleg TwistedTwigleg force-pushed the 3.2.skeleton_inspector_backport branch from c1d4845 to abc9814 Compare June 24, 2020 15:48
@TwistedTwigleg
Copy link
Contributor Author

Removing the hide was patched in #39543

Fixed!

@mbrlabs
Copy link
Contributor

mbrlabs commented Sep 25, 2020

What's the status on this? Now that 3.2.3 is out can this be merged now?

@akien-mga
Copy link
Member

If any of the skeleton-involved coredevs can have a look at this before Halloween, that's be great ;)

@TwistedTwigleg Needs a rebase to solve merge conflicts.
I'd also suggest amending the last commit's message to use a more impersonal style (instead of "Backported the improvements to the SkeletonInspector that I made in my Godot 4.0 PR", it should be something like "Backport SkeletonInspector improvements from #39353" (if that's the one). That's more concise and more informative. It's still fine to say "I" in commit logs but this pertains more to in-depth explanations in the body of the commit message than its title.

@fire
Copy link
Member

fire commented Oct 26, 2020

Reviewing the code:

emit_signal("skeleton_updated");

Should be done like

emit_signal(SceneStringNames::get_singleton()->pose_updated);

But it's not this pr.

@fire fire force-pushed the 3.2.skeleton_inspector_backport branch from abc9814 to 1241259 Compare October 26, 2020 13:59
@fire
Copy link
Member

fire commented Oct 26, 2020

Re-based and merged in the upstream emit_signal("skeleton_updated"); change.

Renamed commit to Backport SkeletonInspector improvements from 39353.

Waiting on cicd.

@TwistedTwigleg
Copy link
Contributor Author

Thanks @fire for rebasing (and renaming) this PR! I am currently working through the rest of my PRs and rebasing them with master 👍

@RevoluPowered
Copy link
Contributor

Can you please make sure to co-author @marstaik?

I needed this plugin today while working, I will ask in IRC if this can be prioritised

@TwistedTwigleg
Copy link
Contributor Author

Sure, though I have no idea how to add authors or co-authors... 😅

@TwistedTwigleg TwistedTwigleg requested a review from a team as a code owner March 12, 2021 12:26
Base automatically changed from 3.2 to 3.x March 16, 2021 11:11
@aaronfranke aaronfranke modified the milestones: 3.2, 3.3 Mar 16, 2021
@akien-mga akien-mga modified the milestones: 3.3, 3.4 Mar 26, 2021
@akien-mga akien-mga changed the title [3.2] SkeletonEditorPlugin backport [3.x] SkeletonEditorPlugin backport Mar 26, 2021
@Chaosus Chaosus modified the milestones: 3.4, 3.5 Nov 8, 2021
@Zireael07
Copy link
Contributor

poke I just realized this hasn't made it into 3.4, when will it be in?

@TwistedTwigleg
Copy link
Contributor Author

I'm not sure, I honestly forgot I had this PR.

One thing with this PR that I'm not sure on, and may be an issue with merging it, is how old the code is compared to the skeleton inspector code in Godot 4.0. This PR is based on what is now pretty old code, since the PR was made during GSOC 2020. I'm not sure how many improvements have been made to the skeleton inspector since, but I imagine this code has several bugs that have been fixed in Godot 4.0. This makes me hesitant to recommend merging this PR. It may be easier and better to see if how difficult it would be to port the Godot 4.0 skeleton inspector to Godot 3.x, so it has the latest fixes and improvements.
(Though I have no idea how much effort or time it would take)

This PR also has some merge conflicts currently, though I took a quick glance and they don't seem too major.

@Zireael07
Copy link
Contributor

Whatever the implementation/form this finally takes, I am very much in need of moving some bones around in a 3.x project... it doesn't need to be perfect at first attempt, in fact I'd happily test with the project I have in mind.

@TwistedTwigleg
Copy link
Contributor Author

This PR only adds the inspector for the bones, I do not think it adds anything for modifying them. I think it might allow for editing the bone child-parent order by drag-and-dropping the bones, but otherwise I believe it's just a different way to visualize the bones from list that is displayed when selecting a Skeleton.

How do you need to move the bones around? Depending on the use case, you might be able to use Twisted IK 2's Twisted_Bone3D node to modify the positions of the bones. It applies the position using the global pose override, but it should be modifiable to adjust the rest, pose, or custom_pose depending on what you need.

@akien-mga akien-mga force-pushed the 3.x branch 2 times, most recently from 71cb8d3 to c58391c Compare January 6, 2022 22:40
@akien-mga akien-mga modified the milestones: 3.5, 3.x Jul 2, 2022
@YuriSizov YuriSizov removed this from the 3.x milestone Dec 2, 2023
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.

10 participants