Rework fuzzy comparison for 99 percent threshold (#3367)

* Rework fuzzy comparison for 99 percent threshold

The current implementation is quite convoluted and can handle 99%
incorrectly when items exceed a certain damage range.

* Use metadata explicitly for comparison

Some item could use metadata to pack damageable and undamageable items
into the same id. Thus could lead to get ItemStack#getItemDamage() to
return bogus values for undamageable items.

src/main/java/appeng/client/render/FacadeItemModel.java

@@ -43,22 +43,16 @@ public class FacadeItemModel implements IModel
 	// We use this to get the default item transforms and make our lives easier
 	private static final ResourceLocation MODEL_BASE = new ResourceLocation( AppEng.MOD_ID, "item/facade_base" );
 
-	private IModel baseModel = null;
-
 	private IModel getBaseModel()
 	{
-		if( this.baseModel == null )
+		try
 		{
-			try
-			{
-				baseModel = ModelLoaderRegistry.getModel( MODEL_BASE );
-			}
-			catch( Exception e )
-			{
-				throw new RuntimeException( e );
-			}
+			return ModelLoaderRegistry.getModel( MODEL_BASE );
+		}
+		catch( Exception e )
+		{
+			throw new RuntimeException( e );
 		}
-		return this.baseModel;
 	}
 
 	@Override
@@ -84,6 +78,6 @@ public class FacadeItemModel implements IModel
 	@Override
 	public IModelState getDefaultState()
 	{
-		return getBaseModel().getDefaultState();
+		return this.getBaseModel().getDefaultState();
 	}
 }

src/main/java/appeng/util/helpers/ItemComparisonHelper.java

@@ -21,7 +21,6 @@ package appeng.util.helpers;
 
 import javax.annotation.Nonnull;
 
-import net.minecraft.init.Items;
 import net.minecraft.item.Item;
 import net.minecraft.item.ItemStack;
 import net.minecraft.nbt.NBTBase;
@@ -95,55 +94,23 @@ public class ItemComparisonHelper
 			return false;
 		}
 
-		/*
-		 * if ( a.itemID != 0 && b.itemID != 0 && a.isItemStackDamageable() && ! a.getHasSubtypes() && a.itemID ==
-		 * b.itemID ) { return (a.getItemDamage() > 0) == (b.getItemDamage() > 0); }
-		 */
-
 		// test damageable items..
-		if( a.getItem() != Items.AIR && b.getItem() != Items.AIR && a.getItem().isDamageable() && a.getItem() == b.getItem() )
+		if( a.getItem() == b.getItem() && a.getItem().isDamageable() )
 		{
-			try
+			if( mode == FuzzyMode.IGNORE_ALL )
 			{
-				if( mode == FuzzyMode.IGNORE_ALL )
-				{
-					return true;
-				}
-				else if( mode == FuzzyMode.PERCENT_99 )
-				{
-					final Item ai = a.getItem();
-					final Item bi = b.getItem();
-
-					return ( ai.getDurabilityForDisplay( a ) > 1 ) == ( bi.getDurabilityForDisplay( b ) > 1 );
-				}
-				else
-				{
-					final Item ai = a.getItem();
-					final Item bi = b.getItem();
-
-					final float percentDamagedOfA = 1.0f - (float) ai.getDurabilityForDisplay( a );
-					final float percentDamagedOfB = 1.0f - (float) bi.getDurabilityForDisplay( b );
-
-					return ( percentDamagedOfA > mode.breakPoint ) == ( percentDamagedOfB > mode.breakPoint );
-				}
+				return true;
 			}
-			catch( final Throwable e )
+			else if( mode == FuzzyMode.PERCENT_99 )
 			{
-				if( mode == FuzzyMode.IGNORE_ALL )
-				{
-					return true;
-				}
-				else if( mode == FuzzyMode.PERCENT_99 )
-				{
-					return ( a.getItemDamage() > 1 ) == ( b.getItemDamage() > 1 );
-				}
-				else
-				{
-					final float percentDamagedOfA = (float) a.getItemDamage() / (float) a.getMaxDamage();
-					final float percentDamagedOfB = (float) b.getItemDamage() / (float) b.getMaxDamage();
+				return ( a.getItemDamage() > 1 ) == ( b.getItemDamage() > 1 );
+			}
+			else
+			{
+				final float percentDamagedOfA = (float) a.getItemDamage() / (float) a.getMaxDamage();
+				final float percentDamagedOfB = (float) b.getItemDamage() / (float) b.getMaxDamage();
 
-					return ( percentDamagedOfA > mode.breakPoint ) == ( percentDamagedOfB > mode.breakPoint );
-				}
+				return ( percentDamagedOfA > mode.breakPoint ) == ( percentDamagedOfB > mode.breakPoint );
 			}
 		}
 
@@ -155,17 +122,6 @@ public class ItemComparisonHelper
 			return true;
 		}
 
-		/*
-		 * // test ore dictionary.. int OreID = getOreID( a ); if ( OreID != -1 ) return OreID == getOreID( b );
-		 * if ( Mode != FuzzyMode.IGNORE_ALL ) { if ( a.hasTagCompound() && !isShared( a.getTagCompound() ) ) { a =
-		 * Platform.getSharedItemStack( AEItemStack.create( a ) ); }
-		 * if ( b.hasTagCompound() && !isShared( b.getTagCompound() ) ) { b = Platform.getSharedItemStack(
-		 * AEItemStack.create( b ) ); }
-		 * // test regular items with damage values and what not... if ( isShared( a.getTagCompound() ) && isShared(
-		 * b.getTagCompound() ) && a.itemID == b.itemID ) { return ((AppEngSharedNBTTagCompound)
-		 * a.getTagCompound()).compareFuzzyWithRegistry( (AppEngSharedNBTTagCompound) b.getTagCompound() ); } }
-		 */
-
 		return a.isItemEqual( b );
 	}
 

src/main/java/appeng/util/item/AEItemStack.java

@@ -167,126 +167,30 @@ public final class AEItemStack extends AEStack<IAEItemStack> implements IAEItemS
 	{
 		if( st instanceof IAEItemStack )
 		{
-			final IAEItemStack o = (IAEItemStack) st;
+			final IAEItemStack other = (IAEItemStack) st;
 
-			if( this.sameOre( o ) )
+			if( OreHelper.INSTANCE.sameOre( this, other ) )
 			{
 				return true;
 			}
 
-			if( o.getItem() == this.getItem() )
-			{
-				if( this.getDefinition().getItem().isDamageable() )
-				{
-					final ItemStack a = this.getDefinition();
-					final ItemStack b = o.getDefinition();
+			final ItemStack itemStack = this.getDefinition();
+			final ItemStack otherStack = other.getDefinition();
 
-					try
-					{
-						if( mode == FuzzyMode.IGNORE_ALL )
-						{
-							return true;
-						}
-						else if( mode == FuzzyMode.PERCENT_99 )
-						{
-							final Item ai = a.getItem();
-							final Item bi = b.getItem();
-
-							return ( ai.getDurabilityForDisplay( a ) < 0.001f ) == ( bi.getDurabilityForDisplay( b ) < 0.001f );
-						}
-						else
-						{
-							final Item ai = a.getItem();
-							final Item bi = b.getItem();
-
-							final float percentDamageOfA = 1.0f - (float) ai.getDurabilityForDisplay( a );
-							final float percentDamageOfB = 1.0f - (float) bi.getDurabilityForDisplay( b );
-
-							return ( percentDamageOfA > mode.breakPoint ) == ( percentDamageOfB > mode.breakPoint );
-						}
-					}
-					catch( final Throwable e )
-					{
-						if( mode == FuzzyMode.IGNORE_ALL )
-						{
-							return true;
-						}
-						else if( mode == FuzzyMode.PERCENT_99 )
-						{
-							return ( a.getItemDamage() > 1 ) == ( b.getItemDamage() > 1 );
-						}
-						else
-						{
-							final float percentDamageOfA = (float) a.getItemDamage() / (float) a.getMaxDamage();
-							final float percentDamageOfB = (float) b.getItemDamage() / (float) b.getMaxDamage();
-
-							return ( percentDamageOfA > mode.breakPoint ) == ( percentDamageOfB > mode.breakPoint );
-						}
-					}
-				}
-
-				return this.getItemDamage() == o.getItemDamage();
-			}
+			return this.fuzzyItemStackComparison( itemStack, otherStack, mode );
 		}
 
-		if( st instanceof ItemStack )
+		if( st instanceof IAEItemStack )
 		{
-			final ItemStack o = (ItemStack) st;
+			final ItemStack otherStack = (ItemStack) st;
 
-			OreHelper.INSTANCE.sameOre( this, o );
-
-			if( o.getItem() == this.getItem() )
+			if( OreHelper.INSTANCE.sameOre( this, otherStack ) )
 			{
-				if( this.getDefinition().getItem().isDamageable() )
-				{
-					final ItemStack a = this.getDefinition();
-
-					try
-					{
-						if( mode == FuzzyMode.IGNORE_ALL )
-						{
-							return true;
-						}
-						else if( mode == FuzzyMode.PERCENT_99 )
-						{
-							final Item ai = a.getItem();
-							final Item bi = o.getItem();
-
-							return ( ai.getDurabilityForDisplay( a ) < 0.001f ) == ( bi.getDurabilityForDisplay( o ) < 0.001f );
-						}
-						else
-						{
-							final Item ai = a.getItem();
-							final Item bi = o.getItem();
-
-							final float percentDamageOfA = 1.0f - (float) ai.getDurabilityForDisplay( a );
-							final float percentDamageOfB = 1.0f - (float) bi.getDurabilityForDisplay( o );
-
-							return ( percentDamageOfA > mode.breakPoint ) == ( percentDamageOfB > mode.breakPoint );
-						}
-					}
-					catch( final Throwable e )
-					{
-						if( mode == FuzzyMode.IGNORE_ALL )
-						{
-							return true;
-						}
-						else if( mode == FuzzyMode.PERCENT_99 )
-						{
-							return ( a.getItemDamage() > 1 ) == ( o.getItemDamage() > 1 );
-						}
-						else
-						{
-							final float percentDamageOfA = (float) a.getItemDamage() / (float) a.getMaxDamage();
-							final float percentDamageOfB = (float) o.getItemDamage() / (float) o.getMaxDamage();
-
-							return ( percentDamageOfA > mode.breakPoint ) == ( percentDamageOfB > mode.breakPoint );
-						}
-					}
-				}
-
-				return this.getItemDamage() == o.getItemDamage();
+				return true;
 			}
+
+			final ItemStack itemStack = this.getDefinition();
+			return this.fuzzyItemStackComparison( itemStack, otherStack, mode );
 		}
 
 		return false;
@@ -457,4 +361,33 @@ public final class AEItemStack extends AEStack<IAEItemStack> implements IAEItemS
 		return this.sharedStack;
 	}
 
+	private boolean fuzzyItemStackComparison( ItemStack a, ItemStack b, FuzzyMode mode )
+	{
+		if( a.getItem() == b.getItem() )
+		{
+			if( a.getItem().isDamageable() )
+			{
+				if( mode == FuzzyMode.IGNORE_ALL )
+				{
+					return true;
+				}
+				else if( mode == FuzzyMode.PERCENT_99 )
+				{
+					return ( a.getItemDamage() > 1 ) == ( b.getItemDamage() > 1 );
+				}
+				else
+				{
+					final float percentDamageOfA = (float) a.getItemDamage() / (float) a.getMaxDamage();
+					final float percentDamageOfB = (float) b.getItemDamage() / (float) b.getMaxDamage();
+
+					return ( percentDamageOfA > mode.breakPoint ) == ( percentDamageOfB > mode.breakPoint );
+				}
+			}
+
+			return a.getMetadata() == b.getMetadata();
+		}
+
+		return false;
+	}
+
 }