MinecraftForge / ForgeGradle

Minecraft mod development framework used by Forge and FML for the gradle build system
GNU Lesser General Public License v2.1
509 stars 437 forks source link

Fields that shadow remappable fields get inadvertently remapped when referred to from a subclass #821

Closed quat1024 closed 2 years ago

quat1024 commented 3 years ago

A full compileable test case is at https://github.com/quat1024/bugtest if you want to try it

FG version "5.1.+" but I can also reproduce it on "3.0.+"

Easiest to explain by example, so here's the scene:

public abstract class Superclass extends DirectionalBlock {
  public Superclass(Properties props) {
    super(props);

    registerDefaultState(defaultBlockState()
      .setValue(FACING, Direction.UP)
      .setValue(POWERED, false));
  }

  public static final EnumProperty<Direction> FACING = DirectionalBlock.FACING; //shadows a field from superclass (!)
  public static final BooleanProperty POWERED = BlockStateProperties.POWERED; //for comparison's sake

  @Override
  protected void createBlockStateDefinition(StateContainer.Builder<Block, BlockState> builder) {
    builder.add(FACING, POWERED);
  }
}

//...

public class Subclass extends Superclass {
  public Subclass(Properties props) {
    super(props);
  }

  @Nullable
  @Override
  public BlockState getStateForPlacement(BlockItemUseContext ctx) {
    return defaultBlockState()
      .setValue(FACING, ctx.getClickedFace().getOpposite())
      .setValue(POWERED, false);
  }
}

The important parts:

After building a reobf jar with the build task and examining the output through the IntelliJ decompiler:

This fails at runtime with a NoSuchFieldError, see https://github.com/quat1024/incorporeal-2-forge/issues/11.

Only accesses from subclasses are incorrectly remapped. Referring to the public static field Superclass.FACING from an unrelated class uses the correct name of FACING.

quat1024 commented 3 years ago

unrelated, but according to the jvm spec part about field resolution it should recursively check superclasses of Superclass for a field named field_176387_N, and in production that field exists, so i dunno why this crashes at all! (still, if the fields actually held different values, it would behave differently than it does in dev)

LexManos commented 3 years ago

This would better be brought up on the repo of the tool that is used to remap mods. https://github.com/md-5/SpecialSource Weird case, but should be simple enough to do a find first.

SizableShrimp commented 2 years ago

Probably would be fixed by moving to FART. For now, this is not us. Issue is still open on SpecialSource's repo.