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

Controlling registered event handlers order #314

Open
aardschok opened this issue Apr 24, 2018 · 8 comments
Open

Controlling registered event handlers order #314

aardschok opened this issue Apr 24, 2018 · 8 comments
Labels

Comments

@aardschok
Copy link
Collaborator

Issue

The registered_event_handlers can hold multiple callbacks under the same key. Currently the callbacks are stored in a weakref.WeakSet under the given event name, e.g: "taskChanged". As a set in unordered it does not allow us to add a "pre" callback for the event before the original registered callback.

Motivation

By allowing the callbacks to be stored ordered for an event allows configurations to insert pre or post event callbacks. This in turn allows for custom logic which a studio might need. An OrderedDict or defaultdict(list) might be an valid substitute for the current weakref.WeakSet.

User case

Our ( Colorbleed ) case is as follows: We want to reinitialize the Maya based on the application's toml file to ensure all folders are there when a user switched context to a task which is not on disk yet.
This often occurs when the user creates a task for an asset from within Maya through the project manager. The task does not exists on disk yet and with the current configuration of Avalon it forces the user to open a new application instance.

@mottosso
Copy link
Contributor

Ah yes, I remember why I did it this way. Having ordered callbacks is a recipe for complexity, as you can easily become dependent on the order but be unable to control it or modify it. Callbacks in general are unordered and for good reason.

I think if we wanted another level of callbacks, we should do what Maya and others does; which is a "post this" and "post that".

api.on("pre_init", on_pre_init)
api.on("init", on_init)
api.on("post_init", on_post_init)

It isn't pretty and can't really be pushed any further (e.g. post_post_init?), but I think we'll dodge a bullet this way. If we really do need finer control of ordering, we should consider a task-queue system rather than callbacks.

Speaking of which, to solve this particular problem, would it be possible to form a minor task-queue inside of your callback?

def on_init():
  my_pipeline.pre_init()
  my_pipeline.init()
  print("Almost ready..")
  my_pipieline.post_init()

api.on("init", on_init)

@aardschok
Copy link
Collaborator Author

Speaking of which, to solve this particular problem, would it be possible to form a minor task-queue inside of your callback?

Currently I have tinkered together a wrapped function similar to that which replaces the taskChanged callback.

@BigRoy
Copy link
Collaborator

BigRoy commented Apr 25, 2018

Currently I have tinkered together a wrapped function similar to that which replaces the taskChanged callback.

But that's not the reliable solution. :) Because it will replace any taskChanged callback registered at that time - which you might not want.

I'd go the way Marcus suggested and do post_taskChanged Or maybe after_taskChanged?

@mottosso
Copy link
Contributor

One last thing, about the use case above. Could/should it be solved by extracting the method for creating folders?

Example

def on_init():
  my_pipeline.create_folders()

def on_task_changed():
  my_pipeline.create_folders()

That way, the functionality to create folders can be called explicitly in whatever mechanism is responsible for switching tasks. Pros, cons?

@BigRoy
Copy link
Collaborator

BigRoy commented Apr 26, 2018

Not really. The problem we are facing specifically is that Avalon maya will try to set the project on task changed but prior to that we want to call some initializing code, but we can't because this callback will trigger earlier because it's registered in core.

Could/should it be solved by extracting the method for creating folders?

This we have done already. There's just no moment we can trigger it now prior to this callback Avalon Maya host will already be doing. :)

@aardschok
Copy link
Collaborator Author

aardschok commented Apr 30, 2018

I think this might be a better implementation but I do have a question regarding the before and after.

Is there a way that allows to automate the order of a custom callback?
A Breakdown:

  1. We create a "before_taskChanged"
  2. We create a "taskChanged"
  3. We create a "after_taskChanged"

If we trigger the "taskChanged" is there a logic that ensures that the other two callbacks are executed as well in the proper order: before_taskChanged > taskChanged > after_taskChanged?

I think this would be a handy (to be honest, it is a logical behavior to expect), thoughts?

@mottosso
Copy link
Contributor

If we trigger the "taskChanged" is there a logic that ensures that the other two callbacks are executed as well in the proper order

There isn't, but I wouldn't expect it to either. Rather, I'd expect each to be called when appropriate.

def change_task():
  api.emit("before_taskChanged")
  # do some things
  my_pipeline.change_task()
  api.emit("taskChanged")
  # clean-up, call other callbacks etc.
  api.emit("after_taskChanged")

@mottosso
Copy link
Contributor

In the Maya case, there's a callback triggered exactly on time changed, and then there's another one triggered after time has changed and simulation has had a chance to run. I think these things need to be explicitly called this way.

But yes, I think you're right, I had forgotten about that mechanism. Much cleaner IMO.

tokejepsen pushed a commit to tokejepsen/core that referenced this issue May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants