Resurrection manager for dependency properties and metadata

[aienabled]

Hello!

Here is the promised (https://www.noesisengine.com/bugs/view.php?id=1897) resurrection manager for dependency properties and metadata (workaround for .NET 5 as static constructors cannot be called more than once there). So far it works great for the past 2.5 weeks—thousands of players, no issue reports. Consider refactoring/renaming as you wish. Also, I was not paying max attention to keep the old C# syntax—you may need to edit it a bit to make it work under older C# version.

Regards!

[s-fernandez-v]

Thanks a lot for this, I'll review it and try to incorporate to our code as soon as possible.

How is Noesis.ExternalTypeHelper.CheckTypeCanContainDependencyProperties() defined? it seems to be missing in the pull request.

[aienabled]

@s-fernandez-v, sorry, I forgot to remove this line (there was another place where it was located). It's just a class storing a static function reference:

namespace Noesis
{
    using System;

    public static class ExternalTypeHelper
    {
        public static CheckDelegate CheckTypeCanContainDependencyProperties { get; private set; }

        public static void Setup(CheckDelegate func)
        {
            CheckTypeCanContainDependencyProperties = func;
        }

        public delegate bool CheckDelegate(Type type);
    }
}

The idea is that I don't want NoesisGUI to even process certain outer C# types as they're not relevant to the UI (e.g. engine parts, such as physics engine). It was essential before when there was a risk of NoesisGUI calling C# static constructors more than once (ticket 1840).

[s-fernandez-v]

Reviewing the code in DependencyPropertyResurrectionManager.TryResurrect() I see a possible bug because that function never returns true, so it will end creating a new DependencyProperty instance always. Is that intentional?

[aienabled]

@s-fernandez-v, you're right! There should be a line: return true; right after this line in that method: dependencyProperty.ReplacePointer(cPtr);

I'm unable to test it right now, unfortunately (away until the next Monday). But you need to test it thoroughly anyway.

Regards!

[aienabled]

Hi Sergio! @s-fernandez-v How is the progress with this task? Have you found any issues with my approach?

[s-fernandez-v]

Hi, I have integrated your changes and refactored everything a bit. I tested it in Unity and is working fine, but that only covers the part where DP are initially registered. Haven't tested the DP resurrection yet.

I'll attach here my changes in case you can help me verify they are working fine in a big project like yours.

DependencyPropertyResurrection.zip

[aienabled]

Hi Sergio @s-fernandez-v, sorry for the delay. I have finally checked everything and it seems to work well The only issue I've found is this if condition in DependencyPropertyExtend.cs:

DependencyProperty dependencyProperty = existingProperty;
if (dependencyProperty != null)

It's never satisfied because you have an override for == and != operator—that checks whether the pointer is zero. It is certainly zero after NoesisGUI releases all the dependency properties during reloading!

With this code it works as intended:

DependencyProperty dependencyProperty = existingProperty;
if (!ReferenceEquals(existingProperty, null))

I will keep using this version to see if there are any other issues.

Regards!

[s-fernandez-v]

You are right, the comparison should be done with ReferenceEquals, thanks a lot for the feedback.

[aienabled]

I'm glad to help!

1. This line in DependencyPropertyRegistry.cs is not used:

   private static HashSet<Type> _constructedTypes = new HashSet<Type>();

2. The Register method there simply returns if the property is already registered. I find this odd. It should never happen under normal conditions so maybe it's a good place to throw an exception and prevent some hard-to-debug issues appearing in the future.

3. I'm also not sure about the metadata Override method there—it doesn't perform any checks whether the override for the dependency property is already made (unlike the Register method) so it accumulates all the overrides. If it's essential to keep all of them (and their order), then it's probably fine. Otherwise, it may be better to just keep only the latest override for the particular property. (I guess earlier overrides have no effect but as I'm never overriding metadata more than once so I really have no idea!)

Regards!

[s-fernandez-v]

1. You are right, the _constructedTypes member is unnecessary, I removed it from our code.

2/3. In fact it is not necessary to check here about duplicate registrations/overrides because that is already done in the native code, so I removed the else part from the Register function.

I created a new branch to work on these changes until we integrate them into the master branch: https://github.com/Noesis/Managed/tree/dp_resurrection

Thanks again for your help.

[aienabled]

The new branch seems to work fine so far! Looking forward for the inclusion of these changes into the master branch. Regards!

[s-fernandez-v]

Thanks for the update, we will include them in the next release.

[s-fernandez-v]

Already included in NoesisGUI 3.0.12