Skip to content

Conversation

@Tilix4
Copy link

@Tilix4 Tilix4 commented Dec 3, 2025

Summary

Adds animation keyframe copying support and frame range control to importTemplate, enabling precise template imports with proper animation data transfer.

Features

  1. Animation Keyframe Copying

    • Configures paste special settings to ensure animation keys are copied when pasting to existing destinationNodes
    • Uses actionTemplateMode = false and setPasteSpecialFullTransfer(true) for proper keyframe transfer
  2. Frame Range Control

    • startFrame: Destination frame to paste at (defaults to 1)
    • frameRange: Source frame range [startFrame, endFrame] from template
  3. Error Handling

    • Returns false on template read failures or paste mismatches
    • Proper cleanup of paste special settings

API

oGroupNode.prototype.importTemplate(
  tplPath, destinationNodes, extendScene, nodePosition, pasteOptions, 
  startFrame,    // NEW: Destination frame (defaults to 1)
  frameRange     // ENHANCED: Source range [startFrame, endFrame]
)

Examples

// Import frames 5-15 from template to frame 20
group.importTemplate(tplPath, nodes, true, new oPoint(0,0,0), undefined, 20, [5, 15]);

// Import whole template to frame 10
group.importTemplate(tplPath, nodes, true, new oPoint(0,0,0), undefined, 10);

@mchaptel
Copy link
Collaborator

mchaptel commented Dec 3, 2025

I like what you're doing here and where you're going with this but I feel like it might be too drastic a change to enforce some of those paste special options, which could potentially break existing workflows. For example, the extend scene parameter and the matching behaviors could be pipeline breaking.

How about we create another method for example importActionTemplate with a couple of parameters for the more opinionated options being enforced right now?

@Tilix4
Copy link
Author

Tilix4 commented Dec 4, 2025

which could potentially break existing workflows

I'm not aware of side effects which may break existing workflows, I'll trust you on this.

How about we create another method for example importActionTemplate with a couple of parameters for the more opinionated options being enforced right now?

I don't mind adding a new function, but IMO it'll be better if we keep everything in one function. Maybe you could test it on your use cases and see how breaking it is?

@mchaptel
Copy link
Collaborator

mchaptel commented Dec 4, 2025

It's a classic code sin to have a function perform more than one action based on parameters ;)
Anyway the point is to be the most context agnostic, so enforcing some settings appears to me like a bad call. It would be easier for example to support an options object and pass it to the function from the script level where the function is called?
I would be fine with adding an extra method to avoid potentially breaking compatibility. It doesn't cost anything :)

some extra reading on the subject:
https://softwareengineering.stackexchange.com/questions/145055/are-there-guidelines-on-how-many-parameters-a-function-should-accept

One example of breaking compat for "extend scene" would be for people preparing scenes by importing the rigs. If you're using this function to do it, all your scenes would now be the length of your rig for ex (instead of the length of your animatic for example). Not ideal since it's an common way to setup new scenes in production.

As a side note, if you're interested, the most up to date set of features is on the 'require-refactor' branch. It will eventually be merged into master when it's matured, but it also includes a test framework so adding new features is a good time to add some tests as well

@Tilix4
Copy link
Author

Tilix4 commented Dec 4, 2025

Okay, do you really want to start a debate? :) Importing a single frame is a special case of importing a sequence of frames, so we should do it with the same function IMO. And I believe the nodePosition isn't a relevant parameter in this context and may be removed BUT I won't insist on that if you think it is the best.

I've tried removing the special paste settings and it seems to work fine still... So I don't understand why they are in the Pose_Copier code from Harmony. I may remove them from the main code.
Also, let me know if you confirm importActionTemplate is the name you like (which will have the exact same amount of parameters as importTemplate implementation I proposed I'm afraid)

@mchaptel
Copy link
Collaborator

mchaptel commented Dec 4, 2025

Haha sure debating sounds good. It's an important part of finding the right solution!

This function's primarily used to import nodes from a template, not just anim/frames. That's where the distinction from templates/action templates comes from. That's why the node position argument is there.
That's exactly what I mean about it needing a separate function.

For the paste special params, it may be that they are correctly set on your machine so now you're getting the behavior you're expecting, from a side effect? In which case it could be good to test with other settings in the paste special window and test? As well as potentially backing up the existing settings and restoring them after to avoid creating side effects?

Since we are going to use this function to import keys onto existing nodes, maybe we don't need the node position argument and some other params related to that? (ignore that last bit, I saw you already added that)

It's confusing that Harmony itself conflates those two behaviors to be honest. I regret adding the nodes parameter there, it would have been clearer. I don't want to break backwards compatibility by removing it though, maybe at some point I'll add a cleaner alias

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants