Works fine in single player, but in multiplayer the dust always renders an incorrect colour

@pahimar How do you get the minium dust in the Calcinator? I want to try some solutions to fix it but I don't know how to make it

Smelt diamonds of course :)

@pahimar Noticed a thing:
If you disconnect from the server, then you get back in, the dust is rendered correctly.
Then if you open the calcinator GUI the dust is rendered wrongly again. By relogging the issue is fixed again.

@pahimar Actually, the dust starts rendering wrongly when you close the GUI; I've noticed that after closing it the dust turned from red to blue.

@pahimar And when the GUI is open, it flickers fast between red and blue render.

@chris54721 @pahimar Is the dust and the GUI a separate render?

@TheBaloneyboy I think yes; basically it renders the dust correctly whilst you have the GUI open (it flickers a bit between the correct and the wrong one). So I think that the problem should be in TileCalcinator.updateEntity()

What you are seeing is a mismatch of data between client and server (when you open a GUI, it opens a Container and synchs data between client and server)

Because the dust renders correctly when not using the inventory we know that the data send by the descriptivePacket is correct and all the method used in that packet (get...size/color). Opening the GUI will sync information between client and server, this is where the problem lies.

Looks like the packet used to send the data is a Packet54PlayNoteBlock with the color value as the "pitch" which gets written out with DataOutput.write() which, per the comment on the interface "writes to the output stream the eight low-order bits of the argument" ignoring the 24 high-order bits.

Hence ending up with 0xA0 instead of 0xA0A0A0, etc.

@pcrov I can't find any reference of Packet54PlayNoteBlock...It seems like it's sending a packet of type PacketTileCalcinator.

@chris54721 Packet54PlayNoteBlock is what's used by WorldServer.sendAndApplyBlockEvents(), so when TileCalcinator.sendDustPileData() runs those addBlockEvent()s, that's apparently what's going out.

@pcrov @pahimar The sent blockEvent data is correct, but when receiveClientEvent() is called, a "69" pops up randomly between the correct values (whilst the GUI is open). Maybe the event ID 3 is already used somewhere else?

So, the server prints the correct values, but the client prints wrong values if I log the output for receiveClientEvent().
If the gui is open, on the client I get the correct value and this:
R = 0, G = 0, B = correct blue
so as already said, it's dropping red and green and keeping blue. Probably @pcrov is right, so I suppose that the eight low-order bits of eventData represent the blue colour, the others are green and red.

The only thing I can't understand is why when the GUI is open it renders correctly...

And the proper cause reveals itself:
https://twitter.com/minecraftcpw/status/422499854024069120

Is the bug in ItemCalcinator java file?

@TheBaloneyboy TileEntityCalcinatorRenderer.java

@pahimar Maybe a malformed TE packet?

@pahimar I think the float array in ColourUtils.java should have a 4th value with the alpha of the int colour.
Something like this:

colourByteArray[0] = ((intColour) & 0xFF) / 255F; //red
colourByteArray[1] = ((intColour >> 8) & 0xFF) / 255F; //green
colourByteArray[2] = ((intColour >> 16) & 0xFF) / 255F; //blue
colourByteArray[3] = ((intColour >> 24) & 0xFF) / 255F; //alpha

(I don't think that this will fix but should avoid any further problems)

@TheBaloneyboy its neither the Item, nor is it the TE packet (TE packets are only sent when you first come into range of the TE)

@chris54721 wouldn't matter; in the TESR for the Calcinator I go with full alpha on the blended colour. It seems like it's dropping the R and G channels.

@pahimar Maybe add some redundancy to the R and G channels? I haven't done much with renderings.

If possible could you post debug data, so the values of the fields R,B, and G immediately before render, along with what they are supposed to be?

The only thing I can think of is some problem with endianness, where you have ABGR in an int, but it's being loaded as GRAB, so your B value is actually the R value, G is being discarded, A is probably 0 initially so would give no G. Wouldn't explain the lack of Red though...

I had had some debug related stuff, and I was getting R: 0, G: 0, B: 1F (so 255)

Interesting idea; I'll pursue it further. What's odd is its solely a MP related issue. Works 100% as expected in SP.

@pahimar What are the ARGB expected values?

Let's use Minium as the example, which has a colour of #FF4545

R: FF = 255 / 255 = 1.0
G: 45 = 69 / 255 = 0.270588235
B: 45 = 69 / 255 = 0.270588235

What I was getting in debug was:

R: 0.0
G: 0.0
B: 1.0

@pahimar this might be unrelated, but the method Item.getColorFromItemStack(ItemStack, int) is client side only, but you seem to be calling it on the server side and sending the value from it to the client. This seems unnecessary and maybe is what is causing the problem.
I'm surprised it haven't caused crashes for you when running on a server actually.

@ganymedes01 It did, hence me changing how the colour is sent to the client (d81d559)

Could it be a combination of an endianness issue giving B the value R should have and an integer division problem giving 69/255 = 0 for the others? Might the intColor && 0xFF part need to be cast to float before the division?

@pahimar Oh right, I was behind on your commits then.

But it still seems like you are sending the colour to the client using block updates and packets.
Wouldn't it be easier to just send the item id and metadata, and on the client side you use that to get the stack colour?
I think the way it is now will still cause crashes under certain conditions.
Try reloading a world with a calcinator that has a pile of ash in it. It should crash because it will try to get the colour on the server side.

@unpairedbracket Thought of the same thing, and could be the problem as I already had some issues with int/float conversions like this...I'll try some solutions on my EE3 fork and if I fix I'll submit a PR.

eac7f32. Colours are still off, but at least its sending the proper sized data. Will troubleshoot more in the morning.

Bytes in Java are always signed, so now you're trying to send a value (255 for red channel in minium) that the byte can't handle; by using & 0xFF in convertIntColourToByteArray() you change that to 255, but it is loaded as -1, because a byte narrowing conversion of 255 is -1. So you should do something like this in rendering: if the byte represents a negative integer, add 256 to that integer.

Quote @pahimar "@TheBaloneyboy its neither the Item, nor is it the TE packet (TE packets are only sent when you first come into range of the TE)":
You can actually send it like any other packet (PacketDispatcher.sendPacketToAllPlayers(getDescriptionPacket()););

@mak326428 It's not a packet issue anymore (it's fixed with eac7f32), the new problem is explained in my comment before yours.

@mak326428 Yes, that's always been true, but sending the full TE packet every update sends unnecessary info. Doing it this way syncs only the data we need (reducing the amount of network overhead)

@chris54721 I wrote that up quick last night and went to bed. Woke up this morning and realized that I could simplify it further by sending only the metadata for the dust in the slot, and have the TESR blend the colours (leaving all "rendering" related stuff on the client side which is optimal).

I'll be committing a change to this effect shortly

@pahimar I also fixed by adding a few lines in TileEntityCalcinatorRenderer:

float[] dustColour = new float[]{(tileCalcinator.dustRedChannel / 255F), (tileCalcinator.dustGreenChannel / 255F), (tileCalcinator.dustBlueChannel / 255F), 1F};

// new code
for(int dc=0;dc<dustColour.length-1;dc++) {
if(dustColour[dc] < 0) {
dustColour[dc] += 1F;
}
}
// new code end
GL11.glColor4f(dustColour[0], dustColour[1], dustColour[2], 1F);

@chris54721 the fact that java uses bytes as signed doesn't mean you can't use them as unsigned. Adding a 1 to it will give a wrong value. You simply have to & FF when using it as an unsigned number:

float[] dustColour = new float[]{(tileCalcinator.dustRedChannel / 255F), (tileCalcinator.dustGreenChannel / 255F), (tileCalcinator.dustBlueChannel / 255F), 1F};

should be:

float[] dustColour = new float[]{(int)(tileCalcinator.dustRedChannel & 0xFF) / 255F, (int)(tileCalcinator.dustGreenChannel & 0xFF) / 255F, (int)(tileCalcinator.dustBlueChannel & 0xFF) / 255F, 1F};

@psxlover 0xFF is already added in ColourUtils to the dust colour channels, and it works, but as said, a byte narrowing conversion of 255 is -1, and as the byte represents a colour channel (red in this case), when rendering Java loads -1 as the red amount in the colour, so it basically drops the red channel if you don't add the code in rendering (pull #577)

The way it is displayed has nothing to do with the way it is stored. Whether or not it is treated as signed when you print it the internal number is the same. You just have to make sure java treats it as unsigned when converting it to something that can store more like a short or int.

Should be fixed with 71b9d9a

Changed what data is being sent to the client (so that all colour calculations are handled client side), and changed up how the blended colour is calculated (you should notice a much more gradual change in colour now)