[11:52:53 INFO]: [Chunkli] running on Java 1.7.0_80 amd64

> perhaps combined with some logic when chunks are saved to strip empty chunk sections (if this doesn't cause more lighting issues).

I actually wrote that over the last few days to see how much space i get back from the *.mca Files if i do this. In the End it was about 15% size-decrease on a heavily used server (150.000+ Chunk-Plots). I´d expect much less on 'normal' servers. These 15% are mostly due to once created but in the end unused spaces in the *.mca-DataStructure. Only a very small percentage was caused by EmptyChunk-Regions. But still - 15% less *.mca Files is 15% less Data-Cache on the Server Memory/SSD which is nice.

On the other hand the maxima here is quite interesting. If you populate a ChunkRegion (32x32Chunks) with random Data your *.mca File will grow to 50+MB and never decrease in size. Optimizing this decreases the size to ~4 MB again. So if you tested alot of stuff with WorldEdit etc. this actually helps a lot.

Parsign all that Data an loading or saving chunks would be overkill imo.
I added it as command to a Plugin of mine "/chunk optimize" -> Optimize all *.mca in the current selection. Using it two times a year should be sufficient.

Thinkofdeath

I avoid clientside stuff, so I'll take your word for it.

Checked up on the minecraft generator, it only generates chunk sections for sections with non-air blocks. Some comments I read on mojang's bug tracker led me to believe it allocated them all. After some testing it appears the lighting engine causes these sections to be allocated (e.g. placing a torch in an adjacent chunk section).

Not all spigot servers are survival or use mojang's chunk generator, some do have large amounts of empty chunk sections, and would still be adversely affected by the fix.

I'm half convinced, still think it might be better as an optional thing in spigot which defaults to being disabled, perhaps combined with some logic when chunks are saved to strip empty chunk sections (if this doesn't cause more lighting issues).

> I don't think it serializes the world data via packets

That's actually exactly what happens in SP. This was not always the case but i think at least since release of 1.0 the SP is just a MP-Server wrapped with some code. You can see this with a stacktracer like visualvm inside the jdk and flying around the world/loading many chunks at once

Please review the detailed Mojang-Bug report - its SP too: https://bugs.mojang.com/browse/MC-80966

The screenshot was a local world (I did say that), I don't think it serializes the world data via packets, so there shouldn't be any desync between the client and the server (since they're working off the same data model). I did say I'd have to write some code to test if the server side really suffers from that lighting bug.

An alternative way to fix the problem would be to check the light similar to the ChunkSection -> nonEmptyBlockCount

The method public void recalcBlockCounts() checkes on every load if a section should be delivered or not (way more expensive compared to some more networkdata). You could introduce a new variable for the light:

    public int nonEmptyLightDataCount;  // kade fix light


    public boolean a() {
//	     return this.nonEmptyBlockCount == 0;
	     return  this.nonEmptyBlockCount == 0 && this.nonEmptyLightDataCount == 0 ; //kade fix, bukkit reports wrongly if a part of a chunk is empty or not.
    }


   public void recalcBlockCounts() {
        this.nonEmptyBlockCount = 0;
        this.tickingBlockCount = 0;
        this.nonEmptyLightDataCount = 0; // kade fix light

        for (int i = 0; i < 16; ++i) {
            for (int j = 0; j < 16; ++j) {
                for (int k = 0; k < 16; ++k) {
                    Block block = this.b(i, j, k);

                    if (block != Blocks.AIR) {
                        ++this.nonEmptyBlockCount;
                        if (block.isTicking()) {
                            ++this.tickingBlockCount;
                        }
                    }

                    if(this.d(i, j, k) != 0 || this.e(i, j, k) != 0){  // kade fix light
                    	this.nonEmptyLightDataCount++;  
                    }  

                }
            }
        }

    }

But as stated above i think the downside of this would be bigger as the CPU-bottleneck is generally more frequent and i rarely hear about bandwidth-problems. Still - this would be the correct fix for the problem.

Hey,

your screenshot is the result of the missing emittedLight-Data for a ChunkSection. The ChunkSection was not delivered as the method mentioned above reported it was not needed - when in fact it is needed to correct the lightning. The lightning on the server is not broken - just the delivery to the client if no blocks are in that section.

ChunkSizes: Where did you find it generates all the sections? I´d argue against that - but i dont know what you are referring to with "fix lightning".

Regarding the size: You heavily misjudge the implications.
As i´ve been running this code for a long time now let me summarize the impact:

• Only a small part of all chunks is affected by this bug ~<5%
• The size-increase will only be one ChunkSection in general (the top one)
• So on average you have an incrased ChunkSection-Delivery of 4 Sections on a ViewDistance of 4 at normally 324 Sections. Thats 1,2% in a very passive calculation. Probably <0.5% in reality.
  ( ViewDistance 4 -> 81 Chunks to be delivered. On a low-estimate those 81 Chunks have 4 Sections -> 324 Sections total. 5% affected Chunks would be 4 Chunks. Resulting in an increase of 4 Sections. 324 vs 328 Sections to be delivered. )

A special case - visible in your secreenshot - would be a empty section somewhere in the middle. This could increase the amount of sections per chunk by more than one. But still - look at how shitty it looks if you dont send that information. I dont view the argument about bandwidth-size as valid in those cases.

"Would have to write some serverside lighting debug code to verify though."
Just disable the mentioned line in the ChunkSection - as stated the light is correct on the server side.

> 1. "But the server does not send lighting information to the client at all, it is computed clientside."
Yeh, sorry I missed this, the decompiler hid the piece of code which writes the emitted light data inside a for(;.

> In fact Chunks cannot decrease in size once they grow.
Yeh, I'm aware of that one.

I think the lighting model is broken serverside as well:
http://cyberiantiger.org/1.8.7-brokelighting-singleplayer.png
Single player game with a wall adjacent to a chunk and a roof a few blocks above that chunk, have to save & quit, and reopen the world to reproduce it.

Would have to write some serverside lighting debug code to verify though.

An empty chunk section adds 8192 bytes of block data, 2048 bytes for emitted light and another 2048 bytes for the sky light info, apparently minecraft generates all the chunk sections up to the max height to fix lighting (but not custom generators - that might be a separate bug), so that's an extra 4-8 * 12288 bytes per chunk (before compression to be fair) bytes. I don't think you can call this a negligible performance impact, especially as the chunk may be sent to multiple players. Also of note, packets are compressed separately for each player receiving the chunk.

Hey,

there are several errors in your logic. I guess this is caused by not fully knowing what the difference between skyLight & emittedLight is. I´ll try to explain:

1. "But the server does not send lighting information to the client at all, it is computed clientside."
This is wrong. Please review the implementation of "ChunkSection emittedLight". Do not misread as skyLight.

2. "Your proposed fix won't actually work if there are ChunkSectionss which have always been empty".
This is wrong to. The Section is not emtpy as it was created to store the emittedLight wether or not it contains blocks.
In fact Chunks cannot decrease in size once they grow. This is a bad implementation in Minecraft similart to the DB-File-Size-Problem in mysql.

3. "downside of doubling the size of the chunk update packet since you'd normally have 8-4 unsent chunk sections out of a max of 16."
This is incorrect. You wont double anything here. ChunkSections with actually nothing in them are null and wont be delivered. Only the one Section above a Section with blocks will be delivered if it contains emittedLight.

The implications of this fix are described here as mentioned: https://bugs.mojang.com/browse/MC-80966
The performance-issue mentioned is definitely not a problem.

Edit:
"Also of note, the server suffers from almost exactly the same bug (with the caveat that lighting can be computed for empty chunksections which are allocated either by the world generator or by placing and removing blocks)."

This here is the Server-Bug and the presented fix for that Bug. The ClientBugs are just the Side-Effect of that Bug. Please review the Mojang Topic and the Video explanation ( https://www.youtube.com/watch?v=sfikoyETsJU ). The Video shows the wrongly hold back light-information by the server on a ChunkSection-Change (63 to 64 Y ).

>Your proposed fix won't actually work if there are ChunkSectionss which have always been empty as they will be null on the server so it will never reach the .a() method you propose changing, and it also has the downside of doubling the size of the chunk update packet since you'd normally have 8-4 unsent chunk sections out of a max of 16.

Yeah that's more or less the conclusion we came to.

But the server does not send lighting information to the client at all, it is computed clientside.

I assume the bug is that the client (and server) can't compute lighting for unallocated ChunkSections, because the lighting map is contained in those ChunkSections.

Your proposed fix won't actually work if there are ChunkSectionss which have always been empty as they will be null on the server so it will never reach the .a() method you propose changing, and it also has the downside of doubling the size of the chunk update packet since you'd normally have 8-4 unsent chunk sections out of a max of 16.

Given the performance issues, and that this is a workaround for a cosmetic client bug, if this is ever fixed, I hope the fix defaults to being disabled.

Also of note, the server suffers from almost exactly the same bug (with the caveat that lighting can be computed for empty chunksections which are allocated either by the world generator or by placing and removing blocks).

Your video shows that the light map is not working in the way I thought it'll work. Obviously it will only be used for transparent blocks and illuminate the neighbor blocks.

This bug is present in Vanilla Minecraft.
I reported it here: https://bugs.mojang.com/browse/MC-80966
Please review the more detailed explanation and the Example-Video including the World-Seed for reproduction.

Let me try to explain it another way:

1. I do not change anything in the world.
2. I do not generate anything in the world.
3. I do not change anything in the code.
4. I do not use plugins
5. I use the vanilla spigot build
With that setup the client will get the light-bugs visible in the screenshots. The light on the server-side is fine. Its only - as mentioned above - caused by missing information. The server does not report the necessary light-levels of the highest ChunkSelection-Layer in that special case. The screenshots are taken at a naturally occuring lava-lake to show that this is happening without any manipulation whatsoever.

Placing any block inside the problematic chunk above forcing the ChunkSelection to be instantiated will fix the problem.

Now I see, you're manipulating the light map and want Spigot to support that.
If you want that it should be easy for you to place some blocks into the chunk sections.

Vanilla behaviour is that AIR blocks don't emit light, so it's not a Spigot bug. Rather you're making a "feature" request.

@BlackHole
Your comment is not even related to the bug presented here.
This has nothing to do with calculations/worledit nor mapgeneration.

This bug ist only visible to players as the server fails to send all the needed chunk-information. And this happens because of the "this.nonEmptyBlockCOunt == 0;" check.

Think about it: If the Blocks are only in Chunksection 1 (0-15) and the light-values for layer 15 need to be stored in Y=16. Those information get lost because of the check above.

Lightning issues occur because lightning caluculation is only done if all neighbor chunks are loaded. So WorldEdit updates far away from players will lead to this.
If you still beleave you're having a different issue, make some test cases with empty chunk sections und chunks sections with some blocks (like glas).

Some lightning issues occor while generating the world. These are also independant from chunk sections.