LlamaLad7 / MixinExtras

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

[Suggestion] @ModifyVariables #6

Closed MattiDragon closed 1 year ago

MattiDragon commented 2 years ago

What it does Like a @ModifyArgs is for multiple @ModifyArgs, a @ModifyVariables should be for multiple @ModifyVariables. Quite simple (unless you need to do hacks with the LVT) and should probably be in mixin, but this is the next best thing.

Why it's a good idea You often need to modify parts of a vector of color that have been split up into multiple locals, and it's annoying to have to write three of them (even if they are similar).

LlamaLad7 commented 2 years ago

I'd probably want this to use the computations already done by other mixins if possible, which would mean having to detect the presence of fabric, detect the loader version, and handle the locals key accordingly. Not impossible but slightly annoying. The other big question is what kind of structure to store the locals in, because unlike args, forcing capturing of the whole LVT would be very annoying to interact with and very unstable. So while this is a good idea, I will have to put some thought into how to implement it properly, and as such it will be more of a long-term goal.

florensie commented 2 years ago

Something like this? Still a bit messy maybe.

@ModifyVariables(method = "move", at = @At(value = "INVOKE", target = "..."))
public VariableHolder modifyCoordinates(@Variable(name = "x") int x, @Variable(name = "x") int y, @Variable(name = "x") int z, VariableHolder varHolder) {
    varHolder.put("x", x + 2);
    varHolder.put("y", x + y);
    varHolder.put("z", z - 2);
    return varHolder;
}
public static class VariableHolder {
    Map<String, Object> vars = new HashMap<>();

    public VariableHolder(String... varNames) {
        for (String varName : varNames) {
            vars.put(varName, null);
        }
    }

    public Object get(String varName) {
        return vars.get(varName);
    }

    // Will need separate put methods for primitive types
    public void put(String name, Object value) {
        if (!vars.containsKey(name)) {
            throw new IllegalArgumentException("No such variable: " + name);
        }
        vars.put(name, value);
    }
}

Code generated at the call site

VariableHolder varHolder = new VariableHolder("x", "y", "z");
callback$modifyCoordinates(x, y, z, varHolder);
x = varHolder.get("x") != null ? (int) varHolder.get("x") : x;
y = varHolder.get("y") != null ? (int) varHolder.get("y") : y;
z = varHolder.get("z") != null ? (int) varHolder.get("z") : z;
MattiDragon commented 2 years ago

I was thinking that it would be cleaner for the var holder to have the values. The used vars could maybe be specified in the @ModifyVariables annotation

LlamaLad7 commented 1 year ago

I plan to add an optional mutable arg to @Local which would mean that you can just assign to the captured locals in your handler method and their new values will be written back to the target method afterwards, which I believe is the cleanest and simplest approach.

KingContaria commented 1 year ago

would there be a way to also make the method parameters mutable?

LlamaLad7 commented 1 year ago

You can capture them already with @Local(argsOnly = true) and tbh I think modifying them that way would be clean enough given how often that would be needed.

KingContaria commented 1 year ago

oh i didnt know @Local could target parameters aswell, my bad. and yea that definitely works

LlamaLad7 commented 1 year ago

Closed by 0.2.0-beta.5