Skip to content
This repository has been archived by the owner on Mar 8, 2024. It is now read-only.

[1.16.5 / Fabric] Certain areas not lighting up #25

Closed
sm9cc opened this issue Jan 24, 2021 · 19 comments
Closed

[1.16.5 / Fabric] Certain areas not lighting up #25

sm9cc opened this issue Jan 24, 2021 · 19 comments
Assignees
Labels
bug Something isn't working fabric Fabric version of the mod

Comments

@sm9cc
Copy link

sm9cc commented Jan 24, 2021

Hi there, sorry for not being much help here but as you can see in the image, the area inside square and tunnel is not lighting up correctly, the surrounding areas seem to be alright though, please let me know if there is any more information I should provide which may help.

https://i.imgur.com/Q9VNlav.png

Thanks

@Spottedleaf Spottedleaf added the bug Something isn't working label Jan 24, 2021
@Spottedleaf
Copy link
Member

can you give the world save? also was this world loaded in with starlight or did you create the world with starlight and build stuff and all of a sudden it's broken now?
also what version are you running?

@sm9cc
Copy link
Author

sm9cc commented Jan 24, 2021

The world is on my dedicated server, it was originally loaded with Phosphor, so I only noticed the problem when testing Starlight, speaking of which... I still have Phosphor on my server but Starlight on my client, could this be the problem?

I am running the latest commit built from source.

If you still need the world save let me know, in the meantime I will test with Starlight on the server, cheers.

@Spottedleaf
Copy link
Member

Spottedleaf commented Jan 24, 2021

Hm. Does using phosphor on the client give correct lighting?

Also, don't use starlight on the server before getting the world save! starlight will overwrite the light data completely

@sm9cc
Copy link
Author

sm9cc commented Jan 24, 2021

Gotcha, I will test Phosphor on the client again and I wont put Starlight on the server until I have copied the save.

@sm9cc
Copy link
Author

sm9cc commented Jan 24, 2021

Hmm, I am now unable to replicate this issue anymore with Phosphor server and Starlight client.

I have also tested
Phosphor server, Phosphor client
Starlight server, Phosphor client
Starlight server, Starlight client

I took some screenshots of the configurations and they all look normal to me - https://imgur.com/a/X3Jfv6u
(Note in the first screenshot its pitch black because of New moon phase and True Darkness mod)

I will continue testing with Starlight on both server and client and let you know if I find any issues.

World download (Prior to Starlight on server) if you still need it - https://mega.nz/file/GnRRFSZD#bo63SHHjYPRmDwzcBU4yoIa-yVQ3zQplqudL2dEswiQ

Please let me know if you would like me to send you a MultiMC instance of the pack which I am running.

Lastly, I added you on discord under CrucialCoding#4386 (Do you have a Discord server?) as its probably easier to communicate there.

Thanks for your help.

@SrBedrock
Copy link

Lastly, I added you on discord under CrucialCoding#4386 (Do you have a Discord server?) as its probably easier to communicate there.

Discord server: https://discord.gg/CgDPu27

@sm9cc
Copy link
Author

sm9cc commented Jan 24, 2021

Lastly, I added you on discord under CrucialCoding#4386 (Do you have a Discord server?) as its probably easier to communicate there.

Discord server: https://discord.gg/CgDPu27

Thanks, should have mentioned I already found it.

@Spottedleaf
Copy link
Member

Spottedleaf commented Jan 24, 2021

One of the reasons this would've looked fine on a more vanilla-like engine (vanilla or phosphor) is because vanilla does do aggressive relighting of areas sometimes, even on the client. That's probably why it was fixed later when you looked at it again.
However starlight never attempts any kind of relighting on the client, in fact the logic to do that simply doesn't exist. So when the server side data is totally wrong, starlight will not fix it.

So technically this is a bug as in "the lighting looks incorrect", but it's also not a bug as in "starlight messed up the data"

@konsolas
Copy link

In that case, is there any value to running Starlight on the client if it is not also running on the server?

@Spottedleaf
Copy link
Member

It's going to depend on how buggy the the light engine is on the server and how often the client would fix that lighting (it doesn't always do it, I know that it just can sometimes do it). Even in this case, the server ended up fixing the light, so we really don't know if it would've shown incorrect light on a vanilla-like client.

@Azarattum
Copy link

I had a similar problem after upgrading my server to starlight from phosphor. Most of the light sources were just ignored (even mob spawning occurred) and caves appeared bright for no reason.

That was simply resolved by restarting the server. No problems were further noticed.

@Spottedleaf
Copy link
Member

I can't help but feel if it's fixed by restarting that there's something wrong with how the client handles lighting updates from the server. I'm going to rework this code for next version and see how it handles it.

@Spottedleaf Spottedleaf self-assigned this Mar 24, 2021
@Spottedleaf Spottedleaf added the fabric Fabric version of the mod label Mar 24, 2021
Spottedleaf added a commit that referenced this issue Mar 28, 2021
- Completely rewrite how lighting is loaded in for clients
 Attempts to fix #25
 and #40
 The old hook was vulnerable to mods doing unexpected things, especially
 serverside. The old method also relied on unreliable ordering of events
 inside packet handling. New method just hooks directly into the
 packet handling methods to avoid conflicting behavior with mods and
 to create more reliable hooks.

- Completely rewrites internal scheduling of light tasks
 Old scheduling was very vulnerable to block updates completely
 overloading everything. The old scheduling was also incapable of
 indicating to the chunk system whether block updates or other
 light modifying tasks were pending for a chunk. The new
 scheduling now groups tasks by chunk (light update, needs lighting,
 edge checks) and simply drains tasks by chunks. In the event
 that there are a lot of block updates, they no longer pile up
 in a queue since they are just grouped onto their chunk, which
 will eliminate duplicate updates. And since it's a FIFO by chunk,
 the maximum latency for a light update to happen is simply the
 number of chunks queued. The new queue also tells the chunk
 system to not unload chunks until any lighting task is fully complete
 for that chunk and its 1 radius neighbours. This will eliminate
 problems where the light engine was never able to complete tasks
 before server shutdown. This also officially kills light suppression.
 Before you ask, no I'm not going to re-add lighting _bugs_ to my light
 engine. I want lighting to work, which means I need to fix it when
 it clearly doesn't work.

- Mark the vanilla light engine fields in LevelLightEngine as null
 This will break any mod expecting them to be non-null. But since
 we replace the light engine, it's better this way since it forces
 an explicit break rather than a silent break.
@Spottedleaf Spottedleaf reopened this Mar 28, 2021
Spottedleaf added a commit that referenced this issue Mar 28, 2021
- Completely rewrite how lighting is loaded in for clients
 Attempts to fix #25
 and #40
 The old hook was vulnerable to mods doing unexpected things, especially
 serverside. The old method also relied on unreliable ordering of events
 inside packet handling. New method just hooks directly into the
 packet handling methods to avoid conflicting behavior with mods and
 to create more reliable hooks.

- Completely rewrites internal scheduling of light tasks
 Old scheduling was very vulnerable to block updates completely
 overloading everything. The old scheduling was also incapable of
 indicating to the chunk system whether block updates or other
 light modifying tasks were pending for a chunk. The new
 scheduling now groups tasks by chunk (light update, needs lighting,
 edge checks) and simply drains tasks by chunks. In the event
 that there are a lot of block updates, they no longer pile up
 in a queue since they are just grouped onto their chunk, which
 will eliminate duplicate updates. And since it's a FIFO by chunk,
 the maximum latency for a light update to happen is simply the
 number of chunks queued. The new queue also tells the chunk
 system to not unload chunks until any lighting task is fully complete
 for that chunk and its 1 radius neighbours. This will eliminate
 problems where the light engine was never able to complete tasks
 before server shutdown. This also officially kills light suppression.
 Before you ask, no I'm not going to re-add lighting _bugs_ to my light
 engine. I want lighting to work, which means I need to fix it when
 it clearly doesn't work.

- Mark the vanilla light engine fields in LevelLightEngine as null
 This will break any mod expecting them to be non-null. But since
 we replace the light engine, it's better this way since it forces
 an explicit break rather than a silent break.
@Spottedleaf
Copy link
Member

1.0.0-RC1 has fixes in the suspected area. If you see this in that version let me know.

@Small-Ku
Copy link

Small-Ku commented May 5, 2021

I just use 1.0.0-RC2 in single player when exploring the GTA map which originally from https://youtu.be/hyD5IUejg6k and just directly upgraded to 1.16.5 in Minecraft. This happens a lot when I travel a long distance from the original place that I "logged in".
https://imgur.com/a/5cG3WcX
If I just switch to Phosphor and switch back, it seems fine again until I travel to some place again

@mrmangohands
Copy link

I just use 1.0.0-RC2 in single player when exploring the GTA map which originally from https://youtu.be/hyD5IUejg6k and just directly upgraded to 1.16.5 in Minecraft. This happens a lot when I travel a long distance from the original place that I "logged in".
https://imgur.com/a/5cG3WcX
If I just switch to Phosphor and switch back, it seems fine again until I travel to some place again

The map in question is from 1.10, and either it's corrupted or 1.16 can't handle an upgrade straight from 1.10 properly. The map has lighting issues when loaded with either starlight or vanilla, but relighting the chunks fixes it. Switching between starlight and phosphor is probably just forcing a relight, which fixes the issue locally, then when you go to areas that haven't been re-lit you see more errors.

@Spottedleaf Spottedleaf added needs-detail Lack of critical information vanilla Problem occurs with vanilla labels Jul 21, 2021
@Spottedleaf
Copy link
Member

Can you provide the world download (and coords) so I can see this locally?

@mrmangohands
Copy link

I got the mediafire link off an archive.org backup of the website in the video description http://www.mediafire.com/file/f7aa5h0ihxdj6hy/GTA_5.zip/file, coords I found corruption at were 5588 24 8544.

@Spottedleaf
Copy link
Member

I verified the corruption with starlight, I also verified this with Vanilla. So I really don't know what's going on here.

@Spottedleaf Spottedleaf added wontfix This will not be worked on and removed needs-detail Lack of critical information labels Jul 21, 2021
@Spottedleaf
Copy link
Member

Spottedleaf commented Jul 21, 2021

Yes your world does have conversion problems. That was caused by a notorious bug in spigot code dating back to that era where the map was created. You have no choice but to use a spigot (or one of its derivatives) to convert the world. I recommend at least Paper because it has a fix I wrote for this kind of thing: PaperMC/Paper@aed5031 (See Fix incorrect status dataconverter for pre 1.13 chunks)

This is also fixed by https://github.com/Tuinity/DataConverter

@Spottedleaf Spottedleaf removed vanilla Problem occurs with vanilla wontfix This will not be worked on labels Jul 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working fabric Fabric version of the mod
Projects
None yet
Development

No branches or pull requests

7 participants