Bulldog83 / JustMap

A minimap mod for Minecraft with Fabric launcher.
GNU General Public License v3.0
42 stars 17 forks source link

Compatibility issue with Sodium #71

Closed calloatti closed 4 years ago

calloatti commented 4 years ago

MC 1.16.1

justmap-1.1.92-1.16.1-unlimited.jar

https://github.com/jellysquid3/sodium-fabric/releases/tag/mc1.16.1-0.1.0

Log:

[main/WARN]: Method overwrite conflict for getSprite in justmap.mixins.client.json:BakedQuadMixin, previously written by me.jellysquid.mods.sodium.mixin.pipeline.MixinBakedQuad. Skipping method.

Suggested on the Fabric Discord:

Tell him to rewrite the mixin as either an accessor or to make it compatible with Sodium's properly

Discord: https://discord.com/channels/507304429255393322/507982463449169932/731956542269685882

I am just the messenger, and have no idea what all this is about. Thank you.

PhoenixVX commented 4 years ago

Posting the Sodium mixin here incase you'd rather not rewrite the mixin as an access widener.

package me.jellysquid.mods.sodium.mixin.pipeline;

import me.jellysquid.mods.sodium.client.model.quad.ModelQuadView;
import me.jellysquid.mods.sodium.client.model.quad.properties.ModelQuadFlags;
import net.minecraft.client.render.model.BakedQuad;
import net.minecraft.client.texture.Sprite;
import net.minecraft.util.math.Direction;
import org.spongepowered.asm.mixin.Final;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Shadow;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;

import static me.jellysquid.mods.sodium.client.util.ModelQuadUtil.*;

@Mixin(BakedQuad.class)
public class MixinBakedQuad implements ModelQuadView {
    @Shadow
    @Final
    protected int[] vertexData;

    @Shadow
    @Final
    protected Sprite sprite;

    @Shadow
    @Final
    protected int colorIndex;
    private int cachedFlags;

    @Inject(method = "", at = @At("RETURN"))
    private void init(int[] vertexData, int colorIndex, Direction face, Sprite sprite, boolean shade, CallbackInfo ci) {
        this.cachedFlags = ModelQuadFlags.getQuadFlags((BakedQuad) (Object) this);
    }

    @Override
    public float getX(int idx) {
        return Float.intBitsToFloat(this.vertexData[vertexOffset(idx) + POSITION_INDEX]);
    }

    @Override
    public float getY(int idx) {
        return Float.intBitsToFloat(this.vertexData[vertexOffset(idx) + POSITION_INDEX + 1]);
    }

    @Override
    public float getZ(int idx) {
        return Float.intBitsToFloat(this.vertexData[vertexOffset(idx) + POSITION_INDEX + 2]);
    }

    @Override
    public int getColor(int idx) {
        return this.vertexData[vertexOffset(idx) + COLOR_INDEX];
    }

    @Override
    public Sprite getSprite() {
        return this.sprite;
    }

    @Override
    public float getTexU(int idx) {
        return Float.intBitsToFloat(this.vertexData[vertexOffset(idx) + TEXTURE_INDEX]);
    }

    @Override
    public float getTexV(int idx) {
        return Float.intBitsToFloat(this.vertexData[vertexOffset(idx) + TEXTURE_INDEX + 1]);
    }

    @Override
    public int getFlags() {
        return this.cachedFlags;
    }

    @Override
    public int getLight(int idx) {
        return this.vertexData[vertexOffset(idx) + LIGHT_INDEX];
    }

    @Override
    public int getNormal(int idx) {
        return this.vertexData[vertexOffset(idx) + NORMAL_INDEX];
    }

    @Override
    public int getColorIndex() {
        return this.colorIndex;
    }
}

The reason why i said to

Tell him to rewrite the mixin as either an accessor or to make it compatible with Sodium's properly

is because that mixin functions as an access widener and causes needless conflicts with other mods as indicated by this.

comp500 commented 4 years ago

This doesn't actually seem to cause any issues, only a warning - but marking the mixin method as an Accessor may help Mixin's warning checks here and remove the need to shadow the variable, see https://github.com/SpongePowered/Mixin/issues/307#issuecomment-467510303. Definitely don't make it an access widener - there are several reasons why Accessors are preferred to access wideners.