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

[Suggestion] @ModifyVariables #6

Closed
MattiDragon opened this issue Jan 22, 2022 · 8 comments
Closed

[Suggestion] @ModifyVariables #6

MattiDragon opened this issue Jan 22, 2022 · 8 comments
Labels
enhancement New feature or request

Comments

@MattiDragon
Copy link

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
Copy link
Owner

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.

@LlamaLad7 LlamaLad7 added the enhancement New feature or request label Feb 6, 2022
@florensie
Copy link

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
Copy link
Author

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
Copy link
Owner

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
Copy link

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

@LlamaLad7
Copy link
Owner

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
Copy link

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

@LlamaLad7
Copy link
Owner

Closed by 0.2.0-beta.5

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

No branches or pull requests

4 participants