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

DUI: Re-implement logic of getting know ElementType of each node #4190

Merged
merged 14 commits into from Apr 30, 2015

Conversation

aosyatnik
Copy link

This is the first PR, that is connected to task "Show category type on hover mouse".

Purpose

On hover over the library item, the type of add-on is display to its right. PKG / DLL
image

If Package contains custom nodes, these custom nodes consider as package members. The same with dlls. (if package contains dll, this dll considers as package member)

Solution

I want to add new property ElementType, which can be RegularNode, RegularCategory, CustomNode, CustomDll, Package.

Overview

If user imports custom dll, it should be consider as custom dll.
If user opens custom node, it should be consider as custom node.
If user loads package(with custom dlls and custom nodes), they should be consider as package members.

BUT for Dynamo there is no difference, if custom node/dll is part of package or not.
That's why I added new properties IsPackageMember and IsBuiltIn for Custom node and Usual node respectively.

Since Dynamo uses the same functions to load custom nodes/dlls during importing Packages and simple importing/opening, the only posibily, to determine is it member of Package or not, is, when we call Package Manager, say to loading method THIS IS Package Member.

Reviewers

@Benglin , please take a look.

Link to YouTrack:
MAGN-5740 DUI: Re-implement logic of getting know ElementType of each node

@@ -504,7 +504,7 @@ protected DynamoModel(IStartConfiguration config)
PackageLoader.MessageLogged += LogMessage;
PackageLoader.RequestLoadNodeLibrary += LoadNodeLibrary;
PackageLoader.RequestLoadCustomNodeDirectory +=
(dir) => this.CustomNodeManager.AddUninitializedCustomNodesInPath(dir, isTestMode);
(dir) => this.CustomNodeManager.AddUninitializedCustomNodesInPath(dir, isTestMode, true);
Copy link
Author

Choose a reason for hiding this comment

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

If custom node is loaded with PackageLoader it's package member!

@Benglin Benglin self-assigned this Apr 10, 2015
else
return ElementTypeEnum.RegularCategory;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use Any here instead of All and not iterate through all the sub elements? Also, the last return can now be ElementTypes.None after the above change.

Copy link
Author

Choose a reason for hiding this comment

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

Hi, @Benglin . I thought about that and had little conversation with @elayabharath .
We can have next situation: in "Core" user adds some custom node or custom dll. But even with new added node/dll Core should remain as regular category... As well as other builtin categories e.g. operators, geometry.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, so we are going to just return ElementTypes.None here always?

Copy link
Author

Choose a reason for hiding this comment

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

I think next structure:
Core -> ElementTypes.BuiltIn
Core, inside of which added some custom node -> ElementTypes.None
Category with just user's custom nodes -> ElementTypes.CustomNode
Category with just user's imported assembly -> ElementTypes.ZeroTouch
Rhynamo -> ElementTypes.Packaged

I think we can use None for mixed categories...

@@ -795,6 +795,7 @@ private void LoadNodeLibrary(Assembly assem)
{
NodeFactory.AddTypeFactoryAndLoader(type.Type);
NodeFactory.AddAlsoKnownAs(type.Type, type.AlsoKnownAs);
type.IsPackageMember = true;
Copy link
Author

Choose a reason for hiding this comment

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

Case for "Interactive" dlls. E.g. Color Range.

return ElementTypes.None;
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer this to be done less frequently, I'm imagining this ElementType to be called each time the UI needs the information to display an icon, then that might be too many evaluation on these collections entries and subCategories. If we can do them when these two collection changed, then it's one-time evaluation:

private void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
{
    // Perform the "entries.Any" and "subCategories.Any" calls here, then set "ElementType"...
    if (entries.Any ... && subCategories.Any ...)
        ElementType = ElementTypes.BuiltIn;
}

public ElementTypes ElementType { get; private set; }

If you can do a count and see if we get more calls for ElementType.get or OnCollectionChanged, then decide which is the best way to go about doing it. Please let me know.

Copy link
Author

Choose a reason for hiding this comment

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

You are right... I tried. Element type called MUCH frequently then Collection Changed.
image
First number is ElementTypeCalled, second number is CollectionChanged.

@aosyatnik
Copy link
Author

Hi, @Benglin .
Finally I found the best solution of how and where to determine ElementType!
The best solution to determine it during mouse over UI element, BUT I have to determine it just ONCE. That means if type is None, we have to fire DetermineElementType(). So, there is no need to determine type everytime mouse moves into control.
I also tried to determine type during CollectionChanged event. But it turned out, that it takes more iterations, than the first case.

@Benglin
Copy link
Contributor

Benglin commented Apr 29, 2015

Thanks @aosyatnik, this looks good. I have two questions:

  1. Is BuiltIn type really needed and used anywhere?
  2. I don't see any XAML file update, is that coming from another PR later?

@aosyatnik
Copy link
Author

Thanks @Benglin .
IsBuiltIn is used in FunctionDescriptor.

if (functionDescriptor.IsBuiltIn)
               ElementType |= ElementTypes.BuiltIn;

BuiltIn type is used to determine whether zerotouch library(e.g. geometry) comes with dynamo or was imported by user.

Xaml will be in next PR.

@Benglin
Copy link
Contributor

Benglin commented Apr 30, 2015

Thanks for doing this, @aosyatnik! LGTM.

Benglin added a commit that referenced this pull request Apr 30, 2015
DUI: Re-implement logic of getting know ElementType of each node
@Benglin Benglin merged commit b7b146c into DynamoDS:master Apr 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants