"Default value for parameter", serialization, and 2.7 / 2.8 differences

[kalms]

Hi,

it appears that FlowCanvas 2.7 used to serialize inputPortValues, both if they had their default values, or if they had been set to something non-default. However, with FlowCanvas 2.8, the default values do not get serialized.

This is understandable, and it should not make any difference, except that for a few ValueInput<bool> we have wanted to change the default for the parameter to true. With FlowCanvas 2.7 we used to do it like this:

1 2 3 4 5 6 7 8

protected override void RegisterPorts()     {         base.RegisterPorts();         ignoreSelfInput = AddValueInput<bool>("Ignore Self");         ignoreSelfInput.serializedValue = true;         ...      }

The above worked with FlowCanvas 2.7 because: at node instantiation time, RegisterPorts() would be run, but not deserializer. That would leave the input port set to true. At load time, RegisterPorts() would first get run, followed by deserialization – which would always write a false or true to the input port. Since FlowCanvas 2.8 no longer serializes/deserializes the value of any port which is set to a default value, this breaks.

We can make our project work again, by removing any ...serializedValue = true we have in any RegisterPorts() implementation, it is not a biggie.

I would however like to hear what the intentions are for default value handling within FlowCanvas. Here are my thoughts.

First: this is how I think that FlowCanvas works today.

Default values are always the type’s default. If someone changes the implementation of the default constructor for a type, then any input values which were previously set to the default value will instantly, and retroactively, change to the new default value.

When values are serialized/deserialized, they will only be serialized if they happen to be set to the current default value. If they are deserialized in the future, any default values will be deserialized to the new constructor’s values.

I believe this is not an optimal approach. There is no way for the flow user (nor for the new-node developer) to differentiate between the user not having set a value for an input, and a user deliberately set the value of the input to the default value. Changes in default constructors will then apply to existing flows.

I think there are two different things at play here. First, the idea that of int “0” and bool “false” being treated as special default values. This is dangerous. In an ideal world, default-ness should be an attribute separate from the value itself. (so a value can be ‘set’ and then it has a value – which could be equal to, or different to, the current default – or the value is ‘unset’ and then it will automatically assume the current default when read) I understand that this adds processing & memory overhead, but I think it would be a better conceptual model for FlowCanvas as a language.

The second question is when a “default” value should be applied. Should it be applied at the moment that a node is added to a flow by a user? Or should it be applied whenever a graph is deserialized?

I see the following combinations making sense:
– Default-ness is not a separate attribute (like it is in FlowCanvas today & before), but all parameter values are frozen at the time that you create a node (like it was in FlowCanvas 2.7)
– Default-ness is a separate attribute, and parameter values are frozen at the time that you create a node
– Default-ness is a separate attribute, and default parameter values will be modified when default constructors change

The one combination that does not make sense to me is what we have with FlowCanvas 2.8:
– Default-ness is not a separate attribute, and default parameter values will be modified when default constructors change

Finally, it might be useful to have a feature to be able to set the default value for an input separate from the default constructor, i.e. allow an input value – say, an integer – to be set to something else than “0” at the time when a new node is added to a graph, through an additional argument to AddValueInput<type>(name) . I am not 100% certain what the relation would be between this or the default value concept.

[kalms]

Or, alternatively: abolish the current ‘default’ concept. When a node is added to a flow graph, all of its parameters will be given values somehow (typically assigned the constructor default values), and all these values will be saved by serialization. This ensures that old flows don’t change functionality when someone changes default values.

With that model, the developer of a new node type could still specify defaults for a value through some mechanism, but it should only be applied when the user adds new nodes, or the first time that a user loads an old graph which does not include values for all the inputs which exist in the latest version of a node.

The ‘default value’ concept i described here last is different from the default function argument concept that is used in C# but I think this variation maps better to visual programming languages. It also has the benefit that there is no need for a ‘set’ / ‘not set’ kind of flag separate from the actual value.

[kalms]

After a good night’s sleep I have a better understanding of what the ‘no distinct “default value”‘ model would mean.

Description:

(I think this is how NodeCanvas 2.7 worked)

When you add a new node to a flow graph, the node’s value inputs will be assigned values according to default(T) for each input.
All the node values will be serialized and deserialized, even values that happen to match default(T) at the time of serialization.

If a node is deserialized and there is information missing for some value inputs (because new value inputs have been added to the node’s C# implementation), then default(T) will be assigned as the node’s value input.

The UI will display values for value inputs that match default(T) in discrete semi-transparent text, and values that do not match in strong non-transparent text.

There is a possibility here to support the feature of allowing the C# programmer to change the default value for value inputs. With This would be done not by modifying the class’s default constructor, but either by adding extra arguments to the ValueInput<> type or by keeping the default-value as a member of each ValueInput object. This programmer-specified value would override the use of default(T) in all places in the above text.

It means that if a C# programmer modifies the default value for an existing value input, then it will only affect new nodes, and nodes which were serialized with an old format where the value input didn’t exist. If the FlowGraph programmer has ever seen (and saved) the node with a certain value displayed for a value input, that value will remain fixed.

For FlowCanvas 2.7 we could sort-of accomplish this this by modifying .serializedValue for an input during RegisterPorts(). However, that did not propagate to the UI as described below (which would make the logical model more consistent):

The UI’s semi-transparent colouring of a value input means: The value of this value input matches the C#-programmer-specified default value right now. It does not mean that the value is the default value, nor that it changes along with the default value. A “Reset to default” function could exist, which would change any value input to its current default value.

[kalms]

OK, summary time:

* NodeCanvas 2.8 not serializing value inputs equal to default(T) has broken a number of our FlowGraphs. We will go through all our c# node implementations, remove any use of .serializedValue during RegisterPorts() to change ports’ default values, and then go through all our FlowGraphs and ensure that their settings are correct.

* We would like a future update of NodeCanvas to go back to serializing value inputs equal to default(T). At that point, we can reintroduce our current default-value mechanism and continue working like we did up until the 2.8 release.

* We would like a future update NodeCanvas to offer a default-value option that works as described toward the end of my last post — assuming you too think it makes sense for the language & editor.

[Gavalakis]

Hello!

First of all thanks for the detailed and thorough posts.

Indeed, in v2.8 default(T) values are no longer serialized to save up on performance. Basically, the important piece of code for this change is in FlowNode.cs line #75, where it reads if (!port.isConnected && !port.isDefaultValue){. Removing the !port.isDefaultValue will basically revert to how it used to work in 2.7.

Avoiding serialization of default values is something good generally speaking. Based on your posts though, I understand that the ability to provide custom defaults can be quite useful as well. Thankfully, after a few minutes of thought, implementing this seems quite easy, albeit you will need a few changes to your code though.

—1st Solution—-
For the implementation, please open up Ports.cs file and at line about #300 of the ValueInput class..

1) Add this piece of code:

1 2 3 4 5 6 7

private T _defaultValue = default(T); public T defaultValue{ get {return _defaultValue;} set {_defaultValue = value; serializedValue = value;} }

2) Replace existing isDefaultValue with this:

1 2 3 4 5

public override bool isDefaultValue{ get { return Equals(_value, defaultValue); } }

That’s basically it 🙂
Then within your nodes, instead of setting/using serializedValue, use defaultValue instead:

1 2 3 4 5 6 7 8 9 10 11 12 13

public class TestNode : FlowNode {   protected override void RegisterPorts(){   var input = AddValueInput<string>("Value"); input.defaultValue = "Hello";   var trigger = AddValueInput<bool>("Trigger"); trigger.defaultValue = true; } }

– Custom default values set, will now show in transparent grey as well, instead of only default(T).
– Changing the defaultValue of a node of which it’s serializedValue has not changed for example from within the inspector, will also propagate the default value. What I mean is, that in the above example the string defaultValue = "Hello". If we add a bunch of TestNodes in a graph and do not change their string value input from inspector for example and at a later time the node implementation change so that defaultValue = "World", all “untouched” nodes will also change their input to the new default “World”. Now, this last part might not seem ideal.

—2nd Solution—-
Another completely different solution (disregard all the above, would be to set the serializedValue in the OnCreate method callback (this is what is actually done in SimplexNodes). This is naturally only called when the node is created (but after RegisterPorts).

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17

public class TestNode : FlowNode {   private ValueInput<string> text; private ValueInput<bool> trigger;   public override void OnCreate(Graph assignedGraph){ text.serializedValue = "Hello"; trigger.serializedValue = true; }   protected override void RegisterPorts(){ text = AddValueInput<string>("Value"); trigger = AddValueInput<bool>("Trigger"); } }

—3rd Solution—-
Lastly, the simplest of all solution, would be to simply remove the !port.isDefaultValue in FlowNode.cs #75, which is basically how v2.7 worked before.

Please let me know what you think on the above. Does the 1st solution sounds good to you?
Thank you!

Join us on Discord: https://discord.gg/97q2Rjh

[kalms]

Hi — you say that:

Avoiding serialization of default values is something good generally speaking.

I think you mean good, from a performance point of view. However, I am not convinced that it is good from a usability point of view.

I think what made this problematic has to do with the concept ‘default value’ being used for two things at the same time in this discussion: 1) the default value of a datatype, which can be relied upon as a performance optimization (and this is what 2.8 does) and 2) the default value of a parameter, which can be used as a convenience mechanism for the user when adding new nodes to flows.

Optimizations that ignore serializing/deserializing default(T) values are fine. If done correctly they should never affect functionality, just improve performance.

So what is the ‘default value of a parameter’ then? It is a different concept than default(T). It should be a value that is assigned to the parameter when adding a new node to a graph (which I presume is what the OnCreate() solution will do). It should ideally also be accessible to a ‘reset parameter to default’ UI feature, and provide grey/white coloring to tell whether the current value matches the ‘default value of the parameter’.

If people were to use the ‘default value of a parameter’ feature a lot, then the don’t-serialize-default(T)-values optimization becomes less effective. However, that is probably not a big problem in practice.

Based on this, I do not like solution 1. It looks nice on the surface, but it has the problem with tying default(T) and default-value-of-parameter together and this causes problem retroactively when changing the default-value-in-parameter in the C# code after a node has already come into use.

I like solution 2, because it is small and it does not come with any gotchas. We will likely go with that approach. At this point I must double-check: OnCreate() isn’t called when deserializing a saved node, is it? because if it is called at each deserialization then this too would suffer from the same problem as solution 1.

As for solution 3, that is also a viable option to us, but we would prefer not to have any modifications in core FlowCanvas – which is why we solution 2.

[Gavalakis]

Hello again and thanks for the follow up.

Yes, OnCreate is indeed only called when the node is created and only once when it does. Not every time the node deserialize, so that is a safe place to set the default serialized values.
A UI option for reseting the node to default values could follow in the next version. The only problem in that, is that the “OnCreate” no longer makes a lot of sense, but rather a “Reset” method would make more sense (akin to Unity’s Reset method callback). I could of course call “OnCreate” when we click the Reset option in the UI, but it’s not very consistent, thus I would prefer adding a “Reset” callback as well for this case.

Let me know what you think.
Thank you 🙂

Join us on Discord: https://discord.gg/97q2Rjh

[kalms]

Yes, OnCreate is indeed only called when the node is created and only once when it does. Not every time the node deserialize, so that is a safe place to set the default serialized values.

Cool, thanks, then we will use that to set defaults for now.

A UI option for reseting the node to default values could follow in the next version. The only problem in that, is that the “OnCreate” no longer makes a lot of sense, but rather a “Reset” method would make more sense (akin to Unity’s Reset method callback). I could of course call “OnCreate” when we click the Reset option in the UI, but it’s not very consistent, thus I would prefer adding a “Reset” callback as well for this case.

I am doubtful about the usefulness of having a reset option that resets all inputs for an entire node. Usually when I want to reset something, I want to reset only a single ValueInput, not all the node’s inputs. Also, before I decide to reset the value of a node, I would like to be able to see whether or not it already has the default value (grey/white coloring). Introducing a ‘Reset node to defaults’ UI mechanism does not solve any of those problems. Therefore I don’t think it is worth implementing a “Reset” method for an entire node.

[Author]

Posts