Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SymbolExtensions does not resolve overriden method #600

Closed
krafs opened this issue Mar 15, 2024 · 3 comments
Closed

SymbolExtensions does not resolve overriden method #600

krafs opened this issue Mar 15, 2024 · 3 comments
Assignees
Labels

Comments

@krafs
Copy link

krafs commented Mar 15, 2024

Describe the bug
Getting a MethodInfo for an overriden method using SymbolExtensions resolves the base class method, not the child class method.

To Reproduce

  1. Open a terminal in a new empty folder.
  2. dotnet new console --framework net8.0
  3. dotnet add package Lib.Harmony --version 2.3.1.1
  4. Create file called Program.cs and paste the code below
  5. Save the file
  6. dotnet run

Expected behavior
I expect the MethodInfo retrieved for the overriden method in Child class using SymbolExtensions to be the same MethodInfo I get when retrieving it using regular reflection.

Code

using HarmonyLib;
using System.Reflection;

MethodInfo parentHello = typeof(Parent).GetMethod("Hello");
MethodInfo childHello = typeof(Child).GetMethod("Hello");
Console.WriteLine(parentHello == childHello);
// Expected: False
// Actual: False

MethodInfo symbolParentHello = SymbolExtensions.GetMethodInfo<Parent>(x => x.Hello());
MethodInfo symbolChildHello = SymbolExtensions.GetMethodInfo<Child>(x => x.Hello());

// Check if MethodInfo for Child.Hello resolved via Reflection is same the one from SymbolExtensions
Console.WriteLine(childHello == symbolChildHello);
// Expected: True
// Actual: False

// Check if MethodInfo for Parent.Hello and Child.Hello resolved via SymbolExtensions are the same
Console.WriteLine(symbolParentHello == symbolChildHello);
// Expected: False
// Actual: True
public class Parent
{
    public virtual void Hello() { }
}

public class Child : Parent
{
    public override void Hello() { }
}

Runtime environment:

  • OS: Windows 11, 64bit
  • .NET version: .NET 8.0/.NET 472
  • Harmony version: 2.3.1.1

I couldn't find any tests on SymbolExtensions, so not sure if this is an expected limitation on the API?

@pardeike
Copy link
Owner

The reason is that reflections do not work well with inheritance. Its the same reason that does not allow you to call a base method on a child instance if you don't let the compiler do it. That includes casting, ordinary reflection and delegates. It's the reason Harmony has a reverse patch. I extended your code to demonstrate this effect:

public class Parent
{
	public virtual void Hello() { Console.WriteLine("PARENT"); }
}

public class Child : Parent
{
	public override void Hello() { Console.WriteLine("CHILD"); }
}

parentHello.Invoke(new Parent(), null);
parentHello.Invoke(new Child(), null); // should log PARENT but logsCHILD
childHello.Invoke(new Child(), null);

SymbolExtension uses reflection and thus cannot choose which level of overwrite it should get.

@krafs
Copy link
Author

krafs commented Mar 21, 2024

Maybe I've misunderstood the use of SymbolExtensions then. Is SymbolExtensions not for getting a MethodInfo on a given type? It doesn't need to determine the level of the overwrite to return, I am telling it to return the one from Child.

This gets me what I expect.
MethodInfo hello1 = typeof(Child).GetMethod("Hello");

And this call has all the same amount of information, so I feel like it should be able to give the same result, no?
MethodInfo hello2 = SymbolExtensions.GetMethodInfo<Child>(x => x.Hello());

Or is this not the purpose of SymbolExtensions? It's not an alternative to Reflection for getting a MethodInfo?

@pardeike
Copy link
Owner

The purpose is the same. The technical limitations are not the same. SymbolExtensions basically wrap a super simple concept and there is no way to improve that: < https://github.com/pardeike/Harmony/blob/669729f618e90ecf7fe943f7ed70f8150216dc5a/Harmony/Tools/SymbolExtensions.cs#L47>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants