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

280: Split-parent animations needs support for re-entry case #504

Closed
tngreene opened this issue Dec 8, 2019 · 3 comments
Closed

280: Split-parent animations needs support for re-entry case #504

tngreene opened this issue Dec 8, 2019 · 3 comments

Comments

@tngreene
Copy link
Collaborator

@tngreene tngreene commented Dec 8, 2019

Suppose this blend file:

Version 1 (which may technically work right now):

Exportable Collection
  1_AnimatedPart  
  3_Cube_parent_out_of_collection
Master Scene
2_parent_in_master_collection

When accessed via bpy, you'd see this as a parent-child chain of 1_AnimatedPart as the top, 2_parent_in_collection, 3_Cube_parent_out_of_collection at the bottom. As you can see this snakes in and out and back in of the collection. A niave implementation of the feature would actually handle this as a re-use case since the bone for 1_AnimatedPart already exists.

Here is version 2:

Exportable Collection
  3_Cube_parent_out_of_collection
  one_AnimatedPart  
Master Collection
2_parent_in_master_collection

Here I've changed the name to force the algorithm to deal with this in the worst order possible. By the time the recursion algorithm gets to one_AnimatedParent it will already exist!!

We can check for that but this whole conversation is stupid.

  1. I can't think of any workflow for this
  2. This feature is about enabling people to share animations across OBJs, not hide meshes in some kind of chipmunk's cheek pouch.
  3. If the exporter actually was smart enough to do this we'd probably have some pretty astonished users when they have something not visible yet also a child of something inside the OBJ still affecting animations
    4.and it gives users a hack to exploit to temporarily exclude geometry from export which is not what this feature is about at all!

Unless Ben or someone else tells me otherwise, this is something that needs to be guarded against, not technically allowed, supported, encouraged, or advertised.

*Note: It doesn't matter 1_ or 2_ is animated. The whole case stinks.

Related to #494

@tngreene tngreene self-assigned this Dec 8, 2019
@tngreene tngreene changed the title 280: Split-parent animations needs guard against re-entry case 280: Split-parent animations needs support for re-entry case Dec 9, 2019
@tngreene

This comment has been minimized.

Copy link
Collaborator Author

@tngreene tngreene commented Dec 9, 2019

Nope! We're supporting this. The reason being that one day an artist's project could get so complicated it will snake in and out and we don't want to make artists suddenly have to re-organize their whole tree because I don't think they should have such a disorganized artist.

@tngreene

This comment has been minimized.

Copy link
Collaborator Author

@tngreene tngreene commented Dec 9, 2019

The fix for version two is actually very simple. When recursing down we see if that bone for that Blender Object is already in the tree and if so, use that instead of making a new one. Continue recursing from there. This "already exists" case needs

  • unit test
  • code in xplane_file.py

And boy does it get fun when the already exist case ALSO has a parent to walk up ("new branch"). And what if that new branch then has a "reuse" case!

Or, during the "already exist with parent", that parent is a reuse case? How crazy can we get? I'm just rattling stuff off. I'm sure that some of these case may not be possible.

@tngreene

This comment has been minimized.

Copy link
Collaborator Author

@tngreene tngreene commented Mar 10, 2020

Okay, now it is really really closed with d124308

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.