Add dynamic drum colors#164
Conversation
| "south": {"uv": [0, 5, 2.5, 6], "texture": "#0", "tintindex": 0}, | ||
| "west": {"uv": [2.5, 5, 5, 6], "texture": "#0", "tintindex": 0}, | ||
| "up": {"uv": [7.5, 7.5, 5, 5], "texture": "#0"}, | ||
| "down": {"uv": [2.5, 6, 0, 8.5], "texture": "#0"} |
There was a problem hiding this comment.
Why do we have entries for the top & bottom faces for the middle parts of the drum? Those shouldn't need to be rendered, and thus should be removed, I think.
| GL11.glGetTexImage(GL11.GL_TEXTURE_2D, 0, GL12.GL_BGRA, GL12.GL_UNSIGNED_INT_8_8_8_8_REV, buf); | ||
|
|
||
| int[] pixels = new int[width * height]; | ||
| buf.get(pixels); |
There was a problem hiding this comment.
Why allocate an array to hold a copy of the buffer instead of getting the pixels directly from the IntBuffer?
| @Override | ||
| public Packet getDescriptionPacket() { | ||
| NBTTagCompound tag = new NBTTagCompound(); | ||
| writeToNBT(tag); | ||
| return new S35PacketUpdateTileEntity(xCoord, yCoord, zCoord, 2, tag); | ||
| } |
There was a problem hiding this comment.
Why send over all the data, and not just the fluid ID?
There was a problem hiding this comment.
Because colorMultiplier in BlockDrum accesses the tank. To send over just the fluid id I would have to do a bunch of careful manual overwriting of the state of the tank on the client, which sometimes contains a serialized FluidStack and sometimes does not.
This block has very little NBT data and this packet is fired only on update, I think this is harmless.
| : tank.getFluid() | ||
| .getFluid(); | ||
| if (before != after) { | ||
| worldObj.markBlockRangeForRenderUpdate(xCoord, yCoord, zCoord, xCoord, yCoord, zCoord); |
There was a problem hiding this comment.
worldObj.markBlockForUpdate does the same thing on the client, and it's simpler.
| } | ||
|
|
||
| private void renderUpdate() { | ||
| if (worldObj != null && !worldObj.isRemote) { |
There was a problem hiding this comment.
This doesn't need to be gated behind worldObj.isRemote - if it is called on the client, it will just call markBlockRangeForRenderUpdate
| public void setFluid(FluidStack stack) { | ||
| this.tank.setFluid(stack); | ||
| markDirty(); | ||
| renderUpdate(); |
There was a problem hiding this comment.
No check to see if renderUpdate is necessary?
There was a problem hiding this comment.
The only time this is called is when it's placed. I supposed someone could technically use this method for something else that might benefit from a check, but it's so unlikely to be used frequently. I guess I can add a check though
|
|
||
| for (Fluid fluid : FluidRegistry.getRegisteredFluids() | ||
| .values()) { | ||
| if (colorMap.containsKey(fluid)) continue; |
There was a problem hiding this comment.
This will cause issues. If I load a texture pack which changes the color of a fluid, then this check will skip updating its color and use the color it calculated on game load.
There was a problem hiding this comment.
It's really just for water and lava. I guess a better approach would be to just add them after the fluidregistry loop.
There was a problem hiding this comment.
You never clear the colorMap. The texture build step can happen multiple times, especially if I reload texture packs or something like that. After the first time this method runs, it will just skip all the fluids.
There was a problem hiding this comment.
Yeah, that makes sense. I was thinking of it as a once per load thing.
2 participants
-Split the model's middle block into 3 separate cubes and made two of them have face tinting
-FluidColorCache builds a mapping between FluidRegistry fluids and colors by pulling fluid icons off the block texture atlas during TextureStitchEvent.Post and averaging their colors by pixel. If it gets a "useful" value from getColor (not 0xFFFFFF) it will just add that and skip all of the texture averaging for that fluid.
-When the drum is placed, emptied, or filled after being empty, it sends a packet to the client so the render can be refreshed.