Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Publisher: Enhancement proposals #3897

Merged
merged 7 commits into from
Sep 26, 2022
Merged

Conversation

iLLiCiTiT
Copy link
Member

@iLLiCiTiT iLLiCiTiT commented Sep 22, 2022

Brief description

Proposing possible enhancements related to new publisher.

Description

Instance object has lifetime_data where can creator add objects from scene or other objects not related to instance data, during publishing can be accessed using lifetimeData on instance. Renamed INewPublisher to IHostPublish -> match other interfaces and get rid of "new publisher" (INewPublisher kept for backwards compatibility, but we could remove it now?). Creators have method apply_settings to simplify initialization.

Additional information

The lifetime_data were requested from Fusion implementation where node name is not enough to be able identify the node. I would say it's reasonable enough to provide this option too.

@iLLiCiTiT iLLiCiTiT self-assigned this Sep 22, 2022
@iLLiCiTiT iLLiCiTiT added the type: enhancement Enhancements to existing functionality label Sep 22, 2022
@BigRoy
Copy link
Collaborator

BigRoy commented Sep 22, 2022

Creators have method apply_settings to simplify initialization.

Could you explain the idea behind this?

Instance object has lifetime_data where can creator add objects from scene or other objects not related to instance data, during publishing can be accessed using lifetimeData on instance.

I'm not too sure about the name lifetimeData. It feels like it still needs a lot of explanation as to what to do with it. Maybe we should name it something that sounds like the opposite of "persistent data" (basically something that doesn't save with the scene - does lifetimeData do that sufficiently?)
Maybe transientData?

I also feel that storing it like that on the actual pyblish instance is a bit off since makes it feel less tightly coupled somehow even though it's crucial data to get back to the node that generated the instance. Couldn't we maybe store it into pyblish instance like:

instance.data = {}
instance.data.update(CreatedInstance.transientData)
instance.data.update(copy.deepcopy(CreatedInstance.data))

So that e.g. instance.data["tool"] would be possible.

@iLLiCiTiT
Copy link
Member Author

iLLiCiTiT commented Sep 22, 2022

Could you explain the idea behind this?

If you want apply settings you don't have to override __init__ method and hit potential future changes, but just override apply_settings method where you don't have to call super.

I'm not too sure about the name lifetimeData.

I'm ok with transient data.

I also feel that storing it like that on the actual pyblish instance is a bit off since makes it feel less tightly coupled somehow even though it's crucial data to get back to the node that generated the instance. Couldn't we maybe store it into pyblish instance like:

So you have to explain that you must not use keys that are same as in instance data because you would loose the values and those keys would be automatically propagated to pyblish instance data. I would say it's better to have it under specific key so you know you can safely access it from that key without any worries.

@jakubjezek001
Copy link
Member

jakubjezek001 commented Sep 23, 2022

I wonder if there is really not a way you could get node object by node name attribute. Something like:

node_name = "NodeName01"
comp_list = fu.GetCompList()
for node in comp_list.items():
	if node.GetAttrs()["TOOLS_Name"] == node_name:
		return node

@BigRoy
Copy link
Collaborator

BigRoy commented Sep 23, 2022

@jakubjezek001 I actually implemented that currently - see here.

However, it introduces the problem with the user having multiple comps open at the same time and it's likely that they might activate a different comp tab as they are running the Publish UI. It'd be hard to know which of the open comps that particular tool belonged to - whereas if we'd have the transient data we know exactly because we'd get the comp from the tool instead, using tool.Comp() or alike.

@BigRoy
Copy link
Collaborator

BigRoy commented Sep 23, 2022

This PR seems functional with regards to transient_data. This is what it allowed me to do as a change in Fusion logic, example commit. Pretty good!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: enhancement Enhancements to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants