Adjust reflection API

Question

Adjust reflection API

Pathoschild opened this issue 7 years ago · 4 comments

Pathoschild commented 7 years ago

The reflection API was originally designed to access private fields in the game code. Since then we've made two significant changes:

We added GetPrivateProperty to support changes in SDV 1.2, but GetPrivateValue only works for fields.
All methods now handle public code too to support mod integrations, even though they're called GetPrivate*.

Redesign the reflection API to account for these changes.

Pathoschild · Answer 1 · 2017-12-09T17:26:52.000Z

Proposal A

The API below has two changes:

Removed all mention of 'private' and renamed the return types accordingly.
Removed GetPrivateValue — it'd be inefficient to try wrapping both fields and properties, when it's easy enough to just call GetField(...).GetValue().

/// <summary>Provides an API for accessing inaccessible code.</summary>
public interface IReflectionHelper : IModLinked
{
    /*********
    ** Public methods
    *********/
    /// <summary>Get an instance field.</summary>
    /// <typeparam name="TValue">The field type.</typeparam>
    /// <param name="obj">The object which has the field.</param>
    /// <param name="name">The field name.</param>
    /// <param name="required">Whether to throw an exception if the field is not found.</param>
    IReflectedField<TValue> GetField<TValue>(object obj, string name, bool required = true);

    /// <summary>Get a static field.</summary>
    /// <typeparam name="TValue">The field type.</typeparam>
    /// <param name="type">The type which has the field.</param>
    /// <param name="name">The field name.</param>
    /// <param name="required">Whether to throw an exception if the field is not found.</param>
    IReflectedField<TValue> GetField<TValue>(Type type, string name, bool required = true);

    /// <summary>Get an instance property.</summary>
    /// <typeparam name="TValue">The property type.</typeparam>
    /// <param name="obj">The object which has the property.</param>
    /// <param name="name">The property name.</param>
    /// <param name="required">Whether to throw an exception if the property is not found.</param>
    IReflectedProperty<TValue> GetProperty<TValue>(object obj, string name, bool required = true);

    /// <summary>Get a static property.</summary>
    /// <typeparam name="TValue">The property type.</typeparam>
    /// <param name="type">The type which has the property.</param>
    /// <param name="name">The property name.</param>
    /// <param name="required">Whether to throw an exception if the property is not found.</param>
    IReflectedProperty<TValue> GetProperty<TValue>(Type type, string name, bool required = true);

    /// <summary>Get an instance method.</summary>
    /// <param name="obj">The object which has the method.</param>
    /// <param name="name">The field name.</param>
    /// <param name="required">Whether to throw an exception if the field is not found.</param>
    IReflectedMethod GetMethod(object obj, string name, bool required = true);

    /// <summary>Get a static method.</summary>
    /// <param name="type">The type which has the method.</param>
    /// <param name="name">The field name.</param>
    /// <param name="required">Whether to throw an exception if the field is not found.</param>
    IReflectedMethod GetMethod(Type type, string name, bool required = true);
}

Entoarox · Answer 2 · 2017-12-09T19:03:51.000Z

In order to make method calls reliably efficient like was done for properties, the signature of the method needs to be known during construction, if this is a wanted feature then mechanics need to be provided so that this signature can be defined by the user.

Pathoschild · Answer 3 · 2017-12-10T23:05:58.000Z

Done in develop for the upcoming SMAPI 2.3.

Pathoschild · Answer 4 · 2017-12-09T21:08:38.000Z

Thanks for the suggestion! Caching methods into delegates would be nice, but I don't think it's worth the tradeoffs here.

First, the early signature would make the API less intuitive:

// current usage
int x = helper.Reflection.GetMethod(obj, "DoThing").Invoke<int>(argA, argB);

// usage with early signature
int x = helper.Reflection.GetMethod<int>(obj, "DoThing", new Type[] { typeof(int), typeof(string) }).Invoke(argA, argB);

It would also require four top-level methods instead of two (instance vs static + return value vs none), and two separate types (IReflectedMethod<T> for methods with a return value and IReflectedMethod for those without):

/// <summary>Get an instance method.</summary>
/// <typeparam name="TValue">The return type.</typeparam>
/// <param name="obj">The object which has the method.</param>
/// <param name="name">The field name.</param>
/// <param name="argumentTypes">The argument types the method accepts.</param>
/// <param name="required">Whether to throw an exception if the field is not found.</param>
IReflectedMethod<TValue> GetMethod<TValue>(object obj, string name, Type[] argumentTypes, bool required = true);

/// <summary>Get an instance method.</summary>
/// <param name="obj">The object which has the method.</param>
/// <param name="name">The field name.</param>
/// <param name="argumentTypes">The argument types the method accepts.</param>
/// <param name="required">Whether to throw an exception if the field is not found.</param>
IReflectedMethod GetMethod(object obj, string name, Type[] argumentTypes, bool required = true);

/// <summary>Get a static method.</summary>
/// <typeparam name="TValue">The return type.</typeparam>
/// <param name="type">The type which has the method.</param>
/// <param name="name">The field name.</param>
/// <param name="argumentTypes">The argument types the method accepts.</param>
/// <param name="required">Whether to throw an exception if the field is not found.</param>
IReflectedMethod<TValue> GetMethod(Type type, string name, Type[] argumentTypes, bool required = true);

/// <summary>Get a static method.</summary>
/// <param name="type">The type which has the method.</param>
/// <param name="name">The field name.</param>
/// <param name="argumentTypes">The argument types the method accepts.</param>
/// <param name="required">Whether to throw an exception if the field is not found.</param>
IReflectedMethod GetMethod(Type type, string name, Type[] argumentTypes, bool required = true);

And the performance boost wouldn't be very significant. The difference between reflection and delegates mainly appears on the order of 10⁶ invocations per second (and even then the difference is measured in milliseconds); SMAPI mods are closer to 10² invocations per second, with a cache on the initial reflection already in place.

The reflection API is meant to balance performance with ease of use for most mods. If a mod needs to perform extraordinarily intensive reflection, it should probably use C# reflection directly instead.

Share to

Proposal A