Harmony AccessTools.MakeDeepCopy fails when target contains a constant or static readonly field

[xADDBx]

Describe the bug
When using AccessTools.MakeDeepCopy on a class which contains a constant field, Harmony will cause a System.FieldAccessException: Cannot set a constant field.

To Reproduce

1. Create an executable program
2. Create another class with a constant field in it
3. In the Main method, create an instance of the class and call MakeDeepCopy on it

System.FieldAccessException: Cannot set a constant field.
   at System.Reflection.MdFieldInfo.SetValue(Object obj, Object value, BindingFlags invokeAttr, Binder binder, CultureInfo culture)
   at HarmonyLib.Traverse.SetValue(Object value)
   at HarmonyLib.AccessTools.<>c__DisplayClass102_0.<MakeDeepCopy>b__0(String name, Traverse src, Traverse dst)
   at HarmonyLib.Traverse.<>c__DisplayClass39_0.<IterateFields>b__0(String f)
   at System.Collections.Generic.List`1.ForEach(Action`1 action)
   at HarmonyLib.Traverse.IterateFields(Object source, Object target, Action`3 action)
   at HarmonyLib.AccessTools.MakeDeepCopy(Object source, Type resultType, Func`4 processor, String pathRoot)
   at HarmonyLib.AccessTools.MakeDeepCopy[T](Object source)
   at Harmonyissue.Program.Main(String[] args) in D:\Programming\Projects\Modding\HarmonyStuff\Harmonyissue\Harmonyissue\Program.cs:line 16

Expected behavior
Const fields should be ignored when creating a DeepCopy

Screenshots / Code

Runtime environment (please complete the following information):

• OS: Windows 10, 64bit
• .NET Version: I've managed to reproduce this error in .NET Framework 4.8.1 (Unity and non-Unity runtime), .NET 6.0 and .NET 8.0. I assume it's an issue on all versions
• Harmony Version: Tested on Harmony 2.3.3 and Harmony 2.3.1.1.
• Runtime: I've reproduced it in both Pathfinder: Wrath of the Righteous and Warhammer 40,000: Rogue Trader additionally I can reproduce this in a simple console application

[xADDBx]

I just tried testing the different possible combinations of IsLiteral, IsInitOnly and IsStatic. At the same time I noticed that static readonly fields will also cause an exception:

FieldModifiers	IsLiteral	IsInitOnly	IsStatic	Causes Exception
	False	False	False	No
readonly	False	True	False	No
static	False	False	True	No
static readonly	False	True	True	Yes
const	True	False	True	Yes

Updated Test class:

public class Test {
    public const string MyConstField = "Don't set me";
    public string MyField = "Set me";
    public static string MyStaticField = "Set me";
    public readonly string MyReadonlyField = "Set me";
    public static readonly string MyStaticReadonlyField = "Don't set me";
}

MakeDeepCopy will throw for both MyStaticReadonlyFieldand MyConstField. Changing the Traverse.SetValue to the following seemed to work for my test case:

[HarmonyPatch]
public static class Patches {
    [HarmonyPatch(typeof(Traverse), nameof(Traverse.SetValue)), HarmonyPrefix]
    public static bool SetValue(object value, ref Traverse __result, Traverse __instance) {
        var infoField = AccessTools.Field(typeof(Traverse), "_info");
        var methodField = AccessTools.Field(typeof(Traverse), "_method");
        var rootField = AccessTools.Field(typeof(Traverse), "_root");
        var paramsField = AccessTools.Field(typeof(Traverse), "_params");
        MemberInfo _info = infoField.GetValue(__instance) as MemberInfo;
        MethodBase _method = methodField.GetValue(__instance) as MethodBase;
        object[] _params = paramsField.GetValue(__instance) as object[];
        object _root = rootField.GetValue(__instance);


        if (_info is FieldInfo fi) {
            bool isConst = fi.IsLiteral && !fi.IsInitOnly && fi.IsStatic;
            bool isStaticReadonly = !fi.IsLiteral && fi.IsInitOnly && fi.IsStatic;
            if (!isConst && !isStaticReadonly)
                ((FieldInfo)_info).SetValue(_root, value, AccessTools.all, null, CultureInfo.CurrentCulture);
        }
        if (_info is PropertyInfo)
            ((PropertyInfo)_info).SetValue(_root, value, AccessTools.all, null, _params, CultureInfo.CurrentCulture);
        if (_method is not null)
            throw new Exception($"cannot set value of method {_method.FullDescription()}");
        __result = __instance;
        return false;
    }
}

Actually, is there a reason it tries to set static Fields at all?
While it works on normal static fields, does the method really need to set static fields which should be shared across instances anyway?

In total I see three different approaches:

1. Check for static readonly or const as shown above and don't set those fields
2. Check for any static field and don't set those (depending on where Traverse.SetValue is used, this might not be a good idea)?
3. try/except System.FieldAccessException around the FieldInfo.SetValue call (this might be too broad in what it catches?)

Edit: To make sure this doesn't only apply to public string, I also tested for private int fields, which showed exactly the same behavior.

[pardeike]

I know its late but I am finally getting to this. I have a slighly different approach. I added this to Traverse:

/// <summary>Checks if the current field or property is writeable</summary>
/// <returns>true if writing is possible, false otherwise</returns>
///
public bool IsWriteable
{
	get
	{
		if (_info is FieldInfo fi)
		{
			var isConst = fi.IsLiteral && fi.IsInitOnly == false && fi.IsStatic;
			var isStaticReadonly = fi.IsLiteral == false && fi.IsInitOnly && fi.IsStatic;
			return isConst == false && isStaticReadonly == false;
		}
		if (_info is PropertyInfo pi)
			return pi.CanWrite;
		return false;
	}
}

since its of general interest and add a check in the copy loop of MakeDeepCopy.