Mixin icon indicating copy to clipboard operation
Mixin copied to clipboard

Add backwards option to @At

Open Earthcomputer opened this issue 5 years ago • 1 comments

Consider the following target method (and imagine it being much longer and more complicated):

void foo() {
  blah();
  bar();
  stuff();
  bar();
  derp();
  bar();
  backflip();
}

Also let's assume that this is a highly volatile method that tends to change every update.

Now let's say you want to @Redirect the last occurrence of bar(). Of course, you could use ordinal = 2 in your @At, but what if there are many occurrences in the method, and that number is likely to change between updates? Another scenario, let's say you want to target the bar() which occurs directly before derp(). This call to bar() is logically connected to the call to derp(), so even though the method might change a lot, that very local slice of the method is unlikely to. Both scenarios are actually equivalent if you use @Slice(to = @At("bar")) (pseudocode) in the second case.

Mixin provides an easy way to target by an ordinal forwards relative to the start of the slice, but provides no way to target by an ordinal backwards relative to the end of the slice.

The fix I propose is to introduce a backwards = true option to @At. If ordinal = -1, then this option would have no effect. Otherwise, given a list of injection targets, targets, found by the InjectionPoint, it would return the targets.length - ordinal - 1th injection target.

Earthcomputer avatar Jan 08 '20 17:01 Earthcomputer

I've actually mentioned this a few times in the past but I don't think it's centralised anywhere so this is as good a place as any; basically the built-in injection points are not really meant to be the be-all and end-all of injection behaviour.

The idea is that basically using clever combinations of slices and the built-in injection points should cover like 90-95% of injection scenarios. In fact in pre-mixin EIT days when injectors were declared programatically instead of via annotations you could boolean injectors together using and, or, after, before and shift which are now purely vestigial.

In a nutshell, the built-in injection points are supposed to be simple, easy to use, and with enough flexibility to achieve most things, and when coupled with slices achieve the same stuff as the old and and or combinators. What they aren't meant to do is cover every possible scenario in order to keep them simple. That's why custom injection points are well-supported if not that frequently used.

That's not to say that simple extensions to the built-in injection points won't be considered, or that if something fundamental and widely useful comes along it won't be added to the built-ins, just that in general it's anticipated that for one-offs you'll just bundle an injection point with your mod.


Custom injection points can be specified in the @At either by specifying the full injection point class name in the annotation: eg: @At("my.custom.injection.Point") or by registering the class in your config's injectors block and decorating the injection point class with the AtCode annotation similar to the built-in injection points. eg:

{
    "mixins": [
        "Whatever",
    ],
    "injectors": {
        "injectionPoints": [
            "my.custom.injection.MyCustomInjectionPoint"
        ]
    }
}
@AtCode("MY_CUSTOM")
class MyCustomInjectionPoint extends InjectionPoint {
}

which can then be used in your at annotation as @At("MY_CUSTOM").

An example of a custom injection point with entirely custom logic if you want to see one can be found here (this is actually an EIT injection point but they are functionally identical to mixin injection points because (spoilers) EIT became mixin).


So the question that basically arises is, given that the current injection points cover the majority of uses, and custom injection points are easy to add (and can be a lot smarter than ordinal-based selection), should something like a reverse option still be added, or would it be better to keep the current behaviour of simple-but-utilitarian builtins with a preference for custom injection points as needed?

Mumfrey avatar Jan 08 '20 20:01 Mumfrey