Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Core.Data Serialization and Add Remember Node #14111

Merged
merged 46 commits into from
Nov 14, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
ea9afbe
Add Geometry support to the Data Parse / Stringify nodes
saintentropy Jun 7, 2023
d3479fe
Add converters for Color, Image, and Location
saintentropy Jun 7, 2023
0a12332
missing comma
saintentropy Jun 7, 2023
5179b47
Add Remember Node
saintentropy Jun 7, 2023
ef5e9fc
Move serialization logic to LibG
saintentropy Jun 10, 2023
05d5a89
fix order bug
saintentropy Jun 13, 2023
281ac3d
Merge branch 'master' into DYN-5895
saintentropy Jun 23, 2023
22d624c
Fixed typo preventing Remember node from showing up in Dynamo library
LongNguyenP Sep 21, 2023
cdffedb
Merge branch 'master' into DYN-5895
saintentropy Sep 25, 2023
128d133
Merge branch 'master' into DYN-5895
saintentropy Oct 5, 2023
ae9ddbe
Clean up
saintentropy Oct 5, 2023
3eede66
Temp remove ForgeUnits
saintentropy Oct 5, 2023
4ba4038
Merge branch 'master' into DYN-5895
saintentropy Oct 11, 2023
6fe96a1
Merge branch 'master' into DYN-5895
saintentropy Oct 13, 2023
8e0ef8c
Update tests
saintentropy Oct 13, 2023
1f43200
Add back in the Units support
saintentropy Oct 13, 2023
295af29
Fix tests
saintentropy Oct 13, 2023
eafd245
Add icon
saintentropy Oct 14, 2023
233b4c8
Add new types for solids
saintentropy Oct 19, 2023
01454db
Add initial tests
saintentropy Oct 19, 2023
6557cea
Update tests
saintentropy Oct 20, 2023
c7e5043
Add catch and resource strings
saintentropy Oct 20, 2023
9db3562
Add error check in test
saintentropy Oct 20, 2023
c8c3eb9
Fix typos
saintentropy Oct 22, 2023
f092690
Add mesh support back to convertor
saintentropy Oct 22, 2023
d089075
Add os guards for System.Drawing
saintentropy Oct 22, 2023
105a2a6
Missing using
saintentropy Oct 22, 2023
0dabf04
remove unused variable
saintentropy Oct 22, 2023
62a489d
Fix post error update to remove the non-pointer message
saintentropy Oct 22, 2023
9d9e67c
Update tests
saintentropy Oct 22, 2023
cc807f8
Merge branch 'master' into DYN-5895
saintentropy Oct 23, 2023
2637309
Merge branch 'master' into DYN-5895
saintentropy Oct 23, 2023
0be48cd
Remove the warning message intercept
saintentropy Oct 23, 2023
8db0472
Add missing test file
saintentropy Oct 24, 2023
02ff430
Merge branch 'master' into DYN-5895
saintentropy Oct 24, 2023
88c929d
Merge branch 'master' into DYN-5895
saintentropy Oct 25, 2023
5cf2790
Merge branch 'master' into DYN-5895
saintentropy Oct 26, 2023
95c6a48
Added linter rule for obsolete Remember node.
LongNguyenP Nov 1, 2023
9aaf819
Revert "Added linter rule for obsolete Remember node."
saintentropy Nov 1, 2023
76174bd
Merge branch 'master' into DYN-5895
saintentropy Nov 9, 2023
72baac0
PR comments
saintentropy Nov 13, 2023
2f787ee
Update error handling and logging
saintentropy Nov 13, 2023
0a1abb4
Merge branch 'master' into DYN-5895
saintentropy Nov 13, 2023
58f3a9f
Update bad merge conflict fix and update test
saintentropy Nov 13, 2023
43d4f3c
Merge branch 'master' into DYN-5895
saintentropy Nov 13, 2023
cab7d39
Merge branch 'master' into DYN-5895
saintentropy Nov 14, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions src/Libraries/CoreNodeModels/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions src/Libraries/CoreNodeModels/Properties/Resources.en-US.resx
Original file line number Diff line number Diff line change
Expand Up @@ -641,4 +641,13 @@ Default value: {0}</value>
<data name="TooltipTextQuery" xml:space="preserve">
<value>Nodes that query data</value>
</data>
<data name="RememberDescription" xml:space="preserve">
<value>Store data passing through this node to the Dynamo file. Return the stored data if the input is null.</value>
</data>
<data name="RememberInputToolTip" xml:space="preserve">
<value>Data to sample and store in the file.</value>
</data>
<data name="RememberOuputToolTip" xml:space="preserve">
<value>Data</value>
</data>
</root>
9 changes: 9 additions & 0 deletions src/Libraries/CoreNodeModels/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -641,4 +641,13 @@ Default value: {0}</value>
<data name="TooltipTextQuery" xml:space="preserve">
<value>Nodes that query data</value>
</data>
<data name="RememberDescription" xml:space="preserve">
<value>Store data passing through this node to the Dynamo file. Return the stored data if the input is null.</value>
</data>
<data name="RememberInputToolTip" xml:space="preserve">
<value>Data to sample and store in the file.</value>
</data>
<data name="RememberOuputToolTip" xml:space="preserve">
<value>Data</value>
</data>
</root>
148 changes: 148 additions & 0 deletions src/Libraries/CoreNodeModels/Remember.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
using Dynamo.Graph;
using Dynamo.Graph.Nodes;
using Newtonsoft.Json;
using ProtoCore.AST.AssociativeAST;
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;
using System.Reflection;
using System.Text;
using System.Threading.Tasks;
using VMDataBridge;

namespace CoreNodeModels
{
[NodeName("Remember")]
[NodeDescription(nameof(Properties.Resources.RememberDescription), typeof(Properties.Resources))]
[NodeCategory("Core.Data")]
[InPortNames(">")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Node is designed to look like a watch node with ">" port strings

[InPortTypes("object")]
Copy link
Member

@mjkkirschner mjkkirschner Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be var should it not? OR should it be var[]..[] - does this node replicate or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This node does not replicate. We can change the label to whatever we would like. In the Node model case the port names are just for display. What do you think is the oppriate label @mjkkirschner

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the appropriate label for a none replicating node is var[]..[]

[InPortDescriptions(typeof(Properties.Resources), nameof(Properties.Resources.RememberInputToolTip))]
[OutPortNames(">")]
[OutPortTypes("object")]
Copy link
Member

@mjkkirschner mjkkirschner Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be var should it not? OR should it be var[]..[]

[OutPortDescriptions(typeof(Properties.Resources), nameof(Properties.Resources.RememberOuputToolTip))]
[IsDesignScriptCompatible]
[DynamoServices.RegisterForTrace]
mjkkirschner marked this conversation as resolved.
Show resolved Hide resolved
public class Remember : NodeModel
{
private bool _deactivateNode;
private string _cache = "";
private string _updatedToolTipText = "";
saintentropy marked this conversation as resolved.
Show resolved Hide resolved

[JsonProperty("Cache")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you need this when the property has the same name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

public string Cache
{
get { return _cache; }
set
{
var valueToSet = value == null ? "" : value;
if (valueToSet != _cache)
{
_cache = valueToSet;
MarkNodeAsModified();
}
}
}

[JsonConstructor]
private Remember(IEnumerable<PortModel> inPorts, IEnumerable<PortModel> outPorts) : base(inPorts, outPorts)
{
PropertyChanged += OnPropertyChanged;
}

public Remember()
{
RegisterAllPorts();
PropertyChanged += OnPropertyChanged;
}

private void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
{
switch (e.PropertyName)
{
case "State":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nameof?

if (State == ElementState.Warning)
{
Cache = "";
}
break;

case "ToolTipText":
if (State == ElementState.Warning && ToolTipText != _updatedToolTipText)
{
string[] errorMessages =
ToolTipText.Split(new[] { "\r\n", "\n" }, StringSplitOptions.None);
_updatedToolTipText = errorMessages.Last();
ToolTipText = _updatedToolTipText;
}

break;

default:
// Nothing to handle
break;
}
}

protected override void OnBuilt()
{
base.OnBuilt();
DataBridge.Instance.RegisterCallback(GUID.ToString(), DataBridgeCallback);
}

public override void Dispose()
{
base.Dispose();
DataBridge.Instance.UnregisterCallback(GUID.ToString());
}

private static readonly string BuiltinDictionaryTypeName = typeof(DesignScript.Builtin.Dictionary).FullName;
aparajit-pratap marked this conversation as resolved.
Show resolved Hide resolved
private static readonly string BuiltinDictionaryGet = nameof(DesignScript.Builtin.Dictionary.ValueAtKey);

public override IEnumerable<AssociativeNode> BuildOutputAst(List<AssociativeNode> inputAstNodes)
{
// Check that node can run
if (_deactivateNode)
{
return new[] { AstFactory.BuildAssignment(GetAstIdentifierForOutputIndex(0), AstFactory.BuildNullNode()) };
}

var resultAst = new List<AssociativeNode>();

var funtionInputs = new List<AssociativeNode> { inputAstNodes[0], AstFactory.BuildStringNode(Cache) };

//First build the function call
var functionCall = AstFactory.BuildFunctionCall(
new Func<object, string, Dictionary<string, object>>(DSCore.Data.Remember), funtionInputs);

var functionCallIndent = AstFactory.BuildIdentifier(GUID + "_func");

resultAst.Add(AstFactory.BuildAssignment(functionCallIndent, functionCall));

//Next add the first key value pair to the output port
var getFirstKey = AstFactory.BuildFunctionCall(BuiltinDictionaryTypeName, BuiltinDictionaryGet,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the ASTFactory somewhere has methods for building these calls easily.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that I am aware of or could find. The main thing we do here is that we need to pass the data from the inputPort and the cached string into and back out of the node every time. The dictionary is used so that we can only present to the ports data on the node and keep the cache data internal. I think we look long and hard at the time we wrote this and didn't have any out of the box options of dictionaries in ASTFactory.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

look for:
DictionaryExpressionBuilder

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh. Thanks. I did look at that. Interesting but not exactly what we need. The dictionary is already set up by the function we call Remember() as the return type. The AST construction we are doing is to look up specific keys later... More specifically DictionaryBuilder embeds the constuctor DesignScript.Builtin.Dictionary.ByKeysValues whereas we need the method DesignScript.Builtin.Dictionary.ValueAtKey. There could be a helper for us but not this one.

new List<AssociativeNode> { functionCallIndent, AstFactory.BuildStringNode(">") });

resultAst.Add(AstFactory.BuildAssignment(GetAstIdentifierForOutputIndex(0), getFirstKey));

//Second get the key value pair to pass to the databridge callback
var getSecondKey = AstFactory.BuildFunctionCall(BuiltinDictionaryTypeName, BuiltinDictionaryGet,
new List<AssociativeNode> { functionCallIndent, AstFactory.BuildStringNode("Cache") });

resultAst.Add(AstFactory.BuildAssignment(
AstFactory.BuildIdentifier(GUID + "_db"),
DataBridge.GenerateBridgeDataAst(GUID.ToString(), getSecondKey)));

return resultAst;
}

private void DataBridgeCallback(object callbackObject)
{
if (DSCore.Data.CanObjectBeCached(callbackObject))
{
Cache = callbackObject as String;
}
}
}
}
5 changes: 5 additions & 0 deletions src/Libraries/CoreNodes/CoreNodes.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@
<Project>{c0d6dee5-5532-4345-9c66-4c00d7fdb8be}</Project>
<Name>DesignScriptBuiltin</Name>
</ProjectReference>
<ProjectReference Include="..\DynamoUnits\UnitsCore.csproj">
mjkkirschner marked this conversation as resolved.
Show resolved Hide resolved
<Project>{6e0a079e-85f1-45a1-ad5b-9855e4344809}</Project>
<Name>Units</Name>
<Private>False</Private>
mjkkirschner marked this conversation as resolved.
Show resolved Hide resolved
</ProjectReference>
</ItemGroup>
<ItemGroup>
<Content Include="DSCoreNodes.Migrations.xml">
Expand Down
Loading
Loading