[MC-7508] lighting error due to Chunk.heightMap ignoring block at the top level of an ExtendedBlockStorage instance (off by 1 error)

REPRODUCE
1: load the attached world (mirrored at http://www.mediafire.com/?8o5hj55pwjolwos )
2: walk under the platform. Watch the shadow disappear. This only happens with platforms at level 15, 31, 47, 63, etc, for the reasons described below

CONFIRM FROM SCRATCH
1: start a flat world.
2: Build a large 1 block thick platform at y level 31 (or 15 or 63)
3: exit.
4: Load world again. walk on the ground under the platform. Shadow from the platform disappears as the lighting is updated based on the heightmap (which does not see the platform)

THE CAUSE
Chunk.java has an error in the generation of heightMap (an int[256] ) in the method public void generateHeightMap().

int var1 = this.getTopFilledSegment();
returns the highest ExtendedBlockStorage with a block in it.

inside the x and z loops (from 0 to 16), for each x-z coordinate the method establishes the highest block with opacity > 0.

int var4 = var1 + 16 - 1;
starts us at the top of the ExtendedBlockStorage returned earlier, then it loops down to find the first block with opacity > 0.

The problem is an off by one error. The blocks that are checked are retrieved like so: Block var5 = this.func_150810_a(var2, var4 - 1, var3);

the "var4 - 1" is the problem. The top block is ignored / never checked.

If the top two blocks are solid, the heightmap is set to the height of the lower one. If the top block is solid, then the next four are air then the next is solid, the height map is set to 5 below the top block.

The method either needs to start checking at this.getTopFilledSegment() + 16 instead of this.getTopFilledSegment() + 16 - 1, or inside the loop check this.func_150810_a(var2, var4, var3) instead of this.func_150810_a(var2, var4 - 1, var3)

This bug (and related) is likely the cause of some of the lighting glitches seen. See attached image for a visual representation of this report. Also, all variables, classes and method names are from MCP. I of course do not have access to your internal codebase. Hopefully you can translate.

This bug has been around sine 1.8 BETA Please kill it 😞

IMAGES ATTACHED:
visual explanation of bug, and shot of bug in action

Related issues

is duplicated by

MC-7442 Lighting error in superflat worlds.

MC-51752 lighting error due to Chunk.heightMap ignoring block at the top level of an ExtendedBlockStorage instance (off by 1 error)

relates to

MC-80966 Lightcalculation of ChunkSelection faulty implemented resulting in client bugs (empty chunks don’t show light/render dark)

MC-13544 Wrong LC-values

MC-29490 1.7+ lighting bug / black spots

MC-63346 lighting error at heights Multiples of 16

Attachments

shadowTestFlatWorld.zip

Comments

migrated 2013-01-15T03:03:24Z

[media][media][media][media]

migrated 2013-01-15T03:42:32Z

lighting errors as a result of the heightmap ignoring y levels 31 and 63 (as it does for every level that's at the top of an ExtendedBlockStorage)

kumasasa 2013-01-15T07:27:00Z

Steps to reproduce (see MC-7442 ):

Create a superflat world (Preset Overworld)
Dig a tunnel 1 block below the surface

migrated 2013-01-17T11:53:18Z

So to clarify, this is caused by the subtraction (to get 0-based index from 1-based index) happening twice, at
int var4 = var1 + 16 *- 1*;
and
int var5 = this.getBlockID(var2, var4 *- 1*, var3);

So removing either offending subtraction would fix it.

migrated 2013-02-06T00:07:22Z

"The method either needs to check this.getTopFilledSegment() + 16 instead of this.getTopFilledSegment() + 16 - 1, or inside the loop check this.getBlockID(var2, var4, var3) instead of this.getBlockID(var2, var4 - 1, var3)" Quote from Jason Winzenried

So I tested your theory and guess what...It works, it got rid of the light bug! All I did was remove the -1 from the line "this.getBlockID(var2, var4 - 1, var3)" hopefully the team of Mojang will have a closer look at this part of the code and see what is going on there. I don't want to keep on removing that silly little number all the time when the game updates. 😉

migrated 2013-02-14T21:27:19Z

part of me wants to just wait and see if this gets fixed along with Dinnerbone's lighting changes. At the same time, as long as he is doing that, I'd love to be sure this actually gets addressed. This bug has been around since 1.8 beta at least.

migrated 2013-04-16T09:46:56Z

This bug is not related to MC-13544. That problem is not related to any lighting issues like the one here is.

migrated 2013-07-04T09:00:43Z

deleterious effects, clear explanation, two character fix. six months old.

this bug tracker is useless

Ezekiel 2013-07-10T02:04:32Z

Is this still a concern in the current Minecraft version? If so, please update the affected versions in order to best aid Mojang ensuring bugs are still valid in the latest releases/pre-releases.

bugi74 2013-07-10T15:44:27Z

Source code seems to have this bug still in 1.6.2, but I seem to be unable to reproduce this (or MC-911), so I can not really confirm. Nobody has yet been able to provide me an exact method to reproduce, or somehow my system is immune 😛
(Edit: clarification: I have never been able to confirm this or MC-911, so in that sense, my words are not indication that it might have been fixed, either.)

migrated 2013-09-24T00:32:01Z

still present in 1.6.4 and 1.7pre

REPRODUCE:

1: load this world: http://www.mediafire.com/?8o5hj55pwjolwos
2: walk under the platform. Watch the shadow disappear. This only happens with platforms at level 15, 31, 47, 63, etc, for the reasons described in the initial report.

confirm from scratch
1: start a flat world.
2: Build a large 1 block thick platform at y level 31 (or 15 or 63)
3: exit.
4: Load world again. walk on the ground under the platform. Shadow from the platform disappears as the lighting is updated based on the heightmap (which does not see the platform)

migrated 2013-11-26T20:53:25Z

Is this still a concern in the current Minecraft version 1.7.2 / Launcher version 1.3.4 ? If so, please update the affected versions in order to best aid Mojang ensuring bugs are still valid in the latest releases/pre-releases.

Ezekiel 2014-01-11T15:59:55Z

This ticket is incomplete without the requested information, no response has been received within a reasonable time and we are assuming the issue has been resolved. Should your issue return please submit a new complete ticket with all available information.
Please review the guidelines before reporting issues.

migrated 2014-03-20T21:28:15Z

yes it's still an issue. How can you expect me to check this report every few months when none of you** can take a second to change two characters in the codebase and fix it? How much effort am I supposed to expend on this?

**I unfairly directed this ire towards the moderators. Sorry!

migrated 2014-03-20T22:01:04Z

I'm sorry updating affected versions once every few months to ensure the bug is known to exist in current versions is too much effort, however you reported the bug and I can't say that an additional 15 seconds of your time every few months is too much to ask. Frequently bug reports go stale because the user abandons them, or the issue is fixed by another patch and the user abandons the ticket without informing us it has been fixed. Updating the affected versions ensures the ticket stays alive.

We are moderators not Mojang employees, we cannot change the code.

migrated 2014-03-20T22:17:28Z

of course, I was far to broad in my usage of "you". Of course you can't do anything about it. I wish someone would though! So yeah I slacked off on checking (I've been keeping it up for a while, as you will notice from the post date + list of affected versions). 1.7.2 to 1.7.5 wasn't really a large enough change to prompt me to check again. I've done so now, and with the latest snapshot, and it is still there.

I do wish you would get your story straight between "Should your issue return please submit a new complete ticket with all available information." and "Please do not clone existing tickets"

migrated 2014-03-20T22:21:26Z

The reason that is posted is in many cases a reply after the ticket has been closed may not be noticed if a moderator is not actively watching the queue. However I was, and your ticket was reopened.

Ezekiel 2014-03-20T22:31:23Z

Jason, actually that wording has since been removed, it was incorrect. We now use

This ticket is incomplete without the requested information, no response has been received within a reasonable time and we are assuming the issue has been resolved. If you are still experiencing this issue, we can reopen it at your request.
Please review the guidelines before reporting issues.

migrated 2014-05-25T07:38:09Z

Is this still a concern in the current Minecraft version 14w21b / Launcher version 1.4.4 or later? If so, please update the affected versions in order to best aid Mojang ensuring bugs are still valid in the latest releases/pre-releases.

migrated 2014-06-28T07:22:19Z

Still happening on Launcher version 1.4.4 and MC Version 1.7.10 + 14w26c.
Could a Mod test it and set the Confirmation Status?

kumasasa 2014-06-28T08:53:06Z

How to test ?

migrated 2014-06-28T09:00:24Z

Follow the "REPRODUCE:" or "CONFIRM FROM SCRATCH:"

kumasasa 2014-06-28T09:29:26Z

Derp. Will test later.

migrated 2014-07-10T16:36:40Z

Thank you.
14w28b is affected too.

migrated 2014-07-11T02:04:21Z

Confirmed this is still happening with 14w28b using "CONFIRM FROM SCRATCH" instructions. I used flat terrain and built 3 platforms - one at Y=30, Y=31, and Y=32. Platform built at Y level 31, when re-loaded, has the shadow disappear when walking over top of the shadow on the ground. (Shadow stays for platforms at Y=30 and Y=32)

migrated 2014-07-11T02:22:16Z

Went back and confirmed with 14w28a as well.

migrated 2014-07-16T16:32:55Z

14w29a is affected too. Can a mod please update the affected versions?

migrated 2014-07-23T15:03:16Z

The fix flips the Bug to the upper Side.
And introduces a new one were the blocks on the ground wrongly light up and stays lit up:
https://www.youtube.com/watch?v=VAht6IisnQw
Version 14w30a and 14w30b

migrated 2014-07-23T20:52:58Z

Can a mod please clarify: should this be re-opened or a new ticket submitted for the newly introduced bug that supposedly fixed this one?

kumasasa 2014-07-23T20:55:12Z

Please make a new ticket.

migrated 2014-07-24T02:16:07Z

MC-63346 is the new one

kumasasa 2014-07-24T06:14:50Z

Ok.

migrated 2014-07-24T11:43:03Z

Thank you, and can you please test the bug and set it to confirmed?
So we maybe can get it fixed before 1.8

migrated 2014-07-24T11:56:22Z

it is fixed in the snapshot of 1.8.... so it's fixed in 1.8....

migrated 2014-07-24T12:28:19Z

It is not fixed, look at https://www.youtube.com/watch?v=VAht6IisnQw and MC-63346

migrated 2014-08-05T23:14:49Z

Parts of it were unfixed due to certain trees not generating... https://twitter.com/SeargeDP/status/492316809115086848 I can confirm that it's back, but shows up obscurely.

JIRA should have a "Partially Fixed" field for these kinds of bugs.