From 03f9436ef9e97b326cb6848cecc6d749cbed7459 Mon Sep 17 00:00:00 2001 From: thatsIch Date: Sun, 26 Apr 2015 09:57:42 +0200 Subject: [PATCH] Fixes #1331: Happened on deactivating features for intermediate crafting components If a feature dependency of ItemMultiMaterial was disabled, the returned value was never assigned with the constructed. Pulling out the construction and setting it before checking it, prevents the NPE and also matches the behaviour in ItemMultiPart, where parts are constructed, but never registered. --- .../api/definitions/IBlockDefinition.java | 13 ++++ .../definitions/IComparableDefinition.java | 7 +++ .../appeng/core/features/BlockDefinition.java | 15 +++-- .../core/features/DamagedItemDefinition.java | 7 ++- .../core/features/DefinitionConverter.java | 8 ++- .../appeng/core/features/ItemDefinition.java | 4 +- .../appeng/core/features/ItemStackSrc.java | 3 + .../appeng/core/features/TileDefinition.java | 7 ++- .../features/WrappedDamageItemDefinition.java | 22 ++++--- .../items/materials/ItemMultiMaterial.java | 59 +++++++++---------- .../appeng/items/parts/ItemMultiPart.java | 21 ++++--- 11 files changed, 108 insertions(+), 58 deletions(-) diff --git a/src/api/java/appeng/api/definitions/IBlockDefinition.java b/src/api/java/appeng/api/definitions/IBlockDefinition.java index 7be25bc38..3acbd0896 100644 --- a/src/api/java/appeng/api/definitions/IBlockDefinition.java +++ b/src/api/java/appeng/api/definitions/IBlockDefinition.java @@ -3,6 +3,7 @@ package appeng.api.definitions; import net.minecraft.block.Block; import net.minecraft.item.ItemBlock; +import net.minecraft.world.IBlockAccess; import com.google.common.base.Optional; @@ -18,4 +19,16 @@ public interface IBlockDefinition extends IItemDefinition * @return the {@link ItemBlock} implementation if applicable */ Optional maybeItemBlock(); + + /** + * Compare Block with world. + * + * @param world world of block + * @param x x pos of block + * @param y y pos of block + * @param z z pos of block + * + * @return if the block is placed in the world at the specific location. + */ + boolean isSameAs( IBlockAccess world, int x, int y, int z ); } diff --git a/src/api/java/appeng/api/definitions/IComparableDefinition.java b/src/api/java/appeng/api/definitions/IComparableDefinition.java index db1d9b777..bec1bde11 100644 --- a/src/api/java/appeng/api/definitions/IComparableDefinition.java +++ b/src/api/java/appeng/api/definitions/IComparableDefinition.java @@ -7,6 +7,10 @@ import net.minecraft.world.IBlockAccess; /** * Interface to compare a definition with an itemstack or a block + * + * @author thatsIch + * @version rv2 + * @since rv2 */ public interface IComparableDefinition { @@ -28,6 +32,9 @@ public interface IComparableDefinition * @param z z pos of block * * @return if the block is placed in the world at the specific location. + * + * @deprecated moved to {@link IBlockDefinition}. Is removed in the next major release rv3 */ + @Deprecated boolean isSameAs( IBlockAccess world, int x, int y, int z ); } diff --git a/src/main/java/appeng/core/features/BlockDefinition.java b/src/main/java/appeng/core/features/BlockDefinition.java index e50c403f0..d8ffa0beb 100644 --- a/src/main/java/appeng/core/features/BlockDefinition.java +++ b/src/main/java/appeng/core/features/BlockDefinition.java @@ -21,15 +21,16 @@ package appeng.core.features; import java.lang.reflect.Constructor; -import com.google.common.base.Optional; -import com.google.common.collect.ObjectArrays; - import net.minecraft.block.Block; import net.minecraft.item.Item; import net.minecraft.item.ItemBlock; import net.minecraft.item.ItemStack; import net.minecraft.world.IBlockAccess; +import com.google.common.base.Optional; +import com.google.common.base.Preconditions; +import com.google.common.collect.ObjectArrays; + import appeng.api.definitions.IBlockDefinition; import appeng.block.AEBaseBlock; @@ -42,20 +43,22 @@ public class BlockDefinition extends ItemDefinition implements IBlockDefinition public BlockDefinition( Block block, ActivityState state ) { super( constructItemFromBlock( block ), state ); - assert block != null; + + Preconditions.checkNotNull( block ); + Preconditions.checkNotNull( state ); this.block = block; this.enabled = state == ActivityState.Enabled; } @Override - public Optional maybeBlock() + public final Optional maybeBlock() { return Optional.of( this.block ); } @Override - public Optional maybeItemBlock() + public final Optional maybeItemBlock() { if( this.enabled ) { diff --git a/src/main/java/appeng/core/features/DamagedItemDefinition.java b/src/main/java/appeng/core/features/DamagedItemDefinition.java index ad6f22fc0..5bad94233 100644 --- a/src/main/java/appeng/core/features/DamagedItemDefinition.java +++ b/src/main/java/appeng/core/features/DamagedItemDefinition.java @@ -19,11 +19,14 @@ package appeng.core.features; +import javax.annotation.Nonnull; + import net.minecraft.item.Item; import net.minecraft.item.ItemStack; import net.minecraft.world.IBlockAccess; import com.google.common.base.Optional; +import com.google.common.base.Preconditions; import appeng.api.definitions.IItemDefinition; @@ -32,9 +35,9 @@ public final class DamagedItemDefinition implements IItemDefinition { private final IStackSrc source; - public DamagedItemDefinition( IStackSrc source ) + public DamagedItemDefinition( @Nonnull IStackSrc source ) { - this.source = source; + this.source = Preconditions.checkNotNull( source ); } @Override diff --git a/src/main/java/appeng/core/features/DefinitionConverter.java b/src/main/java/appeng/core/features/DefinitionConverter.java index bcad08c05..8f022955a 100644 --- a/src/main/java/appeng/core/features/DefinitionConverter.java +++ b/src/main/java/appeng/core/features/DefinitionConverter.java @@ -88,7 +88,7 @@ public final class DefinitionConverter @Override public boolean sameAsBlock( IBlockAccess world, int x, int y, int z ) { - return this.definition.isSameAs( world, x, y, z ); + return false; } } @@ -137,6 +137,12 @@ public final class DefinitionConverter { return this.definition.maybeBlock().orNull(); } + + @Override + public boolean sameAsBlock( IBlockAccess world, int x, int y, int z ) + { + return this.definition.isSameAs( world, x, y, z ); + } } diff --git a/src/main/java/appeng/core/features/ItemDefinition.java b/src/main/java/appeng/core/features/ItemDefinition.java index f607b87a9..f31557c0b 100644 --- a/src/main/java/appeng/core/features/ItemDefinition.java +++ b/src/main/java/appeng/core/features/ItemDefinition.java @@ -24,6 +24,7 @@ import net.minecraft.item.ItemStack; import net.minecraft.world.IBlockAccess; import com.google.common.base.Optional; +import com.google.common.base.Preconditions; import appeng.api.definitions.IItemDefinition; import appeng.util.Platform; @@ -36,7 +37,8 @@ public class ItemDefinition implements IItemDefinition public ItemDefinition( Item item, ActivityState state ) { - assert item != null; + Preconditions.checkNotNull( item ); + Preconditions.checkNotNull( state ); this.item = item; this.enabled = state == ActivityState.Enabled; diff --git a/src/main/java/appeng/core/features/ItemStackSrc.java b/src/main/java/appeng/core/features/ItemStackSrc.java index 734f67f12..3966ac24a 100644 --- a/src/main/java/appeng/core/features/ItemStackSrc.java +++ b/src/main/java/appeng/core/features/ItemStackSrc.java @@ -19,6 +19,8 @@ package appeng.core.features; +import javax.annotation.Nullable; + import net.minecraft.block.Block; import net.minecraft.item.Item; import net.minecraft.item.ItemStack; @@ -45,6 +47,7 @@ public class ItemStackSrc implements IStackSrc this.damage = dmg; } + @Nullable @Override public ItemStack stack( int i ) { diff --git a/src/main/java/appeng/core/features/TileDefinition.java b/src/main/java/appeng/core/features/TileDefinition.java index 813e43331..f97728259 100644 --- a/src/main/java/appeng/core/features/TileDefinition.java +++ b/src/main/java/appeng/core/features/TileDefinition.java @@ -22,6 +22,7 @@ package appeng.core.features; import net.minecraft.tileentity.TileEntity; import com.google.common.base.Optional; +import com.google.common.base.Preconditions; import appeng.api.definitions.ITileDefinition; import appeng.block.AEBaseBlock; @@ -34,7 +35,11 @@ public final class TileDefinition extends BlockDefinition implements ITileDefini public TileDefinition( AEBaseBlock block, ActivityState state ) { super( block, state ); - assert !block.hasBlockTileEntity() || block.getTileEntityClass() != null; + + Preconditions.checkNotNull( block ); + Preconditions.checkNotNull( state ); + + Preconditions.checkState( !block.hasBlockTileEntity() || block.getTileEntityClass() != null ); this.block = block; } diff --git a/src/main/java/appeng/core/features/WrappedDamageItemDefinition.java b/src/main/java/appeng/core/features/WrappedDamageItemDefinition.java index deb77b370..5196d8144 100644 --- a/src/main/java/appeng/core/features/WrappedDamageItemDefinition.java +++ b/src/main/java/appeng/core/features/WrappedDamageItemDefinition.java @@ -19,8 +19,6 @@ package appeng.core.features; -import javax.annotation.Nullable; - import net.minecraft.block.Block; import net.minecraft.item.Item; import net.minecraft.item.ItemBlock; @@ -30,6 +28,7 @@ import net.minecraft.world.IBlockAccess; import com.google.common.base.Function; import com.google.common.base.Optional; +import com.google.common.base.Preconditions; import appeng.api.definitions.ITileDefinition; @@ -41,6 +40,9 @@ public final class WrappedDamageItemDefinition implements ITileDefinition public WrappedDamageItemDefinition( ITileDefinition definition, int damage ) { + Preconditions.checkNotNull( definition ); + Preconditions.checkArgument( damage >= 0 ); + this.definition = definition; this.damage = damage; } @@ -72,7 +74,7 @@ public final class WrappedDamageItemDefinition implements ITileDefinition @Override public Optional maybeStack( final int stackSize ) { - return this.definition.maybeBlock().transform( new BlockTransformFunction( stackSize ) ); + return this.definition.maybeBlock().transform( new BlockTransformFunction( stackSize, this.damage ) ); } @Override @@ -92,20 +94,26 @@ public final class WrappedDamageItemDefinition implements ITileDefinition return this.definition.isSameAs( world, x, y, z ) && world.getBlockMetadata( x, y, z ) == this.damage; } - private class BlockTransformFunction implements Function + private static final class BlockTransformFunction implements Function { private final int stackSize; + private final int damage; - public BlockTransformFunction( int stackSize ) + public BlockTransformFunction( int stackSize, int damage ) { + Preconditions.checkArgument( stackSize > 0 ); + Preconditions.checkArgument( damage >= 0 ); + this.stackSize = stackSize; + this.damage = damage; } - @Nullable @Override public ItemStack apply( Block input ) { - return new ItemStack( input, this.stackSize, WrappedDamageItemDefinition.this.damage ); + Preconditions.checkNotNull( input ); + + return new ItemStack( input, this.stackSize, this.damage ); } } } diff --git a/src/main/java/appeng/items/materials/ItemMultiMaterial.java b/src/main/java/appeng/items/materials/ItemMultiMaterial.java index cb9441411..ff7e5f8de 100644 --- a/src/main/java/appeng/items/materials/ItemMultiMaterial.java +++ b/src/main/java/appeng/items/materials/ItemMultiMaterial.java @@ -47,6 +47,7 @@ import net.minecraft.world.World; import net.minecraftforge.common.util.ForgeDirection; import net.minecraftforge.oredict.OreDictionary; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSet; import appeng.api.config.Upgrades; @@ -68,7 +69,7 @@ import appeng.util.InventoryAdaptor; import appeng.util.Platform; -public class ItemMultiMaterial extends AEBaseItem implements IStorageComponent, IUpgradeModule +public final class ItemMultiMaterial extends AEBaseItem implements IStorageComponent, IUpgradeModule { public static final int KILO = 1024; public static ItemMultiMaterial instance; @@ -172,40 +173,36 @@ public class ItemMultiMaterial extends AEBaseItem implements IStorageComponent, public IStackSrc createMaterial( MaterialType mat ) { - if( !mat.isRegistered() ) + Preconditions.checkState( !mat.isRegistered(), "Cannot create the same material twice." ); + + boolean enabled = true; + + for( AEFeature f : mat.getFeature() ) { - boolean enabled = true; - for( AEFeature f : mat.getFeature() ) - { - enabled = enabled && AEConfig.instance.isFeatureEnabled( f ); - } - - if( enabled ) - { - mat.itemInstance = this; - int newMaterialNum = mat.damageValue; - mat.markReady(); - - mat.stackSrc = new MaterialStackSrc( mat ); - - if( this.dmgToMaterial.get( newMaterialNum ) == null ) - { - this.dmgToMaterial.put( newMaterialNum, mat ); - } - else - { - throw new IllegalStateException( "Meta Overlap detected." ); - } - - return mat.stackSrc; - } - - return mat.stackSrc; + enabled = enabled && AEConfig.instance.isFeatureEnabled( f ); } - else + + mat.stackSrc = new MaterialStackSrc( mat ); + + if( enabled ) { - throw new IllegalStateException( "Cannot create the same material twice..." ); + mat.itemInstance = this; + mat.markReady(); + int newMaterialNum = mat.damageValue; + + + if( this.dmgToMaterial.get( newMaterialNum ) == null ) + { + this.dmgToMaterial.put( newMaterialNum, mat ); + } + else + + { + throw new IllegalStateException( "Meta Overlap detected." ); + } } + + return mat.stackSrc; } public void makeUnique() diff --git a/src/main/java/appeng/items/parts/ItemMultiPart.java b/src/main/java/appeng/items/parts/ItemMultiPart.java index 061d3af2a..23b3e3541 100644 --- a/src/main/java/appeng/items/parts/ItemMultiPart.java +++ b/src/main/java/appeng/items/parts/ItemMultiPart.java @@ -63,6 +63,8 @@ import appeng.items.AEBaseItem; public final class ItemMultiPart extends AEBaseItem implements IPartItem, IItemGroup { + private static final Comparator> REGISTERED_COMPARATOR = new RegisteredComparator(); + public static ItemMultiPart instance; private final NameResolver nameResolver; private final Map registered; @@ -216,15 +218,7 @@ public final class ItemMultiPart extends AEBaseItem implements IPartItem, IItemG public void getSubItems( Item number, CreativeTabs tab, List cList ) { List> types = new ArrayList>( this.registered.entrySet() ); - Collections.sort( types, new Comparator>() - { - - @Override - public int compare( Entry o1, Entry o2 ) - { - return o1.getValue().part.name().compareTo( o2.getValue().part.name() ); - } - } ); + Collections.sort( types, REGISTERED_COMPARATOR ); for( Entry part : types ) { @@ -375,4 +369,13 @@ public final class ItemMultiPart extends AEBaseItem implements IPartItem, IItemG this.variant = variant; } } + + private static final class RegisteredComparator implements Comparator> + { + @Override + public int compare( Entry o1, Entry o2 ) + { + return o1.getValue().part.name().compareTo( o2.getValue().part.name() ); + } + } }