diff --git a/src/Gaffer/ArrayPlug.cpp b/src/Gaffer/ArrayPlug.cpp index 5476f46b781..8450860af1d 100644 --- a/src/Gaffer/ArrayPlug.cpp +++ b/src/Gaffer/ArrayPlug.cpp @@ -104,6 +104,10 @@ bool ArrayPlug::acceptsChild( const GraphComponent *potentialChild ) const return true; } + /// \todo We could beef up these checks to check any descendants of + /// `potentialChild` and to check the default value etc. We can't do that + /// until we fix a few violators of our constraint that all children are + /// identical - see `ShaderQuery::ShaderQuery`. return potentialChild->typeId() == m_elementPrototype->typeId(); } diff --git a/src/Gaffer/ContextQuery.cpp b/src/Gaffer/ContextQuery.cpp index 1aaa295cb27..f387eab28e4 100644 --- a/src/Gaffer/ContextQuery.cpp +++ b/src/Gaffer/ContextQuery.cpp @@ -125,6 +125,7 @@ ContextQuery::ContextQuery( const std::string &name ) : Gaffer::ComputeNode( nam { storeIndexOfNextChild( g_firstPlugIndex ); + /// \todo See notes in `ShaderQuery::ShaderQuery`. addChild( new ArrayPlug( "queries", Plug::Direction::In, nullptr, 1, std::numeric_limits::max(), Plug::Flags::Default, false ) ); addChild( new ArrayPlug( "out", Plug::Direction::Out, nullptr, 1, std::numeric_limits::max(), Plug::Flags::Default, false ) ); } diff --git a/src/GafferBindings/Serialisation.cpp b/src/GafferBindings/Serialisation.cpp index f16e442ccfc..5d40ff6a45b 100644 --- a/src/GafferBindings/Serialisation.cpp +++ b/src/GafferBindings/Serialisation.cpp @@ -82,7 +82,7 @@ namespace // and more readable, and opens the possibility of omitting the // overhead of the names entirely one day. /// \todo Consider an official way for GraphComponents to opt in -/// to this behaviour. +/// to this behaviour. Perhaps this could just be driven by metadata? bool keyedByIndex( const GraphComponent *parent ) { return diff --git a/src/GafferScene/OptionQuery.cpp b/src/GafferScene/OptionQuery.cpp index 7c73b6bbd33..f77c960d962 100644 --- a/src/GafferScene/OptionQuery.cpp +++ b/src/GafferScene/OptionQuery.cpp @@ -135,6 +135,7 @@ OptionQuery::OptionQuery( const std::string &name ) : Gaffer::ComputeNode( name storeIndexOfNextChild( g_firstPlugIndex ); addChild( new ScenePlug( "scene" ) ); + /// \todo See notes in `ShaderQuery::ShaderQuery`. addChild( new ArrayPlug( "queries", Plug::Direction::In, nullptr, 1, std::numeric_limits::max(), Plug::Flags::Default, false ) ); addChild( new ArrayPlug( "out", Plug::Direction::Out, nullptr, 1, std::numeric_limits::max(), Plug::Flags::Default, false ) ); diff --git a/src/GafferScene/PrimitiveVariableQuery.cpp b/src/GafferScene/PrimitiveVariableQuery.cpp index f1828d0b70a..5c3305675a0 100644 --- a/src/GafferScene/PrimitiveVariableQuery.cpp +++ b/src/GafferScene/PrimitiveVariableQuery.cpp @@ -134,6 +134,7 @@ PrimitiveVariableQuery::PrimitiveVariableQuery( const std::string& name ) storeIndexOfNextChild( g_firstPlugIndex ); addChild( new ScenePlug( "scene" ) ); addChild( new Gaffer::StringPlug( "location" ) ); + /// \todo See notes in `ShaderQuery::ShaderQuery`. addChild( new Gaffer::ArrayPlug( "queries", Gaffer::Plug::Direction::In, nullptr, 1, std::numeric_limits< size_t >::max(), Gaffer::Plug::Flags::Default, false ) ); addChild( new Gaffer::ArrayPlug( "out", Gaffer::Plug::Direction::Out, diff --git a/src/GafferScene/ShaderQuery.cpp b/src/GafferScene/ShaderQuery.cpp index 7728dcef569..ef8aa7f813a 100644 --- a/src/GafferScene/ShaderQuery.cpp +++ b/src/GafferScene/ShaderQuery.cpp @@ -141,8 +141,16 @@ ShaderQuery::ShaderQuery( const std::string &name ) : Gaffer::ComputeNode( name addChild( new StringPlug( "location" ) ); addChild( new StringPlug( "shader" ) ); addChild( new BoolPlug( "inherit", Plug::In, false ) ); + /// \todo This is violating the ArrayPlug requirements by not providing an `elementPrototype` + /// at construction. And we later violate it further by creating non-homogeneous elements in + /// `addQuery()`. We're only using an ArrayPlug because we want the serialisation to use numeric + /// indexing for the children of `queries` and `out` - since the serialisation uses `addQuery()` + /// to recreate the children, and that doesn't maintain names. So perhaps we can just use a + /// ValuePlug instead of an ArrayPlug, and add a separate mechanism for requesting that children use + /// numeric indices separately (see `keyedByIndex()` in `Serialisation.cpp`). + /// + /// The same applies to OptionQuery, PrimitiveVariableQuery and ContextQuery. addChild( new ArrayPlug( "queries", Plug::Direction::In, nullptr, 1, std::numeric_limits::max(), Plug::Flags::Default, false ) ); - addChild( new ArrayPlug( "out", Plug::Direction::Out, nullptr, 1, std::numeric_limits::max(), Plug::Flags::Default, false ) ); AttributeQueryPtr attributeQuery = new AttributeQuery( "__attributeQuery" );