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

DYN-6602 Make Dynamo TSpline nodes OOTB and hide experiment toggle under feature flag #14923

Merged
merged 7 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 18 additions & 0 deletions src/DynamoCore/Configuration/PreferenceSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1141,6 +1141,9 @@ internal static string GetDefaultPythonEngine()
return defaultPythonEngine;
}

/// <summary>
/// Initialize namespaces to exclude from Library based on conditions
/// </summary>
internal void InitializeNamespacesToExcludeFromLibrary()
{
if (!NamespacesToExcludeFromLibrarySpecified)
Expand All @@ -1152,6 +1155,21 @@ internal void InitializeNamespacesToExcludeFromLibrary()
}
}

/// <summary>
/// Update namespaces to exclude from Library based on feature flags
/// </summary>
internal void UpdateNamespacesToExcludeFromLibrary()
{
// When the experiment toggle is disabled by feature flag, include the TSpline namespace from the library OOTB.
if (!DynamoModel.FeatureFlags?.CheckFeatureFlag("IsTSplineNodesExperimentToggleVisible", false) ?? false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use preferenceSettings.IsTSplineNodesExperimentToggleVisible property here?

{
NamespacesToExcludeFromLibrary.Remove(
"ProtoGeometry.dll:Autodesk.DesignScript.Geometry.TSpline"
);
return;
}
}

//migrate old path token to new path token
private static void MigrateStdLibTokenToBuiltInToken(PreferenceSettings settings)
{
Expand Down
14 changes: 13 additions & 1 deletion src/DynamoCore/Models/DynamoModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
using Dynamo.Selection;
using Dynamo.Utilities;
using DynamoServices;
using DynamoUtilities;
using Greg;
using Lucene.Net.Documents;
using Lucene.Net.Index;
Expand Down Expand Up @@ -716,7 +717,7 @@ protected DynamoModel(IStartConfiguration config)
{
//this will kill the CLI process after cacheing the flags in Dynamo process.
using (FeatureFlags =
new DynamoUtilities.DynamoFeatureFlagsManager(
new DynamoFeatureFlagsManager(
AnalyticsService.GetUserIDForSession(),
mainThreadSyncContext,
IsTestMode))
Expand All @@ -728,6 +729,7 @@ protected DynamoModel(IStartConfiguration config)
}
catch (Exception e) { Logger.LogError($"could not start feature flags manager {e}"); };
});
DynamoFeatureFlagsManager.FlagsRetrieved += HandleFeatureFlags;
Copy link
Contributor Author

@QilongTang QilongTang Feb 7, 2024

Choose a reason for hiding this comment

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

Add feature flag event based TSpline nodes show/hide. FYI: @reddyashish @mjkkirschner

}

// TBD: Do we need settings migrator for service mode? If we config the docker correctly, this could be skipped I think
Expand Down Expand Up @@ -975,6 +977,15 @@ protected DynamoModel(IStartConfiguration config)
DynamoReady(new ReadyParams(this));
}

/// <summary>
/// When feature flags received, handle them and make changes
/// </summary>
private void HandleFeatureFlags()
{
PreferenceSettings.UpdateNamespacesToExcludeFromLibrary();
return;
}

private void HandleAnalytics()
{
if (IsTestMode)
Expand Down Expand Up @@ -1367,6 +1378,7 @@ public void Dispose()
LibraryServices.LibraryLoaded -= LibraryLoaded;

EngineController.VMLibrariesReset -= ReloadDummyNodes;
DynamoFeatureFlagsManager.FlagsRetrieved -= HandleFeatureFlags;

Logger.Dispose();

Expand Down
11 changes: 11 additions & 0 deletions src/DynamoCoreWpf/ViewModels/Menu/PreferencesViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1121,6 +1121,17 @@ public bool NodeAutocompleteMachineLearningIsBeta
}
}

/// <summary>
/// Controls if the TSpline nodes experiment toggle is visible from feature flag
/// </summary>
public bool IsTSplineNodesExperimentToggleVisible
{
get
{
return DynamoModel.FeatureFlags?.CheckFeatureFlag("IsTSplineNodesExperimentToggleVisible", false) ?? false;
}
}

/// <summary>
/// Contains the numbers of result of the ML recommendation
/// </summary>
Expand Down
5 changes: 4 additions & 1 deletion src/DynamoCoreWpf/Views/Menu/PreferencesView.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -1023,7 +1023,10 @@
<RowDefinition Height="Auto"/>
</Grid.RowDefinitions>

<StackPanel Orientation="Horizontal" Margin="0,12,0,0" Grid.Row="0">
<StackPanel Orientation="Horizontal"
Margin="0,12,0,0"
Grid.Row="0"
Visibility="{Binding IsTSplineNodesExperimentToggleVisible, Converter={StaticResource BooleanToVisibilityConverter}}">
<ToggleButton Name="EnableTSplineToggle"
Width="{StaticResource ToggleButtonWidth}"
Height="{StaticResource ToggleButtonHeight}"
Expand Down
8 changes: 4 additions & 4 deletions test/DynamoCoreWpfTests/NodeAutoCompleteSearchTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public void NodeSuggestions_CanAutoCompleteOnCustomNodesOutPort_WithSpaceInPortN

// Results will be nodes that accept Line as parameter.
searchViewModel.PopulateAutoCompleteCandidates();
Assert.AreEqual(44, searchViewModel.FilteredResults.Count());
Assert.AreEqual(58, searchViewModel.FilteredResults.Count());
}
[Test]
public void NodeSuggestions_CanAutoCompleteOnCustomNodesOutPort_WithWhiteSpaceStartingPortName()
Expand All @@ -186,7 +186,7 @@ public void NodeSuggestions_CanAutoCompleteOnCustomNodesOutPort_WithWhiteSpaceSt

// Results will be nodes that accept Line as parameter.
searchViewModel.PopulateAutoCompleteCandidates();
Assert.AreEqual(44, searchViewModel.FilteredResults.Count());
Assert.AreEqual(58, searchViewModel.FilteredResults.Count());
}

[Test]
Expand Down Expand Up @@ -316,7 +316,7 @@ public void NodeSuggestions_OutputPortBuiltInNode_AreCorrect()

// Get the output port type for the node.
var outPorts = nodeView.ViewModel.OutPorts;
Assert.AreEqual(1, outPorts.Count());
Assert.AreEqual(1, outPorts.Count);

var port = outPorts[0].PortModel;
var type = port.GetOutPortType();
Expand All @@ -326,7 +326,7 @@ public void NodeSuggestions_OutputPortBuiltInNode_AreCorrect()
var searchViewModel = ViewModel.CurrentSpaceViewModel.NodeAutoCompleteSearchViewModel;
searchViewModel.PortViewModel = outPorts[0];
var suggestions = searchViewModel.GetMatchingSearchElements();
Assert.AreEqual(44, suggestions.Count());
Assert.AreEqual(58, suggestions.Count());
}

[Test]
Expand Down
Loading