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

Added checkbox to import all animations when importing an OoT skeleton #485

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Nokaubure
Copy link

Added checkbox to import all animations when importing an OoT skeleton
Supports Link skeleton too

image

@fig02
Copy link
Contributor

fig02 commented Dec 16, 2024

Considering theres a whole other tab for animations, maybe this makes sense as a "import all" button there?

Copy link
Contributor

@Yanis002 Yanis002 left a comment

Choose a reason for hiding this comment

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

I'll try this when I can (not an animation expert here)

to make CI happy you need to run black, the instructions should be in the readme (if you run into issues feel free)

fast64_internal/oot/skeleton/importer/functions.py Outdated Show resolved Hide resolved
fast64_internal/oot/skeleton/importer/functions.py Outdated Show resolved Hide resolved
fast64_internal/oot/skeleton/importer/functions.py Outdated Show resolved Hide resolved
fast64_internal/oot/skeleton/utility.py Outdated Show resolved Hide resolved
@Yanis002 Yanis002 added enhancement New feature or request oot Has to do with the Ocarina of Time 64 side labels Dec 16, 2024
@Nokaubure
Copy link
Author

Considering theres a whole other tab for animations, maybe this makes sense as a "import all" button there?

The idea was to make it similar to the old import plugin where loading the skeleton would load all animations. Separating it would be like if the limbs had to be imported with an "import all limbs" button in DL tab, so not a good idea imo.

Copy link
Contributor

@Yanis002 Yanis002 left a comment

Choose a reason for hiding this comment

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

I can't find the menu you're showing on the screenshot but it seems to work for non-Link objects, with Link I get the skeletons but no animations, also you still need to format your branch to make CI happy

about Fig's suggestion: I don't know what would be better since I never use these features but I think for now it's fine?, if people are annoyed by that (or another dev wants) we'll change that

@Nokaubure
Copy link
Author

I can't find the menu you're showing on the screenshot but it seems to work for non-Link objects, with Link I get the skeletons but no animations, also you still need to format your branch to make CI happy

rechecked and it works with Link, it takes like 5 minutes tho since there's a lot of animations but does the job, the menu is in those side tabs

image

@Yanis002
Copy link
Contributor

sorry, I meant the menu that shows the animations at the bottom (and I'll try Link again)
image

@Nokaubure
Copy link
Author

Nokaubure commented Dec 19, 2024

sorry, I meant the menu that shows the animations at the bottom (and I'll try Link again)

For this you go to dope sheet editor, then select action editor in the dropdown next to it, and then you will see a dropdown showing all animations

@fig02
Copy link
Contributor

fig02 commented Dec 19, 2024

"Separating it would be like if the limbs had to be imported with an "import all limbs" button in DL tab, so not a good idea imo"

I dont think the comparison to limbs is really fair. Of course, the tool does not have a separate tab for limbs. It does however have a separate tab for animations. Its breaking pre-existing organization of the tool.
Limb display lists are one piece of the skeleton, while animations are entirely separate.

@Dragorn421
Copy link
Contributor

I think a checkbox with the skeleton props is fine, mostly because it's simpler than if it was a button elsewhere and you'd have to somehow specify where you want the animations from.
Except for Link you typically want to import at least one animation anyway to not have to work with the folded model so this is just more convenient

Note the alternative of a separate "importal all" feature would have its merits too, as it'd allow importing all animations from a different object, for example typically os_anime animations over the various skeletons using them. The checkbox doesn't handle this use case (which is fine to me as a known limitation)

@Yanis002
Copy link
Contributor

sorry, I meant the menu that shows the animations at the bottom (and I'll try Link again)

For this you go to dope sheet editor, then select action editor in the dropdown next to it, and then you will see a dropdown showing all animations

so yeah it seems it doesn't work for me with Link, I found why I think, ootGetAnimNames returns nothing since Link's animations are in link_animetion or something, and this file isn't read it seems, I'm surprised it works for you because afaict it shouldn't?

image

@Nokaubure
Copy link
Author

so yeah it seems it doesn't work for me with Link, I found why I think, ootGetAnimNames returns nothing since Link's animations are in link_animetion or something, and this file isn't read it seems, I'm surprised it works for you because afaict it shouldn't?

Reason it works is because its getting the headers from gameplay_keep, and calling the quick import function which looks for the symbols in link_animetion, so not sure why it doesnt for you.

I'm using Legacy mode, maybe its that...

@Yanis002
Copy link
Contributor

I'm using the newer system so yeah that's probably the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request oot Has to do with the Ocarina of Time 64 side
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants