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] `@ModifyComparisonArg` #21

Closed xanderstuff closed 1 month ago

xanderstuff commented 1 year ago

This would wrap the entire left-hand or right-hand side of a comparison, which could be chained by multiple mixins.

Example:

@ModifyComparisonArg(
    method = "targetMethod", 
    comparison = "LESS_THAN_OR_EQUAL",
    argument = "SECOND" //or maybe side = "RIGHT_SIDE"?
)
private TargetType wrapExpression(TargetType expression){
    //TargetType is either int, long, float, double, boolean(?)
    return expression + 5;
}

Code diff:

- if(x * i <= foo.get() * 4){
+ if(x * i <= wrapExpression(foo.get() * 4)){
    // stuff
}

Note: in this example code diff, there is no way to add 5 to either side. If you used a @ModifyExpressionValue on foo.get(), then any value you add gets multiplied by 4 (which will not be the same result). And if you used a @ModifyConstant or @Redirect on the field access, then (in this example) any change you make will also get multiplied by the other values in the expression (again, not the same result), while also not being compatible with any other mixins using the same target.

xanderstuff commented 1 year ago

Question: would it be beneficial to do something similar for logical operations (AND, OR)? or does @ModifyExpressionValue cover every possible use-case scenario (since logical operations are much simpler)?

xanderstuff commented 1 year ago

I haven't put much thought into the name yet, so here's some alternative names: @WrapComparisonArg, @ModifyComparisonExpression, @WrapComparisonExpression.

LlamaLad7 commented 1 year ago

The trouble is that targeting such operations is inherently brittle because the instructions are so generic. Slices make it a bit better but it's still not ideal. I may implement something along these lines in the future but it's not a priority because the situations are fairly esoteric (I've yet to encounter anything like you've described in the wild). If there's any specific scenarios you'd like this for, feel free to ask about them.

LlamaLad7 commented 7 months ago

I do have a plan for this now.