Fabric API

Fabric API

Mods
106M Downloads

FabricBlockSettings.copyOf(Block.Settings) can mutate argument

reoseah opened this issue · 1 comments

commented

FabricBlockSettings#copyOf(Block.Settings) returns a wrapper of an argument. Calling methods such as 'hardness(...)` on this "copy" changes original, while it's expected to only change the copy.

public static FabricBlockSettings copyOf(Block.Settings settings) {
        return new FabricBlockSettings(settings);
    }
    protected FabricBlockSettings(final Block.Settings delegate) {
        this.delegate = delegate;
    }

Instead, FabricBlockSettings#copyOf(Block.Settings) should make a deep copy of an argument and then wrap it into FabricBlockSettings. Method FabricBlockSettings#copy(Block), for example, calls Block.Settings#copy, which does make a deep copy:

public static Block.Settings copy(Block source) {
         Block.Settings settings = new Block.Settings(source.material, source.materialColor);
         settings.material = source.material;
         settings.hardness = source.hardness;
         settings.resistance = source.resistance;
         settings.collidable = source.collidable;
         settings.randomTicks = source.randomTicks;
         settings.luminance = source.lightLevel;
         settings.materialColor = source.materialColor;
         settings.soundGroup = source.soundGroup;
         settings.slipperiness = source.getSlipperiness();
         settings.dynamicBounds = source.dynamicBounds;
         return settings;
      }
commented

This has since been fixed in object builder api v1.

fabric/fabric-object-builder-api-v1/src/main/java/net/fabricmc/fabric/api/object/builder/v1/block/FabricBlockSettings.java

Lines 51 to 76 in 6d41837

super(((AbstractBlockSettingsAccessor) settings).getMaterial(), ((AbstractBlockSettingsAccessor) settings).getMaterialColorFactory());
// Mostly Copied from vanilla's copy method
AbstractBlockSettingsAccessor thisAccessor = (AbstractBlockSettingsAccessor) this;
AbstractBlockSettingsAccessor otherAccessor = (AbstractBlockSettingsAccessor) settings;
thisAccessor.setMaterial(otherAccessor.getMaterial());
this.hardness(otherAccessor.getHardness());
this.resistance(otherAccessor.getResistance());
this.collidable(otherAccessor.getCollidable());
thisAccessor.setRandomTicks(otherAccessor.getRandomTicks());
this.lightLevel(otherAccessor.getLuminance());
thisAccessor.setMaterialColorFactory(otherAccessor.getMaterialColorFactory());
this.sounds(otherAccessor.getSoundGroup());
this.slipperiness(otherAccessor.getSlipperiness());
this.velocityMultiplier(otherAccessor.getVelocityMultiplier());
thisAccessor.setDynamicBounds(otherAccessor.getDynamicBounds());
thisAccessor.setOpaque(otherAccessor.getOpaque());
thisAccessor.setIsAir(otherAccessor.getIsAir());
// Now attempt to copy fabric specific data
BlockSettingsInternals otherInternals = (BlockSettingsInternals) settings;
FabricBlockInternals.ExtraData extraData = otherInternals.getExtraData();
if (extraData != null) { // If present, populate the extra data on our new settings
((BlockSettingsInternals) this).setExtraData(extraData);
}

  • 👍1