Introduce an EditorTypeTag Property for Choosing More Specialized PropertyEditors

[ilexp]

Summary

Some properties might be better handled by more specialized editors than their type suggests - like a float values that expresses an angle that should be handled by a specialized angle editor. There should be a way for core plugins to express custom specialization on a per-property basis for editor plugins to evaluate.

Analysis

• The direction of information flow should be from core to editor.
  • A core plugin should not have any knowledge about available editor types.
  • Instead, an editor plugin may evaluate information from the core plugin to act accordingly.
• An EditorTypeTag attribute could specify a string tag for a property or field.
• The editor selection algorithm could then evaluate the members EditorTypeTag attributes in some way to make a choice. Maybe by adding a priority boost for editors that can handle a type, but also match one of its tags?
• This should also be exposed through PropertyEditorAssignment attributes in some way to allow editor plugins to easily specify a property editors specialization preference.
• A common case will be to define a property editor for a type that should never be used, except if it matches one of the tags.

[Barsonax]

Is it ok if I look into this one?

[ilexp]

Yep, but be aware that this issue still has a lot of design work left to do. Not expecting an implementation right now, but a design draft of how exactly to implement this, including potentially open questions / issues and approaches how to address them.

[Barsonax]

So for the editor choosing algorithm: -Filter out all editors that do not support the type. -Score the remaining editors based upon how many tags of the property match with the editor tags amd vice versa. The score value will be a float between 0 and 1. 0 is when nothing matches and 1 is when all tags match. -Pick the first editor of the remaining editors.

So a awesomepropertyeditor has 2 tags: awesomeA and awesomeB. A property has 3 tags: awesomeA, awesomeB and awesomeC. The score would be 0.666 in this case as only 2 of the 3 tags match. This way editors with alot of tags wont match on everything as each match has less weight than an editor with just 1 tag.

You probably already saw the problem here when you have multiple editors with the same score. In this case more tags would have to be used so the editor has more information. Maybe a log message to make this clearer to the user could be used?

The next step for me is to understand how it currently works before can say how this could be implemented.

[ilexp]

Doesn't sound too bad so far. The way it currently works is that we have a Duality-specific property editor provider that gets the chance to select a specialized editor before the default behavior kicks in and uses a primitive or default editor from AdamsLair.WinForms. To do that, is gathers all the defined property editors across all editor plugins and uses each editors PropertyEditorAssignment attribute in order to determine a priority score, which is passed to the grid so it can decide what to do.

To make this work with type tags, we'd probably need to start in the WinForms helper lib to extend the property grid logic itself, since editor selection currently works solely based on type and context, but not specific properties. The easiest approach would probably be to extend the existing context object to include an optional (can be null) set of editor type tags as strings, which could then be used as part of the selection process, which would mean that the CreateEditor grid method would need to be extended as well, probably with one overload providing the tags directly and one with a MemberInfo where the tags are automatically retrieved via reflection.

The above would mean that the type tag attribute would be part of AdamsLair.WinForms, which is nonviable because that would mean that core plugins would need a reference to that in order to provide type tags for properties. So we'd need to implement this in a way that AdamsLair.WinForms only knows the concept of an optional tag list in an editor provider context, but has no knowledge about how they are retrieved, even though it needs to trigger their retrieval itself in fallback editors such as MemberwisePropertyEditor. I suspect the solution will be something like introducing a new virtual method to the property grid, which is then implemented in the DualityPropertyGrid where the actual attribute retrieval is done. Maybe there are better alternatives, but nothing specific on the top of my head right now.

After step one of making sure the tag information is available, we could then find an implementation for your current outline where we could extend the existing PropertyEditorAssignment attribute with an optional tag list and adjust score calculation to include potential matches with a score bonus.

To do that, however, the base selection logic would need to be updated, as it currently always prefers builtin primitive types over custom provider results.

So that's kind of a rabbit hole on closer inspection.

[Barsonax]

Brainstorming mode: So the problem is that there is a conceptual difference between how built in property editors and plugin property editors are handled. Wouldn't it be possible to threat them all the same? Its seems that currently the AdamsLair.WinForms has too much responsibility. It shouldn't be doing any selection logic or provide property editors that duality would directly use.

Basically what iam saying that to use a property editor from AdamsLair.WinForms it has to be wrapped in a class that is duality aware (and thus can also have all kind of duality attributes). On top of that Duality itself will implement the selection logic.

Again this is just an idea and I have no idea if this is possible or easy to implement.

[ilexp]

Its seems that currently the AdamsLair.WinForms has too much responsibility. It shouldn't be doing any selection logic or provide property editors that duality would directly use.

Since AdamsLair.WinForms defines the PropertyGrid in the first place, and it is expected for that PropertyGrid to be functional on its own, it actually needs to do editor selection, and it also needs to define some builtin editors for the basics, and use them.

Duality does define additional logic, which isn't needed in the context of the original property grid, but still has to be integrated. So the currently used approach is to make the basic PropertyGrid extensible in all the important spots, so Duality can use that extensibility to provide its own logic. Not that far off from what you're saying I think.

Wouldn't it be possible to threat them all the same?

Yes! They really should be, in fact. The reason they're not is a design flaw in the current implementation where the builtin ones are treated separately from any custom defined ones. Should be possible to fix in a non-breaking way.

[Barsonax]

So globally the solution will look like this:

1. All property editors in duality would have to inherit from DualityPropertyEditor:

   public abstract class DualityPropertyEditor : PropertyEditor
   {
   //Any needed code here
   }

2. Implement all needed property editors. Add any needed attributes as well here.

3. Make CreateEditor in the PropertyGrid virtual.

4. Update AdamsLair.WinForms in duality

5. Override CreateEditor in the DualitorPropertyGrid. Implement the needed scoring logic that determines the propertyeditor here.

6. Remove the now unused DualityPropertyEditorProvider

Step 2 could involve quite a lot of 'wrapper classes' so not sure if I really like that. Since you have to add attributes to them anyway I don't think you can really prevent this. In order to threat them all equally they have to be declared equally as well.

[ilexp]

Hmm, I think we're not yet on the same page here.

Points 1 and 2 are not necessary and don't provide an advantage in this use case as far as I can see. Point 5 and 6 would be one way to address the problem, but require more API and design changes than just changing the base editor provider implementation in AdamsLair.WinForms and updating the existing DualityPropertyEditorProvider.

We don't really need to make a few big or structural changes, but rather a large number of small changes, as outlined in my previous comment. But yeah, this is kind of a rabbit hole to go down into, and my description alone probably wasn't enough for the full picture - especially since AdamsLair.WinForms is not the cleanest of codebases, and the PropertyGrid itself not the most well-designed either. Might be more efficient if I'll look into this myself when I get around to it, unless you really want to do the deep-dive here.

[Barsonax]

But how will you add the needed attributes to the property editors in AdamsLair.WinForms if the attribute is defined in Duality? If you define the attribute in AdamsLair.WinForms then plugins would have to reference it which you don't want either. That's why I wanted to bring the definition of the duality property editors to duality itself by wrapping the ones in AdamsLair.WinForms so you can add the attributes without having to reference AdamsLair.WinForms from a plugin.

[ilexp]

If you define the attribute in AdamsLair.WinForms then plugins would have to reference it which you don't want either.

Yep, that's a big problem, but it's not an entirely new problem. It was the same when first introducing the EditorHint family of attributes for adjusting how individual editors should behave.

But how will you add the needed attributes to the property editors in AdamsLair.WinForms if the attribute is defined in Duality?

Same as with the other Duality-specific attributes: In the DualityPropertyGrid class, using API that is provided by the PropertyGrid base implementation. If I recall correctly, this was the original reason for introducing the ConfigureEditor API, which is now used to apply Duality-specific hints to editors that are otherwise created and maintained by AdamsLair.WinForms.

For the tags feature, there will be the need to extend the base class again as outlined before, but that should be it.

[Barsonax]

Hmm seems its getting a bit complex because of the design flaws. The code is in need of a refactoring but I guess there are other constraints as well. Since you wrote it/know the big picture it might be better you do this I guess.