GregTechCE / GregTech

GregTech rewrite for modern versions of Minecraft
GNU Lesser General Public License v3.0
271 stars 150 forks source link

[BUG] Fluid Cells having a color provider when not needed. #1560

Open idcppl opened 3 years ago

idcppl commented 3 years ago

Describe the bug Cells base layer is being provided a color through metaitem, so it makes the edges of the item a different color.
https://github.com/GregTechCE/GregTech/blob/master/src/main/java/gregtech/api/items/metaitem/MetaItem.java#L162

Versions Forge: 14.23.5.2847 GTCE: Latest

Expected behavior For the edges not to be discolored.

ALongStringOfNumbers commented 3 years ago

As an example image of the issue described in the opening statement, here are some pictures 2021-06-09_18-42-09 2021-06-09_18-43-27

As you can see, the right edge of the cell closest to the player takes on the color of the fluid inside the cell

warjort commented 3 years ago

The issue here as the original bug report points out is that GTCE is automatically creating color providers for any item that has the FluidHandler capability. This handler is for tint index=0 which is effectively the backside of the item for the fluid cell. This only becomes visible when displayed in the player's hand in 3D

The cells use the generic Forge "bucket" model, which automatically applies the fluid color to tint index 1 (the overlay part of the model).

So this default behaviour is both unnecessary and wrong for the fluid cell.

Something like the following fixes the problem, but I don't know if it would be considered too much of a hack?

diff --git a/src/main/java/gregtech/api/items/metaitem/MetaItem.java b/src/main/java/gregtech/api/items/metaitem/MetaItem.java
index dac758f3..b6a71136 100644
--- a/src/main/java/gregtech/api/items/metaitem/MetaItem.java
+++ b/src/main/java/gregtech/api/items/metaitem/MetaItem.java
@@ -770,6 +770,11 @@ public abstract class MetaItem<T extends MetaItem<?>.MetaValueItem> extends Item
             return colorProvider;
         }

+        public MetaValueItem disableFluidColorProvider() {
+            this.colorProvider = (stack, tint) -> 0xFFFFFF;
+            return this;
+        }
+
         @Nullable
         public IItemNameProvider getNameProvider() {
             return nameProvider;
diff --git a/src/main/java/gregtech/common/items/MetaItem1.java b/src/main/java/gregtech/common/items/MetaItem1.java
index bb683e7e..2c093d5d 100644
--- a/src/main/java/gregtech/common/items/MetaItem1.java
+++ b/src/main/java/gregtech/common/items/MetaItem1.java
@@ -285,7 +285,7 @@ public class MetaItem1 extends MaterialMetaItem {
         COVER_SOLAR_PANEL_ULV = addItem(751, "cover.solar.panel.ulv");
         COVER_SOLAR_PANEL_LV = addItem(752, "cover.solar.panel.lv");

-        FLUID_CELL = addItem(762, "fluid_cell").addComponents(new FluidStats(1000, Integer.MIN_VALUE, Integer.MAX_VALUE, false));
+        FLUID_CELL = addItem(762, "fluid_cell").addComponents(new FluidStats(1000, Integer.MIN_VALUE, Integer.MAX_VALUE, false)).disableFluidColorProvider();
         INTEGRATED_CIRCUIT = addItem(766, "circuit.integrated").addComponents(new IntCircuitBehaviour());
         FOAM_SPRAYER = addItem(746, "foam_sprayer").addComponents(new FoamSprayerBehavior());
     }

This gives the fluid cells a color handler that effectively turns off the coloring ... 2021-06-10_12 27 07

idcppl commented 3 years ago

It was added without any reasoning here, and I see no use case for it to be changing the color of that layer. Since all MetaItem's are using the forge bucket model. Unless Arch wants to come and enlighten us on why he decided to do this. In one of my addons, I just ended up wrapping MetaItem and overriding that.