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

System Icomparer shows up in the library #8790

Closed
ksobon opened this issue Apr 25, 2018 · 14 comments
Closed

System Icomparer shows up in the library #8790

ksobon opened this issue Apr 25, 2018 · 14 comments
Labels
2.x Issues related to 2.x versions of Dynamo. engine Issues related to the engine driving Dynamo. tracked

Comments

@ksobon
Copy link

ksobon commented Apr 25, 2018

I am not sure how this one found its way into the library:

image

@mjkkirschner
Copy link
Member

  • do you embed interop types into your dlls?
  • do you reference this type at all?
  • do you have any methods that accept or return an IComparer?

@ksobon
Copy link
Author

ksobon commented Apr 25, 2018

Well yes, I do have a custom implementation of the Comparer class that looks like this:

        /// <summary>
        /// Comparer class that reverses the order of sorting.
        /// </summary>
        [IsVisibleInDynamoLibrary(false)]
        public class ReverseComparer : IComparer
        {
            /// <summary>
            /// 
            /// </summary>
            /// <param name="x"></param>
            /// <param name="y"></param>
            /// <returns></returns>
            public int Compare(object x, object y)
            {
                return new CaseInsensitiveComparer().Compare(y, x);
            }
        }

Again, it looks like the attribute [IsVisibleInDynamoLibrary(false)] is not working. This time it's a class not an enum.

@mjkkirschner
Copy link
Member

@ksobon thanks
@Racel this looks like a pretty bad regression if true...

@mjkkirschner
Copy link
Member

hmm @ksobon well, hmm, thats a little different - I totally get the behavior is changed...
But - your derived class is actually still hidden right? - but now the base class is showing ...

@ksobon
Copy link
Author

ksobon commented Apr 25, 2018

Yes, that's correct. Perhaps the attribute was never meant to hide the base class?

@mjkkirschner
Copy link
Member

rather, I think it was meant to hide the base class, but the regression might be very specific (why we missed it)

can you share your in development package.json file here and an image of your bin folder?

@mjkkirschner
Copy link
Member

mjkkirschner commented Apr 25, 2018

another thought - are you distributing dynamo core or dynamoServices with your packages? That will also cause this behavior. (if it's a different version of dynamo's assemblies) like you if distribute 1.3 core and run it on 2.0. (or building on the old dlls)

@ksobon
Copy link
Author

ksobon commented Apr 25, 2018

So I was able to get it to hide itself when I changed the class to internal - obviously. I did try the [SupressImportIntoVM] attribute and that didn't work. I also tried putting [IsVisibleInDynamoLibrary(false)] attribute on both the class and the method inside of it since that was public as well, but that didn't work either.

Again, luckily I could actually make this one internal, so problem solved but it's something worth checking out.

/// <summary>
    /// Comparer class that reverses the order of sorting.
    /// </summary>
    internal class ReverseComparer : IComparer
    {
        /// <summary>
        /// 
        /// </summary>
        /// <param name="x"></param>
        /// <param name="y"></param>
        /// <returns></returns>
        public int Compare(object x, object y)
        {
            return new CaseInsensitiveComparer().Compare(y, x);
        }
    }

image

I don't distribute any of the mentioned libraries. I do reference them in for some of the UI work.

image

pkg.zip

@mjkkirschner
Copy link
Member

mjkkirschner commented Apr 25, 2018

@ksobon can you verify that your package (that you are testing) is built against the dynamo 2.x nugets and not the 1.3 ones?

@ksobon
Copy link
Author

ksobon commented Apr 25, 2018

Yup.

image

@mjkkirschner
Copy link
Member

tracked with:
https://jira.autodesk.com/browse/QNTM-4094

@aparajit-pratap
Copy link
Contributor

@mjkkirschner I think it would be worth investigating the IsVisibleInDynamoLibrary and SuppressImport attributes to see if they're not broken. I think I saw a similar issue with these recently.

@johnpierson johnpierson added 2.x Issues related to 2.x versions of Dynamo. and removed 2.1 Release labels Oct 3, 2018
@johnpierson johnpierson added engine Issues related to the engine driving Dynamo. tracked and removed Library labels Nov 7, 2018
@johnpierson
Copy link
Member

Tracked internally as QNTM-4094

@aparajit-pratap
Copy link
Contributor

This is fixed in #9242. Closing. Let us know if you still see issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x Issues related to 2.x versions of Dynamo. engine Issues related to the engine driving Dynamo. tracked
Projects
None yet
Development

No branches or pull requests

4 participants