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

Custom Skeleton3DEditorPlugin #36409

Merged
merged 1 commit into from
May 27, 2020

Conversation

fire
Copy link
Member

@fire fire commented Feb 21, 2020

Co-authored-by: Marios Staikopoulos marios@staik.net

Screen Shot 2020-02-20 at 9 15 03 PM

Sponsored by IMVU.

Features:

Drag and drop skeleton parentage move with UNDO!

Rest and pose are exposed as transforms with degrees as units. [UPDATED]

Can key transforms into animation tracks.

Handles the other variables that represent a bone.

Ensure all used labels are translatable.

DOES NOT FREEZE THE EDITOR FOR 30 seconds swapping between the skeleton and any other node!!!

Use for testing:

https://github.com/KhronosGroup/glTF-Sample-Models/blob/master/2.0/Fox/glTF-Binary/Fox.glb

Bugsquad edit: Fixes #35652.

@fire fire force-pushed the skeleton-custom-inspector branch from 8e62e87 to 2cb436b Compare February 21, 2020 06:09
scene/3d/skeleton.cpp Outdated Show resolved Hide resolved
@fire fire force-pushed the skeleton-custom-inspector branch 5 times, most recently from 0f656a4 to d2a866e Compare February 21, 2020 07:55
@fire fire changed the title [WIP] EditorInspector Custom SkeletonEditorPlugin for Skeletons EditorInspector Custom SkeletonEditorPlugin for Skeletons Feb 21, 2020
@AndreaCatania AndreaCatania added this to the 4.0 milestone Feb 21, 2020
@AndreaCatania
Copy link
Contributor

Copy link
Contributor

@AndreaCatania AndreaCatania left a comment

Choose a reason for hiding this comment

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

@fire you did a really good job here, and this addition improves a lot the skeleton usability!

However, you can find some little tweaks that you can do to the code in order to improve it.

Another comment that I've is that there are too much hard written numbers that makes the code hard to read. See:

for (int i = 0; i < 12; i++) {

What 12 stands for? In these cases would be nice have something like:

const int spin_slider_count = 12;
for (int i = 0; i < spin_slider_count; i++) {

editor/plugins/skeleton_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/skeleton_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/skeleton_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/skeleton_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/skeleton_editor_plugin.h Outdated Show resolved Hide resolved
editor/plugins/skeleton_editor_plugin.cpp Outdated Show resolved Hide resolved
@fire fire force-pushed the skeleton-custom-inspector branch 2 times, most recently from 6b9a524 to c4de366 Compare February 21, 2020 17:30
@fire
Copy link
Member Author

fire commented Feb 21, 2020

Ready for second review. I didn't feel like figuring the math for iterating this and it's less clear with that math.

	transform_slider[0]->set_value(tform.get_basis()[Vector3::AXIS_X].x);
	transform_slider[1]->set_value(tform.get_basis()[Vector3::AXIS_X].y);
	transform_slider[2]->set_value(tform.get_basis()[Vector3::AXIS_X].z);
	transform_slider[3]->set_value(tform.get_basis()[Vector3::AXIS_Y].x);
	transform_slider[4]->set_value(tform.get_basis()[Vector3::AXIS_Y].y);
	transform_slider[5]->set_value(tform.get_basis()[Vector3::AXIS_Y].z);
	transform_slider[6]->set_value(tform.get_basis()[Vector3::AXIS_Z].x);
	transform_slider[7]->set_value(tform.get_basis()[Vector3::AXIS_Z].y);
	transform_slider[8]->set_value(tform.get_basis()[Vector3::AXIS_Z].z);

@fire
Copy link
Member Author

fire commented Feb 21, 2020

Noticed that Bone Custom [Pose] is missing.

@fire fire force-pushed the skeleton-custom-inspector branch from c4de366 to 86e066b Compare February 21, 2020 20:08
@fire
Copy link
Member Author

fire commented Feb 21, 2020

Adding bone custom pose to the inspector.

editor_screenshot_2020-02-21T121021-0800

@AndreaCatania
Copy link
Contributor

I didn't feel like figuring the math for iterating this and it's less clear with that math.

You can use this one:

const int matrix_row_count = 3;
const int matrix_column_count = 3;

for(int c = 0; c < matrix_column_count; c++){
    for(int r = 0; r < matrix_row_count; r++){
        transform_slider[(c*matrix_row_count) + r]->set_value(tform.get_basis()[c][r]);
    }
}

Copy link
Contributor

@AndreaCatania AndreaCatania left a comment

Choose a reason for hiding this comment

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

The code looks better now, once fixed the above code it's good to be merged. However, I'm approving it now. Thanks :)

@fire fire force-pushed the skeleton-custom-inspector branch 2 times, most recently from d3bdeb2 to 64f4eef Compare February 24, 2020 17:59
@fire
Copy link
Member Author

fire commented Feb 24, 2020

Merged AndreaCatania's suggestion, but currently testing a bug with the matrix step being "1".

@fire fire force-pushed the skeleton-custom-inspector branch 5 times, most recently from 8237371 to f4ea475 Compare February 24, 2020 18:55
@fire fire force-pushed the skeleton-custom-inspector branch from 3bac5db to 0aac719 Compare May 17, 2020 04:11
@fire
Copy link
Member Author

fire commented May 17, 2020

Updated to master. Waiting on the build.

scene/3d/skeleton_3d.cpp Outdated Show resolved Hide resolved
@fire fire force-pushed the skeleton-custom-inspector branch from 0aac719 to a920714 Compare May 22, 2020 14:37
@JFonS
Copy link
Contributor

JFonS commented May 22, 2020

The spinners used in the inspector only allow integer values, they should have float values.

I would also get rid of the custom grid of spinners and use the same inspector controls useed in Vector3 and Transform properties, but that could be done on a separate PR.

Co-authored-by: Marios Staikopoulos <marios@staik.net>
@fire fire force-pushed the skeleton-custom-inspector branch from a920714 to f7fdc87 Compare May 22, 2020 16:54
@fire
Copy link
Member Author

fire commented May 22, 2020

@JFonS I changed the default to spinner->set_step(0.001f);.

Agreed, the matrix changes can be another pr.

@akien-mga akien-mga merged commit 84d9e10 into godotengine:master May 27, 2020
@akien-mga
Copy link
Member

Thanks a lot!

@fire Would you be able to prepare a PR backporting this for the 3.2 branch? Given the amount of changes, I assume that a simple git cherry-pick might not be trivial nor guaranteed to work as expected. -- There's no hurry though, we might want to wait for some testing in master / further fixes before merging the same changes in 3.2, so it could be for 3.2.3.

@slapin
Copy link
Contributor

slapin commented May 27, 2020

Would be nice to have for 3.2 as skeleton editing is currently very hard with 175 bones
as editor crashes with default settings freezing the computer beforehand. Increasing queue size fixes the crash but editing is still not a piece of cake.

@fire fire deleted the skeleton-custom-inspector branch May 27, 2020 14:47
@keymastar
Copy link

Is it possible to use this on 3.2, and if so, how? I am trying to make a 3d game and it seems impossible if I can't access my rigged models.

@Zireael07
Copy link
Contributor

@slapin @keymastar 3.2 version is being worked on here: #39090 and not merged yet.

@keymastar
Copy link

@slapin @keymastar 3.2 version is being worked on here: #39090 and not merged yet.

That's great to hear. Thanks for letting me know.

@slapin
Copy link
Contributor

slapin commented May 28, 2020 via email

@akien-mga
Copy link
Member

Removing cherrypick:3.2 label as the cherrypicking is done via a separate PR: #39090.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jun 4, 2020
@Sawrr
Copy link
Contributor

Sawrr commented Jun 14, 2020

I am not seeing the "Create physical skeleton" option anymore after this commit. The menu dropdown at the top seems to be missing. Was this intentional?

@fire
Copy link
Member Author

fire commented Jun 14, 2020

Is not intentional.

@Sawrr
Copy link
Contributor

Sawrr commented Jun 14, 2020

@fire
Copy link
Member Author

fire commented Jun 14, 2020

@Sawrr Can you make a pull request, should be simple to test.

@Sawrr
Copy link
Contributor

Sawrr commented Jun 14, 2020

@fire Sure. This is my first PR so let me know if I need to add any additional detail #39543

@giabeni
Copy link

giabeni commented Jun 29, 2020

I think there is still open issues regarding to the Skeleton 3D Editor.

I am using a model with no more than 150 bones and every time that I make a left/right click in the skeleton node, Godot crashes.

I have already tried using the DAE and the GLTF/GLB exports methods... but the result is the same.

I was hopeful that this new release would fix it but no success :( .

Context and Environment:

MacBook Pro macOS Catalina 10.15.1
Processor 2,3 GHz Dual-Core Intel Core i5
Memory 8GB 2133 MHz LPDDR3
Graphics Intel Iris Plus Graphics 640 1536 MB

Is it a memory issue of my computer or is it a real bug?

Btw, the animations and constraints using the bones works normally. It only crashes when selecting the skeleton node.

Thanks in advance

@Zireael07
Copy link
Contributor

IIRC the 3.2 cherrypick hasn't happened yet, so it's on master only?

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.

Editor freeze when clicking Skeleton node of a glTF scene with 500+ bones
10 participants