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

Namespace Collisions with 3rd Party Packages (e.g. Rhynamo) #3208

Closed
andydandy74 opened this issue Nov 21, 2014 · 13 comments
Closed

Namespace Collisions with 3rd Party Packages (e.g. Rhynamo) #3208

andydandy74 opened this issue Nov 21, 2014 · 13 comments
Labels

Comments

@andydandy74
Copy link
Contributor

I noticed this when wanting to create points. Autocomplete behaviour before installing Rhynamo:
ac1
And after:
ac2
So Point now needs to be called as Autodesk.DesignScript.Geometry.Point. Which is not good at all because it means existing scripts won't execute:
acwarning
I find it odd (and actually a bit disconcerting) that third party DLLs can have that kind of effect on Dynamo core functionality.
CC @archinate

@sharadkjaiswal
Copy link
Contributor

Thanks for reporting this issue. We are aware of this problem and working on user story MAGN-5380 to resolve this.
When there are multiple types with the same name, we need to display all the types and to distinguish we start showing them with their complete namespace.

@archinate
Copy link

After doing some digging, in this case I believe the issue actually stems from Dynamo getting confused by the OpenNURBS library (Rhino3dmIO.dll) dependency and is introducing conflicting types. This is very odd because OpenNURBS is being referenced by Rhynamo and should not be referenced by Dynamo directly.

I agree that a third party DLL should NOT impact Dynamo core functionality in this manner... but it appears that it does.

At the most basic level, Rhynamo converts between Rhino types and Dynamo types (ProtoCore). When coding Rhynamo, the type "Point" or "NurbsSurface" or "Curve" are all ambiguous and I need to be explicit within my own code...

For example Autodesk.DesignScript.Geometry.Point vs. Rhino.Geometry.Point

Again... I have no idea why Dynamo, itself, would get confused since OpenNURBS is a Rhynamo dependency and should not be referenced by Dynamo.

@aparajit-pratap
Copy link
Contributor

@archinate I see that Rhynamo has nodes that convert between types such as Rhino.Geometry.Point and Autodesk.DesignScript.Geometry.Point. Even though the Rhino.Geometry.Point is not explicitly imported into Dynamo, it needs to be aware of this type as it's being passed around by other Rhynamo nodes. In order to support this, the Zero Touch system in Dynamo creates empty (dummy) types by the name of Rhino.Geometry.Point etc., which then begin to conflict with native types in Dynamo. While this maybe an answer to your question, I agree it's not a solution. We are working on that.

@andydandy74
Copy link
Contributor Author

Here's a snippet from the 0.7.5 release notes:

Existing Code Block Nodes are no longer affected by name collisions with functions that come from installed packages. 
For instance, Point.ByCoordinates was affected by a collision with a Point. operation in the popular Rhynamo package and would throw an error saying 
“Warning: Dereferencing a non-pointer. Dereferencing a non-pointer.”
Existing Code Block Nodes and new code block nodes now use unique names for each class.

The autocomplete bug is indeed gone, but this is what happens with Rhynamo installed in 0.7.5 RC:
capture
The warning is still about dereferencing a non-pointer.
Without Rhynamo installed, the Point.ByCoordinates command in the CBN computes just fine.

@andydandy74
Copy link
Contributor Author

The same seems to happen with the Document type when DynaWorks is installed.

@aparajit-pratap
Copy link
Contributor

The fix for this issue is still pending design review and is not completely implemented yet. However on importing a new library that has namespace conflicts with classes used in existing code block nodes, you will now see these nodes emit warnings saying the classes used in them now have conflicts. For example, if you import Rhynamo, you will see an existing CBN using 'Point' class now emit a warning saying:
"Warning: Multiple definitions for 'Point' are found as Autodesk.DesignScript.Geometry.Point, Rhino.Geometry.Point"
Again you won't see this if "Run Automatically" is turned on as the node is executed even before this warning can be displayed and it results in a "Dereferening a non-pointer" runtime warning instead.

Agree that the release notes are not quite accurate in this case.

@andydandy74
Copy link
Contributor Author

Then how about listing this as a known issue for 0.7.5 - assuming this fix will not make it into the 0.7.5 release?

@andydandy74
Copy link
Contributor Author

Glad to see that this is partially fixed in 0.8 RC.
Here's a description of the new behaviour:

  1. Entering Point in a CBN for the first time will still yield the following result in autocomplete:
    point-autocomplete
    (So at that point it still looks broken...)
  2. Entering Point. will make Dynamo autocomplete this to PointAnalysisData. - not quite what I was after.
  3. Manually correcting this again to Point. will bring up the following message in the console:
    Failed to perform code block autocomplete with exception: Value cannot be null. Parameter name: source at System.Linq.Enumerable.Select[TSource,TResult](IEnumerable1 source, Func2 selector) at Dynamo.DSEngine.CodeCompletion.CodeCompletionServices.GetCompletionsOnType(String code, String stringToComplete, ElementResolver resolver) at Dynamo.UI.Controls.CodeBlockEditor.GetCompletionData(String code, String stringToComplete) at Dynamo.UI.Controls.CodeBlockEditor.OnTextAreaTextEntered(Object sender, TextCompositionEventArgs e)
  4. I can manually complete the statement like so: Point.ByCoordinates(0,0,0); and it will execute. Nice!
  5. And now it gets really strange: After having typed in the command manually once, it will appear in the autocomplete suggestions without Autodesk.DesignScript.Geometry. in front of it:
    point-autocomplete2

Expected behaviour: Autocomplete for Point (and not Autodesk.DesignScript.Geometry.Point) should be available right away without having to jump through these hoops.

@andydandy74 andydandy74 changed the title Rhynamo install changes autocomplete options in Dynamo Namespace Collisions with 3rd Party Packages (e.g. Rhynamo) Mar 29, 2015
@aparajit-pratap
Copy link
Contributor

@andydandy74 thanks for pointing out this discrepancy. The first auto-complete list where all the conflicting namespaces are displayed on typing Point is expected. The system cannot automatically differentiate between classes with name Point that occur in different libraries (in this case Dynamo itself and Rhino both have classes Point). Autocomplete therefore has to display the fully qualified names to the user so that the user can explicitly specify which of these Point classes he intends to use when he simply says Point.

Secondly if autocomplete displays multiple options for Point, it means that the code block cannot resolve just Point as a class name. It needs to know additionally which namespace it comes from - Autodesk or Rhino. Therefore, I agree with you that the fact that Point.ByCoordinates(0,0,0); executed successfully, is actually confusing because in reality it should have failed. There is a discrepancy in the engine core, which is being tracked as a bug here and will be resolved soon: http://adsk-oss.myjetbrains.com/youtrack/issue/MAGN-6838

@aparajit-pratap
Copy link
Contributor

Also conflicting namespaces are now shortened into shortest names that can be uniquely resolved. For example, Autodesk.DesignScript.Geometry.Point and Rhino.Geometry.Point will now be shortened and displayed as Autodesk.Point and Rhino.Point resp. You may see this in the latest daily build: #4060

@andydandy74
Copy link
Contributor Author

From a user perspective (or you might as well call it my personal bias) I would say that it would make a lot of sense to find a solution where Dynamo's own library always takes precedence. Imagine what would happen if somebody made a package for, lets say geometry conversion to another software / file format that had a lot of commands with names of generic geometric entities like Point, Vector, Curve, Surface etc. In that case I as a user would have to choose between:
a) Explicitly calling the Dynamo methods each time I use a code block,
b) not using code blocks any more for operations like that because they've become a nuisance or
c) uninstalling the 3rd party package.
I know that I, for one, would not pick option a).
@aparajit-pratap - If I understood you correctly, you're saying that after the resolution of the "discrepancy" in the engine, a statement like Point.ByCoordinates(0,0,0) would not execute any longer in the scenario I described in my previous post. Please consider the following scenario and its ramifications:
Person A uses code blocks in custom nodes without explicitly calling Autodesk.DesignScript.Geometry.Point (because he has no "interfering" 3rd party package installed). He then publishes it to package X. Person B installs package X and also another package like Rhynamo that adds a second Point class. This would effectively render the affected nodes in package X useless.

@aparajit-pratap
Copy link
Contributor

Yes that's right, resolution of the engine issue will no longer execute the statement Point.ByCoordinates(); in the above scenario. You will have to specify Autodesk.Point.ByCoordinates(); or the fully qualified Autodesk.DesignScript.Geometry.Point.ByCoordinates(); for it to work. We did consider giving a preference to all Dynamo native classes without having to specify namespaces for them however decided not to take that route for various reasons.
Even in the case where you have class conflicts and have to use native Dynamo classes, I don't see why you would need to "explicitly" call them as autcomplete will still display them, but only with a prefix of Autodesk. which you will have to pick.
As for your concern about the following scenario:

"Person A uses code blocks in custom nodes without explicitly calling Autodesk.DesignScript.Geometry.Point (because he has no "interfering" 3rd party package installed). He then publishes it to package X. Person B installs package X and also another package like Rhynamo that adds a second Point class. This would effectively render the affected nodes in package X useless."

... this issue has already been taken care of: #3255

You can try it out just as you said:
Open a clean Dynamo version without any 3rd party packages /libraries installed. You should be able to use Point as is in a code block node. Save the file (DYN/DYF) with the CBN using Point. Import libraries that introduce conflicts with Point such as Rhynamo. Open the file. Your code block using Point should continue to work as before as Point has been internally mapped to Autodesk.DesignScript.Geometry.Point. Now if you type Point in a new code block you will simply see Point and Rhino.Point in autocomplete as the system now knows how to resolve Point and needs just one additional qualifier to distinguish it from Rhino.Point.

@andydandy74
Copy link
Contributor Author

@aparajit-pratap - thanks for clearing that up. One less thing to worry about. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants