Tk-multi-shotgunpanel & tk-multi-loader2 using same actions code?

Ahuge · March 9, 2021, 10:00pm

Hi

I noticed that the actions schema was very nearly almost the exact same between tk-multi-loader2 and tk-multi-shotgunpanel.
However in loader2, the generate_actions call takes sg_publish_data and in shotgunpanel it takes sg_data.

However, from what I can tell, the rest of it is the same.

I would like to ask that we modify one of those two apps to use the hook schema of the other.
I know this breaks backwards compatibility, however in this case I don’t see any good reason to keep the backwards compatibility, maybe we just major version up this app and change the hook schema.

What I am envisioning is a single actions hook for loader2 and shotgunpanel, and then maybe I extend it in the shotgunpanel to expose additional actions if I needed, or visa versa.

Can I get some internal team thoughts on this? I’ll have a PR incoming for you

Ahuge · March 9, 2021, 10:01pm

Woops, should link the code I am talking about

github.com

shotgunsoftware/tk-multi-shotgunpanel/blob/master/hooks/tk-nuke_actions.py#L23


import glob
HookBaseClass = sgtk.get_hook_baseclass()
class NukeActions(HookBaseClass):
    """
    Shotgun Panel Actions for Nuke
    """
    def generate_actions(self, sg_data, actions, ui_area):
        """
        Returns a list of action instances for a particular object.
        The data returned from this hook will be used to populate the
        actions menu.
        The mapping between Shotgun objects and actions are kept in a different place
        (in the configuration) so at the point when this hook is called, the app
        has already established *which* actions are appropriate for this object.
        This method needs to return detailed data for those actions, in the form of a list

github.com

shotgunsoftware/tk-multi-loader2/blob/master/hooks/tk-nuke_actions.py#L29


import sgtk
HookBaseClass = sgtk.get_hook_baseclass()
class NukeActions(HookBaseClass):
    ##############################################################################################################
    # public interface - to be overridden by deriving classes
    def generate_actions(self, sg_publish_data, actions, ui_area):
        """
        Returns a list of action instances for a particular publish.
        This method is called each time a user clicks a publish somewhere in the UI.
        The data returned from this hook will be used to populate the actions menu for a publish.
        The mapping between Publish types and actions are kept in a different place
        (in the configuration) so at the point when this hook is called, the loader app
        has already established *which* actions are appropriate for this object.
        The hook should return at least one action for each item passed in via the

Fabian · March 10, 2021, 3:04pm

+1 from me! This is the exact reason why I disabled Shotgun Panel loading hooks all together, since maintaining the same code for both is too much atm. @Ahuge would be great if you (or the Shotgun Team) can make this work!

Cheers,
Fabian

jhultgre · March 11, 2021, 12:56am

I think I wrote some kind of wrapper hook for one or the other.

Ahuge · March 11, 2021, 3:49am

I modified the loader2 hook by changing the signature from this

def generate_actions(self, sg_publish_data, actions, ui_area):
    ...

def execute_action(self, name, params, sg_publish_data):
    ...

to this

def generate_actions(self, sg_publish_data=None, actions=[], ui_area=None, sg_data=None):
    if sg_publish_data is None and sg_data:
        sg_publish_data = sg_data
    ...

def execute_action(self, name, params, sg_publish_data=None, sg_data=None):
    if sg_publish_data is None and sg_data:
        sg_publish_data = sg_data
    ...

The key to both are the reassignment of sg_publish_data to sg_data if sg_publish_data is None and sg_data exists.
And then also making sure that in generate_actions, all of the parameters have defaults, in execute_action only the sg_publish_data needs to have a default.

Now it’s important to note, if you decide to do this, and Shotgun updates toolkit, you have to make sure you update your hook to match how they are calling it, if it changes.

I’ll try to put together a better PR instead of this reactionary hack this week.

Fabian · March 11, 2021, 9:12am

Thanks @jhultgre and @Ahuge !
Definitely a good way to go. I will wait on Ahuge’s PR for now to get the compatibility between the hooks.

Ahuge · March 11, 2021, 5:00pm

PR is up here

We need buy in from the team to get it merged. I also posed the question on if shotgunpanel or loader2 was used more frequently. I’d rather api break the least used one.

Fabian · March 12, 2021, 3:08pm

Thanks @Ahuge for your efforts in this! Unfortunately I don’t think the shotgun team will permit API breaking changes. That said I think there is a way of maintaining compatibility. There is a method called sgtk.Application.create_hook_instance which gives you the hook instance. With that and some python inspect magic it would be possible to query what function parameter is currently present and react to that.

Hope that helps

Fabian · March 30, 2021, 4:40pm

Hi @Ahuge !

I’ve send you a PR Maintain backward compatibility when calling action hooks by fabiangeisler · Pull Request #1 · Ahuge/tk-multi-shotgunpanel · GitHub. Curious what you think about it!

Cheers,
Fabian