Fix ConcurrentModificatonException in NetworkEventBus (#3660)

* Fixes crash when either quartz types are disabled and prevent cascading worldgen.
* Also fixes  #3486

src/main/java/appeng/block/storage/BlockDrive.java

@@ -74,13 +74,8 @@ public class BlockDrive extends AEBaseTileBlock
 	public IBlockState getExtendedState( IBlockState state, IBlockAccess world, BlockPos pos )
 	{
 		TileDrive te = this.getTileEntity( world, pos );
-		if( te == null )
-		{
-			return super.getExtendedState( state, world, pos );
-		}
-
 		IExtendedBlockState extState = (IExtendedBlockState) super.getExtendedState( state, world, pos );
-		return extState.withProperty( SLOTS_STATE, DriveSlotsState.fromChestOrDrive( te ) );
+		return extState.withProperty( SLOTS_STATE, te == null ? DriveSlotsState.createEmpty( 10 ) : DriveSlotsState.fromChestOrDrive( te ) );
 	}
 
 	@Override

src/main/java/appeng/block/storage/DriveSlotsState.java

@@ -75,4 +75,14 @@ public class DriveSlotsState
 		}
 		return new DriveSlotsState( slots );
 	}
+
+	public static DriveSlotsState createEmpty( int slotCount )
+	{
+		DriveSlotState[] slots = new DriveSlotState[slotCount];
+		for( int i = 0; i < slotCount; i++ )
+		{
+			slots[i] = DriveSlotState.EMPTY;
+		}
+		return new DriveSlotsState( slots );
+	}
 }

src/main/java/appeng/container/AEBaseContainer.java

@@ -356,6 +356,11 @@ public abstract class AEBaseContainer extends Container
 
 		if( Platform.isServer() )
 		{
+			if( this.tileEntity != null && this.tileEntity.getWorld().getTileEntity( this.tileEntity.getPos() ) != this.tileEntity )
+			{
+				this.setValidContainer( false );
+			}
+
 			for( final IContainerListener listener : this.listeners )
 			{
 				for( final SyncData sd : this.syncData.values() )
@@ -431,6 +436,8 @@ public abstract class AEBaseContainer extends Container
 			}
 			else
 			{
+				tis = tis.copy();
+
 				// target slots in the container...
 				for( final Object inventorySlot : this.inventorySlots )
 				{
@@ -491,7 +498,7 @@ public abstract class AEBaseContainer extends Container
 					{
 						if( d.getHasStack() )
 						{
-							final ItemStack t = d.getStack();
+							final ItemStack t = d.getStack().copy();
 
 							if( Platform.itemComparisons().isSameItem( tis, t ) ) // t.isItemEqual(tis))
 							{
@@ -511,6 +518,8 @@ public abstract class AEBaseContainer extends Container
 								t.setCount( t.getCount() + placeAble );
 								tis.setCount( tis.getCount() - placeAble );
 
+								d.putStack( t );
+
 								if( tis.getCount() <= 0 )
 								{
 									clickSlot.putStack( ItemStack.EMPTY );
@@ -543,7 +552,7 @@ public abstract class AEBaseContainer extends Container
 					{
 						if( d.getHasStack() )
 						{
-							final ItemStack t = d.getStack();
+							final ItemStack t = d.getStack().copy();
 
 							if( Platform.itemComparisons().isSameItem( t, tis ) )
 							{
@@ -563,6 +572,8 @@ public abstract class AEBaseContainer extends Container
 								t.setCount( t.getCount() + placeAble );
 								tis.setCount( tis.getCount() - placeAble );
 
+								d.putStack( t );
+
 								if( tis.getCount() <= 0 )
 								{
 									clickSlot.putStack( ItemStack.EMPTY );
@@ -621,7 +632,7 @@ public abstract class AEBaseContainer extends Container
 				}
 			}
 
-			clickSlot.putStack( !tis.isEmpty() ? tis.copy() : ItemStack.EMPTY );
+			clickSlot.putStack( !tis.isEmpty() ? tis : ItemStack.EMPTY );
 		}
 
 		this.updateSlot( clickSlot );

src/main/java/appeng/container/implementations/ContainerCellWorkbench.java

@@ -148,11 +148,6 @@ public class ContainerCellWorkbench extends ContainerUpgradeable
 		final ItemStack is = this.workBench.getInventoryByName( "cell" ).getStackInSlot( 0 );
 		if( Platform.isServer() )
 		{
-			if( this.workBench.getWorld().getTileEntity( this.workBench.getPos() ) != this.workBench )
-			{
-				this.setValidContainer( false );
-			}
-
 			for( final IContainerListener listener : this.listeners )
 			{
 				if( this.prevStack != is )

src/main/java/appeng/core/AEConfig.java

@@ -213,7 +213,9 @@ public final class AEConfig extends Configuration implements IConfigurableObject
 		{
 			if( feature.isVisible() )
 			{
-				if( this.get( "Features." + feature.category(), feature.key(), feature.isEnabled() ).getBoolean( feature.isEnabled() ) )
+				final Property option = this.get( "Features." + feature.category(), feature.key(), feature.isEnabled(), feature.comment() );
+
+				if( option.getBoolean( feature.isEnabled() ) )
 				{
 					this.featureFlags.add( feature );
 				}

src/main/java/appeng/core/features/AEFeature.java

@@ -155,6 +155,7 @@ public enum AEFeature
 	MOLECULAR_ASSEMBLER( "MolecularAssembler", Constants.CATEGORY_CRAFTING_FEATURES ),
 	PATTERNS( "Patterns", Constants.CATEGORY_CRAFTING_FEATURES ),
 	CRAFTING_CPU( "CraftingCPU", Constants.CATEGORY_CRAFTING_FEATURES ),
+	CRAFTING_MANAGER_FALLBACK( "CraftingManagerFallback", Constants.CATEGORY_CRAFTING_FEATURES, "Use CraftingManager to find an alternative recipe, after a pattern rejected an ingredient. Should be enabled to avoid issues, but can have a minor performance impact." ),
 
 	BASIC_CARDS( "BasicCards", Constants.CATEGORY_UPGRADES ),
 	ADVANCED_CARDS( "AdvancedCards", Constants.CATEGORY_UPGRADES ),
@@ -178,17 +179,29 @@ public enum AEFeature
 	private final String key;
 	private final String category;
 	private final boolean enabled;
+	private final String comment;
 
 	AEFeature( final String key, final String cat )
 	{
 		this( key, cat, true );
 	}
 
+	AEFeature( final String key, final String cat, final String comment )
+	{
+		this( key, cat, true, comment );
+	}
+
 	AEFeature( final String key, final String cat, final boolean enabled )
+	{
+		this( key, cat, enabled, null );
+	}
+
+	AEFeature( final String key, final String cat, final boolean enabled, final String comment )
 	{
 		this.key = key;
 		this.category = cat;
 		this.enabled = enabled;
+		this.comment = comment;
 	}
 
 	/**
@@ -216,6 +229,11 @@ public enum AEFeature
 		return this.enabled;
 	}
 
+	public String comment()
+	{
+		return this.comment;
+	}
+
 	private enum Constants
 	{
 		;

src/main/java/appeng/decorative/solid/BlockQuartzOre.java

@@ -27,6 +27,7 @@ import net.minecraft.item.Item;
 import net.minecraft.util.BlockRenderLayer;
 import net.minecraft.util.math.BlockPos;
 import net.minecraft.util.math.MathHelper;
+import net.minecraft.world.IBlockAccess;
 import net.minecraft.world.World;
 
 import appeng.api.AEApi;
@@ -76,16 +77,15 @@ public class BlockQuartzOre extends AEBaseBlock
 	}
 
 	@Override
-	public void dropBlockAsItemWithChance( final World w, final BlockPos pos, final IBlockState state, final float chance, final int fortune )
+	public int getExpDrop( IBlockState state, IBlockAccess world, BlockPos pos, int fortune )
 	{
-		super.dropBlockAsItemWithChance( w, pos, state, chance, fortune );
-
-		if( this.getItemDropped( state, w.rand, fortune ) != Item.getItemFromBlock( this ) )
-		{
-			final int xp = MathHelper.getInt( w.rand, 2, 5 );
-
-			this.dropXpOnBlockBreak( w, pos, xp );
+		Random rand = world instanceof World ? ( (World) world ).rand : new Random();
+	
+	    if ( this.getItemDropped( state, rand, fortune ) != Item.getItemFromBlock( this ) )  
+	    {
+			return MathHelper.getInt( rand, 2, 5 );
 		}
+		return super.getExpDrop( state, world, pos, fortune );
 	}
 
 	@Override

src/main/java/appeng/helpers/PatternHelper.java

@@ -25,6 +25,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.StringJoiner;
 
 import net.minecraft.inventory.InventoryCrafting;
 import net.minecraft.item.Item;
@@ -40,6 +41,9 @@ import appeng.api.networking.crafting.ICraftingPatternDetails;
 import appeng.api.storage.channels.IItemStorageChannel;
 import appeng.api.storage.data.IAEItemStack;
 import appeng.container.ContainerNull;
+import appeng.core.AEConfig;
+import appeng.core.AELog;
+import appeng.core.features.AEFeature;
 import appeng.util.ItemSorters;
 import appeng.util.Platform;
 import appeng.util.item.AEItemStack;
@@ -265,7 +269,7 @@ public class PatternHelper implements ICraftingPatternDetails, Comparable<Patter
 				return true;
 			}
 		}
-		else
+		else if( AEConfig.instance().isFeatureEnabled( AEFeature.CRAFTING_MANAGER_FALLBACK ) )
 		{
 			final ItemStack testOutput = CraftingManager.findMatchingResult( this.testFrame, w );
 
@@ -273,8 +277,16 @@ public class PatternHelper implements ICraftingPatternDetails, Comparable<Patter
 			{
 				this.testFrame.setInventorySlotContents( slotIndex, this.crafting.getStackInSlot( slotIndex ) );
 				this.markItemAs( slotIndex, i, TestStatus.ACCEPT );
+
+				if( AELog.isCraftingDebugLogEnabled() )
+				{
+					this.warnAboutCraftingManager( true );
+				}
+
 				return true;
 			}
+
+			this.warnAboutCraftingManager( false );
 		}
 
 		this.markItemAs( slotIndex, i, TestStatus.DECLINE );
@@ -395,6 +407,24 @@ public class PatternHelper implements ICraftingPatternDetails, Comparable<Patter
 		return this.pattern.hashCode();
 	}
 
+	private void warnAboutCraftingManager( boolean foundAlternative )
+	{
+		final String foundAlternativeRecipe = foundAlternative ? "Found alternative recipe." : "NOT FOUND, please report.";
+
+		final StringJoiner joinActualInputs = new StringJoiner( ", " );
+		for( int j = 0; j < this.testFrame.getSizeInventory(); j++ )
+		{
+			final ItemStack stack = this.testFrame.getStackInSlot( j );
+			if( !stack.isEmpty() )
+			{
+				joinActualInputs.add( stack.toString() );
+			}
+		}
+
+		AELog.warn( "Using CraftingManager fallback: Recipe <%s> for output <%s> rejected inputs [%s]. %s",
+				this.standardRecipe.getRegistryName(), this.standardRecipe.getRecipeOutput(), joinActualInputs, foundAlternativeRecipe );
+	}
+
 	@Override
 	public boolean equals( final Object obj )
 	{

src/main/java/appeng/items/tools/ToolNetworkTool.java

@@ -83,16 +83,9 @@ public class ToolNetworkTool extends AEBaseItem implements IGuiItem, IAEWrench,
 		{
 			final RayTraceResult mop = AppEng.proxy.getRTR();
 
-			if( mop == null )
+			if( mop == null || mop.typeOfHit == RayTraceResult.Type.MISS )
 			{
-				this.onItemUseFirst( p, w, new BlockPos( 0, 0, 0 ), null, 0, 0, 0, hand ); // eh?
-			}
-			else
-			{
-				if( w.getBlockState( mop.getBlockPos() ).getBlock().isAir( w.getBlockState( mop.getBlockPos() ), w, mop.getBlockPos() ) )
-				{
-					this.onItemUseFirst( p, w, new BlockPos( 0, 0, 0 ), null, 0, 0, 0, hand ); // eh?
-				}
+				NetworkHandler.instance().sendToServer( new PacketClick( BlockPos.ORIGIN, null, 0, 0, 0, hand ) );
 			}
 		}
 

src/main/java/appeng/items/tools/quartz/ToolQuartzWrench.java

@@ -67,7 +67,6 @@ public class ToolQuartzWrench extends AEBaseItem implements IAEWrench, IToolHamm
 
 			if( b.rotateBlock( world, pos, side ) )
 			{
-				b.neighborChanged( Platform.AIR_BLOCK.getDefaultState(), world, pos, Platform.AIR_BLOCK, pos );
 				player.swingArm( hand );
 				return !world.isRemote ? EnumActionResult.SUCCESS : EnumActionResult.FAIL;
 			}

src/main/java/appeng/me/NetworkEventBus.java

@@ -29,6 +29,7 @@ import java.util.Map;
 import java.util.Map.Entry;
 
 import appeng.api.networking.IGridNode;
+import appeng.api.networking.IMachineSet;
 import appeng.api.networking.events.MENetworkEvent;
 import appeng.api.networking.events.MENetworkEventSubscribe;
 import appeng.core.AELog;
@@ -114,10 +115,19 @@ public class NetworkEventBus
 						target.invoke( cache.getCache(), e );
 					}
 
-					for( final IGridNode obj : g.getMachines( subscriber.getKey() ) )
+					// events may create or remove grid nodes in rare cases
+					final IMachineSet machines = g.getMachines( subscriber.getKey() );
+					final List<IGridNode> work = new ArrayList<>( machines.size() );
+					machines.forEach( work::add );
+
+					for( final IGridNode obj : work )
 					{
-						x++;
-						target.invoke( obj.getMachine(), e );
+						// stil part of grid?
+						if( machines.contains( obj ) )
+						{
+							x++;
+							target.invoke( obj.getMachine(), e );
+						}
 					}
 				}
 			}

src/main/java/appeng/me/cluster/implementations/CraftingCPUCluster.java

@@ -19,6 +19,7 @@
 package appeng.me.cluster.implementations;
 
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -227,7 +228,7 @@ public final class CraftingCPUCluster implements IAECluster, ICraftingCPU
 	{
 		if( input instanceof IAEItemStack )
 		{
-			final IAEItemStack is = this.waitingFor.findPrecise( (IAEItemStack) input );
+			final IAEItemStack is = this.waitingFor.findPrecise( input );
 			if( is != null && is.getStackSize() > 0 )
 			{
 				return true;
@@ -243,7 +244,7 @@ public final class CraftingCPUCluster implements IAECluster, ICraftingCPU
 			return input;
 		}
 
-		final IAEItemStack what = (IAEItemStack) input.copy();
+		final IAEItemStack what = input.copy();
 		final IAEItemStack is = this.waitingFor.findPrecise( what );
 
 		if( type == Actionable.SIMULATE )// causes crafting to lock up?
@@ -684,7 +685,25 @@ public final class CraftingCPUCluster implements IAECluster, ICraftingCPU
 
 									if( details.isCraftable() )
 									{
-										for( IAEItemStack fuzz : this.inventory.getItemList().findFuzzy( input[x], FuzzyMode.IGNORE_ALL ) )
+										final Collection<IAEItemStack> itemList;
+
+										if( details.canSubstitute() )
+										{
+											itemList = this.inventory.getItemList().findFuzzy( input[x], FuzzyMode.IGNORE_ALL );
+										}
+										else
+										{
+											itemList = new ArrayList<>( 1 );
+
+											final IAEItemStack item = this.inventory.getItemList().findPrecise( input[x] );
+
+											if( item != null )
+											{
+												itemList.add( item );
+											}
+										}
+
+										for( IAEItemStack fuzz : itemList )
 										{
 											fuzz = fuzz.copy();
 											fuzz.setStackSize( input[x].getStackSize() );

src/main/java/appeng/me/storage/CellInventory.java

@@ -218,11 +218,12 @@ public class CellInventory implements ICellInventory
 			return input;
 		}
 
-		final ItemStack sharedItemStack = input.createItemStack();
-
-		if( CellInventory.isStorageCell( sharedItemStack ) )
+		// This is slightly hacky as it expects a read-only access, but fine for now.
+		// TODO: Guarantee a read-only access. E.g. provide an isEmpty() method and ensure CellInventory does not write
+		// any NBT data for empty cells instead of relying on an empty IItemContainer
+		if( CellInventory.isStorageCell( input.getDefinition() ) )
 		{
-			final IMEInventory meInventory = getCell( sharedItemStack, null );
+			final IMEInventory meInventory = getCell( input.createItemStack(), null );
 			if( meInventory != null && !this.isEmpty( meInventory ) )
 			{
 				return input;
@@ -232,20 +233,20 @@ public class CellInventory implements ICellInventory
 		final IAEItemStack l = this.getCellItems().findPrecise( input );
 		if( l != null )
 		{
-			final long remainingItemSlots = this.getRemainingItemCount();
-			if( remainingItemSlots < 0 )
+			final long remainingItemCount = this.getRemainingItemCount();
+			if( remainingItemCount < 0 )
 			{
 				return input;
 			}
 
-			if( input.getStackSize() > remainingItemSlots )
+			if( input.getStackSize() > remainingItemCount )
 			{
 				final IAEItemStack r = input.copy();
-				r.setStackSize( r.getStackSize() - remainingItemSlots );
+				r.setStackSize( r.getStackSize() - remainingItemCount );
 				if( mode == Actionable.MODULATE )
 				{
-					l.setStackSize( l.getStackSize() + remainingItemSlots );
-					this.updateItemCount( remainingItemSlots );
+					l.setStackSize( l.getStackSize() + remainingItemCount );
+					this.updateItemCount( remainingItemCount );
 					this.saveChanges();
 				}
 				return r;
@@ -269,19 +270,19 @@ public class CellInventory implements ICellInventory
 			{
 				if( input.getStackSize() > remainingItemCount )
 				{
-					final ItemStack toReturn = sharedItemStack.copy();
-					toReturn.setCount( sharedItemStack.getCount() - remainingItemCount );
+					final IAEItemStack toReturn = input.copy();
+					toReturn.setStackSize( input.getStackSize() - remainingItemCount );
 					if( mode == Actionable.MODULATE )
 					{
-						final ItemStack toWrite = sharedItemStack.copy();
-						toWrite.setCount( remainingItemCount );
+						final IAEItemStack toWrite = input.copy();
+						toWrite.setStackSize( remainingItemCount );
 
-						this.cellItems.add( AEItemStack.fromItemStack( toWrite ) );
-						this.updateItemCount( toWrite.getCount() );
+						this.cellItems.add( toWrite );
+						this.updateItemCount( toWrite.getStackSize() );
 
 						this.saveChanges();
 					}
-					return AEItemStack.fromItemStack( toReturn );
+					return toReturn;
 				}
 
 				if( mode == Actionable.MODULATE )

src/main/java/appeng/tile/AEBaseTile.java

@@ -488,7 +488,7 @@ public class AEBaseTile extends TileEntity implements IOrientable, ICommonTile,
 
 	public void saveChanges()
 	{
-		super.markDirty();
+		markDirty();
 	}
 
 	public boolean requiresTESR()

src/main/java/appeng/worldgen/QuartzWorldGen.java

@@ -21,7 +21,6 @@ package appeng.worldgen;
 
 import java.util.Random;
 
-import net.minecraft.block.Block;
 import net.minecraft.util.math.BlockPos;
 import net.minecraft.world.World;
 import net.minecraft.world.chunk.IChunkProvider;
@@ -48,16 +47,22 @@ public final class QuartzWorldGen implements IWorldGenerator
 		final IBlockDefinition oreDefinition = blocks.quartzOre();
 		final IBlockDefinition chargedDefinition = blocks.quartzOreCharged();
 
-		final Block ore = oreDefinition.maybeBlock().orElse( null );
-		final Block charged = chargedDefinition.maybeBlock().orElse( null );
-
-		this.oreNormal = new WorldGenMinable( ore.getDefaultState(), AEConfig.instance().getQuartzOresPerCluster() );
-		this.oreCharged = new WorldGenMinable( charged.getDefaultState(), AEConfig.instance().getQuartzOresPerCluster() );
+		this.oreNormal = oreDefinition.maybeBlock()
+				.map( b -> new WorldGenMinable( b.getDefaultState(), AEConfig.instance().getQuartzOresPerCluster() ) )
+				.orElse( null );
+		this.oreCharged = chargedDefinition.maybeBlock()
+				.map( b -> new WorldGenMinable( b.getDefaultState(), AEConfig.instance().getQuartzOresPerCluster() ) )
+				.orElse( null );
 	}
 
 	@Override
 	public void generate( final Random r, final int chunkX, final int chunkZ, final World w, final IChunkGenerator chunkGenerator, final IChunkProvider chunkProvider )
 	{
+		if( this.oreNormal == null && this.oreCharged == null )
+		{
+			return;
+		}
+
 		int seaLevel = w.provider.getAverageGroundLevel() + 1;
 
 		if( seaLevel < 20 )
@@ -67,26 +72,31 @@ public final class QuartzWorldGen implements IWorldGenerator
 			seaLevel = w.getHeight( x, z );
 		}
 
-		if( this.oreNormal == null || this.oreCharged == null )
-		{
-			return;
-		}
-
 		final double oreDepthMultiplier = AEConfig.instance().getQuartzOresClusterAmount() * seaLevel / 64;
 		final int scale = (int) Math.round( r.nextGaussian() * Math.sqrt( oreDepthMultiplier ) + oreDepthMultiplier );
 
-		for( int x = 0; x < ( r.nextBoolean() ? scale * 2 : scale ) / 2; ++x )
+		for( int cnt = 0; cnt < ( r.nextBoolean() ? scale * 2 : scale ) / 2; ++cnt )
 		{
-			final boolean isCharged = r.nextFloat() > AEConfig.instance().getSpawnChargedChance();
-			final WorldGenMinable whichOre = isCharged ? this.oreCharged : this.oreNormal;
+			boolean isCharged = false;
 
-			if( WorldGenRegistry.INSTANCE.isWorldGenEnabled( isCharged ? WorldGenType.CHARGED_CERTUS_QUARTZ : WorldGenType.CERTUS_QUARTZ, w ) )
+			if( this.oreCharged != null )
 			{
-				final int cx = chunkX * 16 + r.nextInt( 22 );
+				isCharged = r.nextFloat() > AEConfig.instance().getSpawnChargedChance();
+			}
+
+			final WorldGenMinable whichOre = isCharged ? this.oreCharged : this.oreNormal;
+			if( whichOre != null && shouldGenerate( isCharged, w ) )
+			{
+				final int cx = chunkX * 16 + r.nextInt( 16 );
 				final int cy = r.nextInt( 40 * seaLevel / 64 ) + r.nextInt( 22 * seaLevel / 64 ) + 12 * seaLevel / 64;
-				final int cz = chunkZ * 16 + r.nextInt( 22 );
+				final int cz = chunkZ * 16 + r.nextInt( 16 );
 				whichOre.generate( w, r, new BlockPos( cx, cy, cz ) );
 			}
 		}
 	}
+
+	private static boolean shouldGenerate( final boolean isCharged, final World w )
+	{
+		return WorldGenRegistry.INSTANCE.isWorldGenEnabled( isCharged ? WorldGenType.CHARGED_CERTUS_QUARTZ : WorldGenType.CERTUS_QUARTZ, w );
+	}
 }