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

hook default name doesn't match default convention #42

Open
mdfisher opened this issue Jul 12, 2022 · 1 comment
Open

hook default name doesn't match default convention #42

mdfisher opened this issue Jul 12, 2022 · 1 comment

Comments

@mdfisher
Copy link

mdfisher commented Jul 12, 2022

Problem:

Inconsistent naming of app-specific hook_scene_operation hooks provided with the AE engine is causing config issues since Toolkit references the app hook_scene_operation hooks by including the engine name in the default hook name but the default app hooks provided by tk-aftereffects don't match the default name.

The inconsistency means I have to add special cases for that DCC-specific default hook_scene_operation hook all over my configs. This kind of inconsistency in Toolkit code seems very unusual to me -- like an anit-pattern for a pattern which has already been long established.

The default hook_scene_operation hook in tk-aftereffects doesn't match the hook naming convention of tk-multi-workfiles2 and tk-multi-snapshot.

Naming as stock deployed in tk-aftereffects:

  • tk-aftereffects/v0.3.2/hooks/tk-multi-workfiles2/basic/scene_operation.py
  • tk-aftereffects/v0.3.2/hooks/tk-multi-snapshot/basic/scene_operation.py

Typical naming pattern in tk-multi-workfiles2:

  • tk-multi-workfiles2/v0.13.1/hooks/scene_operation_<engine_name>.py

Typical naming pattern in tk-multi-snapshot:

  • tk-multi-snapshot/v0.9.1/hooks/scene_operation_<engine_name>.py

Solution:

Rename the hook_scene_operation hooks for Snapshot and Workfiles2 like so:

  • tk-aftereffects/v0.3.2/hooks/tk-multi-workfiles2/basic/scene_operation_tk-aftereffects.py
  • tk-aftereffects/v0.3.2/hooks/tk-multi-snapshot/basic/scene_operation_tk-aftereffects.py

Other similar issues:

A similar naming issue exists for the Loader2 actions_hook hook in the tk-aftereffects engine as well. The hook name in the AE engine is an anti-pattern to those defined as the defaults in the app. It also breaks pattern with those app hooks provided in other engines for their respective apps (Workfiles2, Snapshot, Loader2).

In general, DCC-specific hooks should be named based on the DCC(and engine) which can run them. This break in pattern causes problems with our config-build scripts which desire to fallback on a default hook name in the apps.


Default hook value in Snapshot (from info.yml):

    hook_scene_operation:
        type: hook
        parameters: [operation, file_path]
        default_value: "scene_operation_{engine_name}"
        description: All the application specific scene operations (open, save etc) that
                     the app needs to carry out are collected in this hook.

Default hook value in Workfiles2 (from info.yml):

    hook_scene_operation:
        type: hook
        default_value: "{self}/scene_operation_{engine_name}.py"
        description: All the application specific scene operations (open, save etc) that
                     the app needs to carry out are collected together in this hook.

A similar problem exists for actions_hook in Loader2 (from info.yml):

The engine name is in the default actions_hook hook name for Loader2 hooks in the app but the default hooks in tk-aftereffects for tk-multi-loader2 doesn't have the engine name in its actions_hook.

    # hooks
    actions_hook:
        type: hook
        default_value: "{self}/{engine_name}_actions.py"
        description: Hook which contains all methods for action management.

Default app hooks within engines should match the default app hook name provided in the apps. Especially in light of the fact that the other engines name their hooks according to the pattern as set forth in the info.yml files for the apps.

Note:

I haven't checked this hook naming issue for other default hooks. There could be more but these are the most obvious as they break the basic default "File Open" logic.

@mdfisher
Copy link
Author

Now that I check other engines (and not the main apps), I see that the hook naming is consistent with the other engines.

Closing Issue.

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

No branches or pull requests

1 participant