v2 Pos classes need to support interrogation for TGD

[tznind]

Executive Summary
It is no longer possible to know important things about a given Pos because of the use of primary constructors, whose parameters are innaccessible through reflection.

Detail
Consider Pos.PosView (aka relative to)

V1
In v1 fields were side and View

• These were accessed via reflection in TGD

For example

    public static bool IsCombine( [NotNullWhen( true )] Pos? p )
    {
        if ( p == null )
        {
            return false;
        }

        return p.GetType( ).Name == "PosCombine";
    }

Also

  var fTarget = posView.GetType().GetField("Target") ?? throw new Exception("PosView was missing expected field 'Target'");
  View view = (View?)fTarget.GetValue(posView) ?? throw new Exception("PosView had a null 'Target' view");

V2
In v2 internals are now visible to TGD (yay!) so can do like

if(p is Pos.PosCombine pc)
{
...
}

However there are some changes to use primary constructors e.g.

internal class PosView (View view, Side side) : Pos

v2 code for PosView

Since Side is no longer a field, we cannot read it in TGD. If in the debugger we examine the source for the object we get that it has this secret hidden field thats super sketchy.

        [CompilerGenerated]
        private Side _003Cside_003EP;

        public readonly View Target;

        public PosView(View view, Side side)
        {
            _003Cside_003EP = side;
            Target = view;
            base._002Ector();
        }

Fix
Fix will be to expose as public readonly the constructor properties e.g. like Side

This will enable the TGD to read them and support its UIs for designing/editting the position stuff is at.

    internal class PosView (View view, Side side) : Pos
    {
         public readonly View Target = view;
+        public Side TargetSide => side;

[tznind]

Given that these classes are internal, is there any issue with making all the private fields public?

Also for those that are currently private e.g. _factor for PosFactor?

If public is acceptable then what is the best way to do that

• public readonly field (Note that doing this for side somehow causes CS9124 )
• public property (e.g. as above)
• other?

If they are to stay private then I can still proceede with the existing reflection but will need private fields for the Pos/Dim that have primary constructors parameters)

[dodexahedron]

Something you may want to check out that avoids manual reflection and is super speedy for member access:

UnsafeAccessorAttribute

That thing can basically be thought of as a reverse InternalsVisibleTo sort of thing that also works for private, and is declared in the consumer's code (InternalsVisibleFrom? 😅).

If we had some interfaces for things, the combination of that attribute and interfaces could make your life soooo much easier.

As for addressing the problem, though:

Why is direct access to the fields necessary? Wouldn't you want to always be going through the property accessors, so that you're not coupled to implementation details?

[tznind]

Why is direct access to the fields necessary?

In editor you add a view and set it X pos relative to another You save and get .Designer.cs file You close and reopen Editor compiles .Designer.cs into in memory assembly and stamps out an instance of the view (to the main editing area). You right click view and edit the X field Designer looks at value of X on the instance of the view and shows that in PosEditor (i.e. tell user the current value) Hence for any Pos the designer needs to be able to work out what kind it is and what its settings are.

…

On Tue, 14 May 2024, 21:30 dodexahedron, ***@***.***> wrote: Something you may want to check out that avoids manual reflection and is super speedy for member access: UnsafeAccessorAttribute <https://learn.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.unsafeaccessorattribute?view=net-8.0> As for addressing the problem, though: Why is direct access to the fields necessary? — Reply to this email directly, view it on GitHub <#3482 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHO3C5HFPWQ6GVGI25GV2M3ZCJX5LAVCNFSM6AAAAABHWYBQRKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJRGA4DONRYGY> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[dodexahedron]

Gotcha.

I happened to update my previous reply like... Seconds after you replied just now, with a little bit more info, btw.

I think that attribute and TG getting interfaces that it should have anyway is probably the best solution overall, for TG and for TGD or anyone else wanting to get fancy with usage beyond the API itself. :)

[tznind]

Ah I see now! The problem is that there are no accessors for PosView Side or the percent one.

…

On Tue, 14 May 2024, 22:32 dodexahedron, ***@***.***> wrote: Gotcha. I happened to update my previous reply like... Seconds after you replied just now, with a little bit more info, btw. — Reply to this email directly, view it on GitHub <#3482 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHO3C5DRUI4CH4B7DIP3G4LZCJ7IDAVCNFSM6AAAAABHWYBQRKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJRGE3TKMRVGU> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[dodexahedron]

And as for the basic issue of how to figure it out...

The nested types are unfortunate.

Really should be an inheritance hierarchy.

There's no benefit at all to the nested types and any perceived benefit is a smell/kludge with far more idiomatic C# solutions available.

In fact, I think that was one of the first things I ever griped about when I initially started helping out with unit tests for TGD, before shifting focus to helping out over here with V2 first. 🤔

[tig]

The right thing to do here is consider Designer as a perfectly valid app and expose things it needs via a suitably designed public API.

I'm knee deep in DimPos.cs right now and have been fighting with myself over this very topic for different reasons.

• Dim.Auto (DimAutoStyle.Text) confuses TextFormatter when Width is absoulte and Height is auto #3469

I wasn't going to tackle solving this before I saw this Issue. Now I am. Stay tuned.

[tig]

This is the PR that will address this:

• Fixes #3469, #3473, #3482 - Dim.Auto fixes and Pos/Dim refactor to support TGD #3480

[dodexahedron]

As far as TGD goes, providing proper interfaces would allow @tznind to access a lot more than he currently can, without reflection or [UnsafeAccessor], which would be even better for him and for consumers of TG, TGD, and TG apps designed in TGD.

[dodexahedron]

Shifting gears to a completely different question for you @tznind:

Are you watching the stuff in .net 9.0 around dynamic persistent assembly creation? 😱 That stuff looks awesome.

[dodexahedron]

BAH.

Ok ignore literally all of that, because I was totally mis-reading the diffs.

My bad.

I'm pulling it down to look at it in-situ, now, instead of trying to make sense of the diffs.

If you totally already took care of that in this PR you are my hero. ❤️

[dodexahedron]

Well.. Except for the dynamic persisted assemblies, for you, @tznind

That looks like a perfect thing for you, down the road.

[dodexahedron]

It's like Christmas in May!

I less than three you, @tig , for biting the bullet and doing this. :)

It's a big and difficult to review PR, so I'm going to compare locally in better tools than the github web UI once you're finished with it.

I do see some things we can and maybe should do, but they shouldn't be part of this one. It should probably be as isolated to the refactor of the type hierarchy as reasonably possible.

I'mma just delete the earlier comments rather than hiding them, since they were totally irrelevant and stoopid.

[dodexahedron]

But, related to one bit I mentioned: We should definitely look at removing any InternalsVisibleTo dependencies between TG and TGD that remain, as well, in support of the goal.