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

Pipeline Integrations
1

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

2 Likes
2

Woops, should link the code I am talking about

3

+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

4

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

1 Like
5

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.

1 Like
6

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. :slight_smile:

7

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.

8

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 :slight_smile:

9

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! :slight_smile:

Cheers,
Fabian