MAYA-127428 - Missing metadata notification. #2846
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
feldstj
requested changes
Jan 26, 2023
lib/mayaUsd/ufe/Utils.h
Outdated
| @@ -216,6 +216,13 @@ Ufe::Selection recreateDescendants(const Ufe::Selection& src, const Ufe::Path& f | |||
| MAYAUSD_CORE_PUBLIC | |||
| std::vector<std::string> splitString(const std::string& str, const std::string& separators); | |||
|
|
|||
| //! Gets the sub strings surrounded by separators. | |||
| MAYAUSD_CORE_PUBLIC | |||
| std::vector<std::string> getSubStrings( | |||
There was a problem hiding this comment.
To me the name of this method makes it sound like this is a generic function, but it really seems kind of specific to the metadata stream, no? Do you think this method is generic enough to warrant it being in Utils.h, or should it just be a helper function in StagesSubject.cpp where it is being used?
There was a problem hiding this comment.
I agree with Jonathan. The example string you give above "'uifolder':,'uisoftmin':0.0, 'uihide':1, 'uiorder':0" is very metadata specific. I would move it like Jonathan suggests and also rename it with metadata in name.
lib/mayaUsd/ufe/StagesSubject.cpp
Outdated
| @@ -321,12 +321,15 @@ void processAttributeChanges( | |||
| sendMetadataChanged = true; | |||
| std::stringstream newMetadataStream; | |||
| newMetadataStream << infoChanged.second.second; | |||
| std::string newMetadataKey = newMetadataStream.str(); | |||
| if (!newMetadataKey.empty()) { | |||
| std::string newMetadataKeys = newMetadataStream.str(); | |||
There was a problem hiding this comment.
Can newMetadataKeys be const?
seando-adsk
requested changes
Jan 26, 2023
feldstj
requested changes
Jan 27, 2023
lib/mayaUsd/ufe/StagesSubject.cpp
Outdated
| @@ -321,13 +341,12 @@ void processAttributeChanges( | |||
| sendMetadataChanged = true; | |||
| std::stringstream newMetadataStream; | |||
| newMetadataStream << infoChanged.second.second; | |||
| std::string newMetadataKeys = newMetadataStream.str(); | |||
| const std::string newMetadataKeys = newMetadataStream.str(); | |||
There was a problem hiding this comment.
(minor) I'd probably rename this to strMetadata for consistency.
- var renaming
seando-adsk
previously approved these changes
Jan 27, 2023
seando-adsk
assigned ecp-maya-devops-adsk and unassigned alicedegirolamo and ecp-maya-devops-adsk
alicedegirolamo
added
the
ready-for-merge
seando-adsk
removed
the
ready-for-merge
|
There is a build failure on Linux and OS/X platforms: |
alicedegirolamo
dismissed stale reviews from seando-adsk and feldstj
via
d45ae66
Thanks for looking in the build logs, now it should be fixed. |
feldstj
approved these changes
Jan 31, 2023
JGamache-autodesk
approved these changes
Jan 31, 2023
| std::vector<std::string> getMetadataKeys(const std::string& strMetadata) | ||
| { | ||
| std::vector<std::string> metadataKeys; | ||
| const std::regex rgx("(')([^ ]*)(':)"); |
There was a problem hiding this comment.
You are using the power of regular expressions, but not to their full extent. Please read more about capture groups. The function can be rewritten as:
std::vector<std::string> getMetadataKeys(const std::string& strMetadata)
{
std::vector<std::string> metadataKeys;
const std::regex rgx("'([^ ]*)':");
for (std::sregex_iterator it(strMetadata.begin(), strMetadata.end(), rgx), it_end; it != it_end;
++it) {
metadataKeys.push_back(std::string((*it)[1]));
}
return metadataKeys;
}
And work exactly as requested by extracting the captured strings.
seando-adsk
approved these changes
Feb 1, 2023
alicedegirolamo
added
the
ready-for-merge
seando-adsk
deleted the
degiroa/MAYA-127428/missing-metadata-notification
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR solves the issue described in Jira Ticket MAYA-127428
Type of change
We were parsing erroneously the string with all the metadata keys and values.
As an example, with the following metadata string:
"'uifolder':,'uisoftmin':0.0, 'uihide':1, 'uiorder':0"we got the list with the following string:
"uifolder':,'uisoftmin':0.0, 'uihide':1, 'uiorder"With the proposed solution we get the populated list:
"uifolder","uisoftmin", "uihide", "uiorder"which triggers the correct notification for the metadatas.