Custom ContextMenu commands

[GeKtvi]

I'd like to propose adding support for custom context menu commands in SolidDna. I've implemented a working solution that allows users to extend the context menu with their own commands.

Implementation Overview

1. Introduced ICommandCreatable interface to handle command creation:

• Mutable before creation
• Returns immutable ICommandCreated collection for disposal management

2. Added CommandContextMenuGroup as a container for sub-items (no SolidWorks API objects created)
3. Implemented CommandContextItem as terminal menu item using ISldWorks.AddMenuPopupItem3 (chosen after evaluating alternatives)

Current Limitations

1. Disposal Issue:

ISldWorks.RemoveMenuPopupItem2 doesn't remove items (returns false) (SW2024 SP1).
Currently commented out remove code to avoid potential issues.

2. Enum Usage:

• Implementation currently uses swSelectType_e extensively
• Proposal: Introduce user-friendly SolidDna enum
• Options for the existing SelectedObject:

1. Change ObjectType property type to new enum
2. Mark ObjectType property as obsolete and add new "Type" property
3. Leave as is

Next Steps

If this implementation approach is acceptable, I can:

1. Add the proposed enum in a follow-up commit
2. Implement the chosen migration path for SelectedObject.ObjectType

Would appreciate feedback on the current implementation and which enum migration path would be preferred.

[brinkdinges]

Cool Can you add a few screenshots of these menus? I haven't created them before and solidworks never includes any images in their docs either. You are way ahead of me.

I'm all for a selectable object enum, shall we call it SelectableObjectType? Because solidworks does lots of things with selectable objects in the API and I'd like to teach others what item types are selectable and which ones are not. Let's not make any breaking changes, so let's introduce a new enum and mark methods that use the old one obsolete. Please make it a separate PR.

Selection needs more work, see #17. In my common code, I have a Select and a GetSelected class and that works well. I don't know how well that translates to DNA. I see it already has a SelectedObject class, we should probably use that in GetSelected.

[GeKtvi]

"SelectableObjectType" is a good name. But I have an idea about using a complicated type object: a combined enum value and string value, because sometimes SW wants a string implementation of type identifier.

If I understood you correctly, you want to find a way to determine which objects user can select and which not, and do it at compile time. As a first idea, I could suggest using a marker interface "ISelectableObject" in all selectable SolidDnaObject (add interface to each derived object). It is not the best practice I think, but it's just an idea.

I'll check the current implementation of SelectionManager. It uses the SolidDna approach, enumerating through items by user's selector (Action<>), and after each iteration it disposes the item. About this approach I have two questions:

1. Can we try other approaches in new selection functionality? The described approach is extremely inconvenient. We can't use LINQ with the collection (it is the most powerful feature of .NET, how can we avoid it?). Also, if user wants to use some item later, there's no way to do it except calling SW API directly.
2. What is the meaning of disposing each object by calling Marshal.ReleaseComObject? I know that in Excel API, if we don't do this, we get a zombie process in Task Manager until garbage collection. But what is the problem with SW in-process API? I've tried not disposing objects obtained directly from API and it looks good - no stuck objects in memory. It looks like the CLR properly releases RCW during garbage collection. I might have misunderstood something - it will be great if you can explain it.

About the new PR: Did I misunderstand the order of work? Should I leave this PR for some time, then create a new one, add the new enum, wait until it is merged, then return to this PR? Is that correct?

[brinkdinges]

We should indeed have access to the string solidworks expects, that also makes it easier to give the enums better names. The Description attribute works well for linking strings to enums:

I think we don't need to add ISelectable, it's more that solidworks hardly ever mentions that those object types are selectable and therefore can be retrieved a certain way. I think we're already improving that by choosing this name for the enum.

I'm also not a fan of the Action approach, it's too rigid and often gets in the way. I prefer a GetSelectedObjects method that just returns a list of objects, or make it generic for only certain types. Way simpler and still better than the array or null that solidworks returns.

Let's keep this PR going and tackle selection after that. We can release a new version after finishing selections so we don't break the new enums.

[GeKtvi]

I think I misunderstood you. You want to use reflection to get the string value? Are you sure this is a good way? I think it might cause performance issues in some scenarios. Maybe we can just use the "AsString" extension method with a switch inside? It's not a perfect solution, but it won't cause any problems.

[brinkdinges]

The DescriptionAttribute doesn't use reflection:
https://learn.microsoft.com/en-us/dotnet/api/system.componentmodel.descriptionattribute?view=netframework-4.8.1

But indeed, my method to get it from the enum does, GetCustomAttributes uses reflection.

I'll check if there is a way to avoid reflection, I may already have found one but I want to do some benchmarking. Generally, nothing we do in our code is meaningfully slow since a single rebuild can take seconds, but that doesn't mean we should become sloppy.

[GeKtvi]

Yes, that's exactly what I mean. In this solution, we have a lot of method calls to get a string, which is just a static value. For example, if a user calls the Description extension while iterating through some loop, it might take longer than a hard-coded alternative (a switch or a composite type). Yes, this is just a potential situation, but in my opinion, it's a good practice to avoid these risks.
But if you use an extension method, we can just leave it as is, and when we have performance issues, we can just add a new overload with a specific enum. So we can leave this option, but I prefer to use a more primitive implementation.

[brinkdinges]

Switch is indeed a bit faster 😅 I've also tested caching the results in a dictionary:

Thanks for the insight. I love enum attributes and I heavily use them in my products. I'll go check if I need to change that in some hotpath.

[GeKtvi]

Can you share the code? Interesting result. I expected more difference in performance
Sorry, I didn't understand right away, yes that's a big gap

[GeKtvi]

I implemented SelectionType by myself. I thought about enum implementation, but there are user custom feature types, and I decided to implement it as a union type with int, string, and custom names values.

[brinkdinges]

I implemented SelectionType by myself. I thought about enum implementation, but there are user custom feature types, and I decided to implement it as a union type with int, string, and custom names values.

It took me a while to figure out why those custom feature types are there, but I see now that those are necessary for the command manager.

I'll attempt to understand this PR later. Right now, it changes the language version, adds the iterator approach, adds selection types and renames/moves files just as side effects. I guess moving files is not a breaking change since we use a single namespace, so that's not too bad.

[GeKtvi]

Right now, it changes the language version
No, it does not. The change only affects the demo CommandItems project and was made solely to demonstrate that the solution does not introduce boilerplate.

That said, upgrading the language version and enabling nullability support is a good practice - I always do it for my add-ins (SolidWorks and other CAD systems) and have no problem with that. However, I’m unsure about the implications of applying this change to the library itself. It’s worth considering, but we should evaluate potential consequences first.