Box : Copy all metadata when promoting plugs.

Except of course layout metadata, because it's really not very useful to the user if the promoted plug appears in a strange section with a frozen activator value.

python/GafferTest/BoxTest.py

@@ -961,5 +961,28 @@ def testPromoteArrayPlug( self ) :
 		self.assertEqual( len( s2["b"]["n"]["in"] ), 3 )
 		self.assertTrue( s2["b"]["n"]["in"].getInput().isSame( s2["b"]["p"] ) )
 
+	def testPromotionIncludesArbitraryMetadata( self ) :
+
+		s = Gaffer.ScriptNode()
+
+		s["b"] = Gaffer.Box()
+		s["b"]["n"] = Gaffer.Node()
+		s["b"]["n"]["user"]["p"] = Gaffer.IntPlug( flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic )
+
+		Gaffer.Metadata.registerPlugValue( s["b"]["n"]["user"]["p"], "testInt", 10 )
+		Gaffer.Metadata.registerPlugValue( s["b"]["n"]["user"]["p"], "testString", "test" )
+
+		p = s["b"].promotePlug( s["b"]["n"]["user"]["p"] )
+		p.setName( "p" )
+
+		self.assertEqual( Gaffer.Metadata.plugValue( p, "testInt" ), 10 )
+		self.assertEqual( Gaffer.Metadata.plugValue( p, "testString" ), "test" )
+
+		s2 = Gaffer.ScriptNode()
+		s2.execute( s.serialise() )
+
+		self.assertEqual( Gaffer.Metadata.plugValue( s2["b"]["p"], "testInt" ), 10 )
+		self.assertEqual( Gaffer.Metadata.plugValue( s2["b"]["p"], "testString" ), "test" )
+
 if __name__ == "__main__":
 	unittest.main()

python/GafferUITest/BoxUITest.py

@@ -180,5 +180,18 @@ def testDisabledNodulesAfterCutAndPaste( self ) :
 
 		self.assertEqual( g.nodeGadget( s["b1"] ).nodule( s["b1"]["p"] ), None )
 
+	def testPromotionIgnoresLayoutSection( self ) :
+
+		s = Gaffer.ScriptNode()
+
+		s["b"] = Gaffer.Box()
+		s["b"]["n"] = Gaffer.Node()
+
+		s["b"]["n"]["user"]["p"] = Gaffer.IntPlug( flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic )
+		Gaffer.Metadata.registerPlugValue( s["b"]["n"]["user"]["p"], "layout:section", "SomeWeirdSection" )
+
+		p = s["b"].promotePlug( s["b"]["n"]["user"]["p"] )
+		self.assertNotEqual( Gaffer.Metadata.plugValue( p, "layout:section" ), "SomeWeirdSection" )
+
 if __name__ == "__main__":
 	unittest.main()

src/Gaffer/Box.cpp

@@ -36,6 +36,7 @@
 
 #include "boost/format.hpp"
 #include "boost/algorithm/string/replace.hpp"
+#include "boost/algorithm/string/predicate.hpp"
 #include "boost/regex.hpp"
 
 #include "Gaffer/Box.h"
@@ -485,16 +486,21 @@ std::string Box::promotedCounterpartName( const Plug *plug ) const
 
 void Box::copyMetadata( const Plug *from, Plug *to )
 {
-	/// \todo Perhaps we should have a more general mechanism for mirroring all metadata?
+	/// \todo Perhaps we should have a more dynamic mechanism for mirroring all metadata?
 	/// If we could register a dynamic metadata value for "*", then we could just answer
 	/// all metadata queries on the fly - would that be a good idea? We'd need to figure
 	/// out how to make it compatible with Metadata::registeredPlugValues(), which needs to
-	/// know all valid names.
-	Metadata::registerPlugValue( to, "nodeGadget:nodulePosition", Metadata::plugValue<IECore::Data>( from, "nodeGadget:nodulePosition" ) );
-	Metadata::registerPlugValue( to, "nodule:type", Metadata::plugValue<IECore::Data>( from, "nodule:type" ) );
-	Metadata::registerPlugValue( to, "nodule:color", Metadata::plugValue<IECore::Data>( from, "nodule:color" ) );
-	Metadata::registerPlugValue( to, "connectionGadget:color", Metadata::plugValue<IECore::Data>( from, "connectionGadget:color" ) );
-	Metadata::registerPlugValue( to, "compoundNodule:orientation", Metadata::plugValue<IECore::Data>( from, "compoundNodule:orientation" ) );
-	Metadata::registerPlugValue( to, "compoundNodule:spacing", Metadata::plugValue<IECore::Data>( from, "compoundNodule:spacing" ) );
-	Metadata::registerPlugValue( to, "compoundNodule:direction", Metadata::plugValue<IECore::Data>( from, "compoundNodule:direction" ) );
+	/// know all valid names. We'd also need to put a lot of thought into how we allowed the
+	/// user to delete values which were being mirrored dynamically.
+	vector<IECore::InternedString> keys;
+	Metadata::registeredPlugValues( from, keys, /* inherit = */ true, /* instanceOnly = */ false, /* persistentOnly = */ true );
+	for( vector<IECore::InternedString>::const_iterator it = keys.begin(), eIt = keys.end(); it != eIt; ++it )
+	{
+		if( boost::starts_with( it->string(), "layout:" ) )
+		{
+			// Don't want to copy layout metadata because the user will be making their own layout.
+			continue;
+		}
+		Metadata::registerPlugValue( to, *it, Metadata::plugValue<IECore::Data>( from, *it ) );
+	}
 }