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

Reference : Support metadata edits. #1552

Merged
merged 2 commits into from Nov 16, 2015

Conversation

johnhaddon
Copy link
Member

Previously we never serialised any metadata from Reference nodes, regardless of whether the metadata originated from within the referenced file or was added/edited after loading. We now support both the addition of new metadata and editing of metadata from the reference.

This implementation uses the persistence flag on each metadata item to track whether it is considered to have come from the referenced file or not. Strictly speaking, this is slightly abusing the semantics of that flag, but this does allow us to support edits with minimal changes, and is similar to the way we treat the Dynamic plug flag already. We still need a full system for tracking reference edits to plug values - when we implement such a thing we should probably track metadata edits with the same mechanism.

Fixes #1536.

@davidsminor, could you take a look and see if this is sufficient for your current purposes? Also, am I right in thinking you have an "import reference" type functionality? If so, could you check that my changes are compatible with that?

The Reference node itself now makes Metadata non-persistent after loading, rather than relying on the Plug bindings having to pick up the slack. Plugs shouldn't need any special knowledge of References, since References are a higher level construct.
Previously we never serialised _any_ metadata from Reference nodes, regardless of whether the metadata originated from within the referenced file or was added/edited after loading. We now support both the addition of new metadata and editing of metadata from the reference.

This implementation uses the persistence flag on each metadata item to track whether it is considered to have come from the referenced file or not. Strictly speaking, this is slightly abusing the semantics of that flag, but this does allow us to support edits with minimal changes, and is similar to the way we treat the Dynamic plug flag already. We still need a full system for tracking reference edits to plug values - when we implement such a thing we should probably track metadata edits with the same mechanism.

Fixes GafferHQ#1536.
@andrewkaufman
Copy link
Contributor

Code looks good to me assuming it works for the purposes you asked @davidsminor to check.

@davidsminor
Copy link
Contributor

Hello,

Thanks for that - it looks like it fixes the problem I'd run into at least (Jabuka metadata was getting wiped out whenever I updated a reference box, now it ain't).

My Swap To Imported action works by loading the .gfr onto a new box, then transferring the plug values/connections on the existing reference onto the box and deleting the reference. I think this means I'm going to need a python binding for that transferPersistentMetadata() method, or at least just reimplement it in python I guess. Sounds like that mechanism might change in the future though - do you think it's worth just implementing the swap to imported functionality in Gaffer?

@johnhaddon
Copy link
Member Author

I do think "swap to imported" makes sense for Gaffer yes. Given time constraints though I suggest we merge this as-is and you transfer the metadata in python for now. What may change in the future is that we might not make the metadata non-persistent in the Reference after loading, but since you'll be wanting the metadata to be persistent on the copied Box anyway, I think you can probably ignore that?

davidsminor added a commit that referenced this pull request Nov 16, 2015
Reference : Support metadata edits.
@davidsminor davidsminor merged commit 80a11e3 into GafferHQ:master Nov 16, 2015
@johnhaddon johnhaddon deleted the referenceMetadata branch November 20, 2015 17:21
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

Successfully merging this pull request may close these issues.

None yet

3 participants