LlamaLad7 / MixinExtras

Companion library to SpongePowered Mixin with many custom injectors for a more expressive experience.
MIT License
309 stars 17 forks source link

[Suggestion] Adding parameters #37

Closed Papierkorb2292 closed 1 year ago

Papierkorb2292 commented 1 year ago

Multiple times now, I wanted to add a new parameter to a method or also pass it to a new object (which can be done by adding a setter method, but being able to add parameters to a constructor would be nicer and also allow usage of that parameter in other functionality of the constructor). Currently, the only other way to achieve this is a ThreadLocal and the target methods needs to be invoked in a try...finally statement.

I am not sure how well this can be implemented within the Mixin system, since it requires changing other methods, but it might be similar to @WrapOperation. I came up with the following syntax, where new parameters are passed as @Share variables:

class MyClass {
    public void method(boolean a) {
        <Some code>
    }
}

@Mixin(MyClass.class)
abstract class MyClassMixin {

    @AddParameters(
        target="method(Z)V
    )
    public abstract void methodWithInt(boolean a, @Share("myInt") int b) //By using int here, the user doesn't need to create a new LocalIntRef object themself

    @Inject(
        target="method(Z)V",
        at=<Somewhere>
    )
    void useNewMethodParameter(boolean a, CallbackInfo ci, @Share("myInt") LocalIntRef b) {
        <Some code>
    }
}

(If the target class isn't an interface and the new method should be callable from somewhere else as well, the Mixin needs to be implement an interface with that method, since afaik changing the Mixin to be an interface wouldn't allow injectors and the new paramater can't be referenced from other Mixins)

This would change MyClass in the following way:

class MyClass {
    public void method(boolean a) {
-      <Some code>
+      generatedName(a, 0);
    }
+   public void methodWithInt(boolean a, int b) {
+       generatedName(a, b);
+   }
+   private void generatedName(boolean a, int b) {
+       //Initialize @Share("myInt")
+      <Some code with an injection>
+   }
+   //Injection handler for useNewMethodParameter
}

This would also work when multiple mods add parameters to the same method. However, because of the use of @Share, the added parameters can't be used in other Mixins. Doing the same with constructors would require some extra code and I think the added overload would need to be static and return the created object, since the constructor can't be called otherwise.

Pulling the code from method to generatedName would need to happen after all other injectors (similar to @WrapOperation).

LlamaLad7 commented 1 year ago

I think this would be much more of a footgun than it's worth. Sure multiple mods could add parameters in theory, but at each call-site only their own custom ones would actually be used, leaving all others with their default values. Moreover to make a vanilla call use the parameters you would need to redirect it, which is far from desirable. IMO @WrapOperationing existing usages and the thread-local approach is much better. Feel free to open if I'm missing something.