Saving the script causes duplicate 'requirements' plugs.

[bentoogood]

To reproduce:

• Open Gaffer, paste in the script below.
• Save as a .gfr file.
• Click render on the RenderManRender node
• Close gaffer
• Reopen gaffer and that file.
• Press render on RenderManRender node again.

You should see a duplicate line has been added below 109

These duplicates appear to accumulate every time the script is reopened and rendered, so might be worth preventing.

import Gaffer
import GafferRenderMan
import GafferScene
import IECore

__children = {}

parent["frameRange"]["start"].setValue( 1 )
parent["frameRange"]["end"].setValue( 100 )
parent["variables"].addChild( Gaffer.CompoundDataPlug.MemberPlug( "projectName", flags = Gaffer.Plug.Flags.Dynamic | Gaffer.Plug.Flags.Serialisable | Gaffer.Plug.Flags.AcceptsInputs | Gaffer.Plug.Flags.PerformsSubstitutions | Gaffer.Plug.Flags.Cacheable, ) )
parent["variables"]["projectName"].addChild( Gaffer.StringPlug( "name", defaultValue = '', flags = Gaffer.Plug.Flags.Dynamic | Gaffer.Plug.Flags.Serialisable | Gaffer.Plug.Flags.AcceptsInputs | Gaffer.Plug.Flags.PerformsSubstitutions | Gaffer.Plug.Flags.Cacheable, ) )
parent["variables"]["projectName"]["name"].setValue( 'project:name' )
parent["variables"]["projectName"].addChild( Gaffer.StringPlug( "value", defaultValue = '', flags = Gaffer.Plug.Flags.Dynamic | Gaffer.Plug.Flags.Serialisable | Gaffer.Plug.Flags.AcceptsInputs | Gaffer.Plug.Flags.PerformsSubstitutions | Gaffer.Plug.Flags.Cacheable, ) )
parent["variables"]["projectName"]["value"].setValue( 'default' )
parent["variables"].addChild( Gaffer.CompoundDataPlug.MemberPlug( "projectRootDirectory", flags = Gaffer.Plug.Flags.Dynamic | Gaffer.Plug.Flags.Serialisable | Gaffer.Plug.Flags.AcceptsInputs | Gaffer.Plug.Flags.PerformsSubstitutions | Gaffer.Plug.Flags.Cacheable, ) )
parent["variables"]["projectRootDirectory"].addChild( Gaffer.StringPlug( "name", defaultValue = '', flags = Gaffer.Plug.Flags.Dynamic | Gaffer.Plug.Flags.Serialisable | Gaffer.Plug.Flags.AcceptsInputs | Gaffer.Plug.Flags.PerformsSubstitutions | Gaffer.Plug.Flags.Cacheable, ) )
parent["variables"]["projectRootDirectory"]["name"].setValue( 'project:rootDirectory' )
parent["variables"]["projectRootDirectory"].addChild( Gaffer.StringPlug( "value", defaultValue = '', flags = Gaffer.Plug.Flags.Dynamic | Gaffer.Plug.Flags.Serialisable | Gaffer.Plug.Flags.AcceptsInputs | Gaffer.Plug.Flags.PerformsSubstitutions | Gaffer.Plug.Flags.Cacheable, ) )
parent["variables"]["projectRootDirectory"]["value"].setValue( '$HOME/gaffer/projects/${project:name}' )
__children["Sphere"] = GafferScene.Sphere( "Sphere" )
parent.addChild( __children["Sphere"] )
__children["Sphere"]["enabled"].setValue( True )
__children["Sphere"]["name"].setValue( 'sphere' )
__children["Sphere"]["transform"]["translate"]["x"].setValue( 0.0 )
__children["Sphere"]["transform"]["translate"]["y"].setValue( 0.0 )
__children["Sphere"]["transform"]["translate"]["z"].setValue( 0.0 )
__children["Sphere"]["transform"]["rotate"]["x"].setValue( 0.0 )
__children["Sphere"]["transform"]["rotate"]["y"].setValue( 0.0 )
__children["Sphere"]["transform"]["rotate"]["z"].setValue( 0.0 )
__children["Sphere"]["transform"]["scale"]["x"].setValue( 1.0 )
__children["Sphere"]["transform"]["scale"]["y"].setValue( 1.0 )
__children["Sphere"]["transform"]["scale"]["z"].setValue( 1.0 )
__children["Sphere"]["type"].setValue( 1 )
__children["Sphere"]["radius"].setValue( 1.0 )
__children["Sphere"]["zMin"].setValue( -1.0 )
__children["Sphere"]["zMax"].setValue( 1.0 )
__children["Sphere"]["thetaMax"].setValue( 360.0 )
__children["Sphere"]["divisions"]["x"].setValue( 20 )
__children["Sphere"]["divisions"]["y"].setValue( 40 )
__children["Sphere"].addChild( Gaffer.V2fPlug( "__uiPosition", flags = Gaffer.Plug.Flags.Dynamic | Gaffer.Plug.Flags.Serialisable | Gaffer.Plug.Flags.AcceptsInputs | Gaffer.Plug.Flags.PerformsSubstitutions | Gaffer.Plug.Flags.Cacheable, ) )
__children["Sphere"]["__uiPosition"]["x"].setValue( -4.4444427490234375 )
__children["Sphere"]["__uiPosition"]["y"].setValue( 10.844447135925293 )
__children["Camera"] = GafferScene.Camera( "Camera" )
parent.addChild( __children["Camera"] )
__children["Camera"]["enabled"].setValue( True )
__children["Camera"]["name"].setValue( 'camera' )
__children["Camera"]["transform"]["translate"]["x"].setValue( 0.0 )
__children["Camera"]["transform"]["translate"]["y"].setValue( 0.0 )
__children["Camera"]["transform"]["translate"]["z"].setValue( 0.0 )
__children["Camera"]["transform"]["rotate"]["x"].setValue( 0.0 )
__children["Camera"]["transform"]["rotate"]["y"].setValue( 0.0 )
__children["Camera"]["transform"]["rotate"]["z"].setValue( 0.0 )
__children["Camera"]["transform"]["scale"]["x"].setValue( 1.0 )
__children["Camera"]["transform"]["scale"]["y"].setValue( 1.0 )
__children["Camera"]["transform"]["scale"]["z"].setValue( 1.0 )
__children["Camera"]["projection"].setValue( 'perspective' )
__children["Camera"]["fieldOfView"].setValue( 50.0 )
__children["Camera"]["clippingPlanes"]["x"].setValue( 0.009999999776482582 )
__children["Camera"]["clippingPlanes"]["y"].setValue( 100000.0 )
__children["Camera"].addChild( Gaffer.V2fPlug( "__uiPosition", flags = Gaffer.Plug.Flags.Dynamic | Gaffer.Plug.Flags.Serialisable | Gaffer.Plug.Flags.AcceptsInputs | Gaffer.Plug.Flags.PerformsSubstitutions | Gaffer.Plug.Flags.Cacheable, ) )
__children["Camera"]["__uiPosition"]["x"].setValue( 9.422225952148438 )
__children["Camera"]["__uiPosition"]["y"].setValue( 11.200000762939453 )
__children["Group"] = GafferScene.Group( "Group" )
parent.addChild( __children["Group"] )
__children["Group"]["enabled"].setValue( True )
__children["Group"]["name"].setValue( 'group' )
__children["Group"]["transform"]["translate"]["x"].setValue( 0.0 )
__children["Group"]["transform"]["translate"]["y"].setValue( 0.0 )
__children["Group"]["transform"]["translate"]["z"].setValue( 0.0 )
__children["Group"]["transform"]["rotate"]["x"].setValue( 0.0 )
__children["Group"]["transform"]["rotate"]["y"].setValue( 0.0 )
__children["Group"]["transform"]["rotate"]["z"].setValue( 0.0 )
__children["Group"]["transform"]["scale"]["x"].setValue( 1.0 )
__children["Group"]["transform"]["scale"]["y"].setValue( 1.0 )
__children["Group"]["transform"]["scale"]["z"].setValue( 1.0 )
__children["Group"].addChild( Gaffer.V2fPlug( "__uiPosition", flags = Gaffer.Plug.Flags.Dynamic | Gaffer.Plug.Flags.Serialisable | Gaffer.Plug.Flags.AcceptsInputs | Gaffer.Plug.Flags.PerformsSubstitutions | Gaffer.Plug.Flags.Cacheable, ) )
__children["Group"]["__uiPosition"]["x"].setValue( 2.4888916015625 )
__children["Group"]["__uiPosition"]["y"].setValue( -0.639439582824707 )
__children["Group"].addChild( GafferScene.ScenePlug( "in1", flags = Gaffer.Plug.Flags.Dynamic | Gaffer.Plug.Flags.Serialisable | Gaffer.Plug.Flags.AcceptsInputs | Gaffer.Plug.Flags.PerformsSubstitutions | Gaffer.Plug.Flags.Cacheable, ) )
__children["Group"].addChild( GafferScene.ScenePlug( "in2", flags = Gaffer.Plug.Flags.Dynamic | Gaffer.Plug.Flags.Serialisable | Gaffer.Plug.Flags.AcceptsInputs | Gaffer.Plug.Flags.PerformsSubstitutions | Gaffer.Plug.Flags.Cacheable, ) )
__children["StandardOptions"] = GafferScene.StandardOptions( "StandardOptions" )
parent.addChild( __children["StandardOptions"] )
__children["StandardOptions"]["enabled"].setValue( True )
__children["StandardOptions"]["options"]["renderCamera"]["name"].setValue( 'render:camera' )
__children["StandardOptions"]["options"]["renderCamera"]["value"].setValue( '/group/camera' )
__children["StandardOptions"]["options"]["renderCamera"]["enabled"].setValue( True )
__children["StandardOptions"]["options"]["renderResolution"]["name"].setValue( 'render:resolution' )
__children["StandardOptions"]["options"]["renderResolution"]["value"]["x"].setValue( 1024 )
__children["StandardOptions"]["options"]["renderResolution"]["value"]["y"].setValue( 778 )
__children["StandardOptions"]["options"]["renderResolution"]["enabled"].setValue( True )
__children["StandardOptions"]["options"]["cameraBlur"]["name"].setValue( 'render:cameraBlur' )
__children["StandardOptions"]["options"]["cameraBlur"]["value"].setValue( False )
__children["StandardOptions"]["options"]["cameraBlur"]["enabled"].setValue( False )
__children["StandardOptions"]["options"]["transformBlur"]["name"].setValue( 'render:transformBlur' )
__children["StandardOptions"]["options"]["transformBlur"]["value"].setValue( False )
__children["StandardOptions"]["options"]["transformBlur"]["enabled"].setValue( False )
__children["StandardOptions"]["options"]["deformationBlur"]["name"].setValue( 'render:deformationBlur' )
__children["StandardOptions"]["options"]["deformationBlur"]["value"].setValue( False )
__children["StandardOptions"]["options"]["deformationBlur"]["enabled"].setValue( False )
__children["StandardOptions"]["options"]["shutter"]["name"].setValue( 'render:shutter' )
__children["StandardOptions"]["options"]["shutter"]["value"]["x"].setValue( -0.25 )
__children["StandardOptions"]["options"]["shutter"]["value"]["y"].setValue( 0.25 )
__children["StandardOptions"]["options"]["shutter"]["enabled"].setValue( False )
__children["StandardOptions"].addChild( Gaffer.V2fPlug( "__uiPosition", flags = Gaffer.Plug.Flags.Dynamic | Gaffer.Plug.Flags.Serialisable | Gaffer.Plug.Flags.AcceptsInputs | Gaffer.Plug.Flags.PerformsSubstitutions | Gaffer.Plug.Flags.Cacheable, ) )
__children["StandardOptions"]["__uiPosition"]["x"].setValue( 2.4888916015625 )
__children["StandardOptions"]["__uiPosition"]["y"].setValue( -6.598423957824707 )
__children["RenderManRender"] = GafferRenderMan.RenderManRender( "RenderManRender" )
parent.addChild( __children["RenderManRender"] )
__children["RenderManRender"]["requirements"].addChild( Gaffer.Plug( "requirement0", flags = Gaffer.Plug.Flags.Dynamic | Gaffer.Plug.Flags.Serialisable | Gaffer.Plug.Flags.AcceptsInputs | Gaffer.Plug.Flags.PerformsSubstitutions | Gaffer.Plug.Flags.Cacheable, ) )
__children["RenderManRender"]["mode"].setValue( 'render' )
__children["RenderManRender"]["ribFileName"].setValue( '${project:rootDirectory}/ribs/${script:name}/${script:name}.####.rib' )
__children["RenderManRender"].addChild( Gaffer.V2fPlug( "__uiPosition", flags = Gaffer.Plug.Flags.Dynamic | Gaffer.Plug.Flags.Serialisable | Gaffer.Plug.Flags.AcceptsInputs | Gaffer.Plug.Flags.PerformsSubstitutions | Gaffer.Plug.Flags.Cacheable, ) )
__children["RenderManRender"]["__uiPosition"]["x"].setValue( 2.4888916015625 )
__children["RenderManRender"]["__uiPosition"]["y"].setValue( -11.969273567199707 )
parent["variables"]["projectName"]["name"].setFlags( Gaffer.Plug.Flags.ReadOnly, True )
parent["variables"]["projectRootDirectory"]["name"].setFlags( Gaffer.Plug.Flags.ReadOnly, True )
__children["Group"]["in"].setInput( __children["Sphere"]["out"] )
__children["Group"]["in1"].setInput( __children["Camera"]["out"] )
__children["StandardOptions"]["in"].setInput( __children["Group"]["out"] )
__children["RenderManRender"]["in"].setInput( __children["StandardOptions"]["out"] )


del __children

[johnhaddon]

I think this in unrelated to actually executing the node - the problem looks to happen during serialisation, and it just happens that at the moment we save the script before rendering. So it should be sufficient to just save the file again to see the problem.

[johnhaddon]

This is also happening for the ImageWriter node, and is probably best addressed as part of the Executable/ExecutableNode refactoring being discussed in #681.

[andrewkaufman]

Adding self.assertEqual( len( s2["n"]["in"] ), 4 ) to the end of ArrayPlugTest.testSerialisation demonstrates this issue.

I originally tried solving this by preventing the last array element from being serialised if it is unconnected and set to the default value. I abandoned that idea when I realized that ValuePlug doesn't actually define getValue or defaultValue, and it would require significant binding re-jigging to figure that out.

Next I started looking into preventing it on scene open (s2.execute in the test). I originally thought the extra element was being created by the InputGenerator, but it turns out it's actually the if( element ) section in the ArrayPlug constructor. This adds the first element (e1), then the next line of the script runs:
__children["n"]["in"].addChild( Gaffer.IntPlug( "e1", minValue = 0, maxValue = 10, flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic, ) )
which tries to add the same element that has just been added, so GraphComponent::addChildInternal renames it as e2. The chain of renaming continues, until you finally end up with one extra element, which is really the last plug you were supposed to add. Any setValue and setInput calls end up affecting the previous element, since they aren't aware the plug was actually renamed. In the end, I think that's alright and the connections and values seem to be correct, we just have this extra element lying around at the end of the array.

Is there a way to detect that we're mid-load inside the ArrayPlug constructor, so we can opt out of adding the default element? Or should I try removing the last element afterwards by hooking into scriptExecutedSignal?

[johnhaddon]

By the time I'd figured out enough to answer your questions I'd figured out a fix anyway, so I've just put in a pull request for this.

As an aside, I'm determined to avoid any kind of areWeLoading()/postConstructor()/scriptExecuted() approaches to things because I think they just lead to more problems in the long run. There are many ways of creating nodes and wiring them together and those concepts just cover some of the situations in which it can happen, and with a fair amount of ambiguity.