search

AtomicStryker / atomicstrykers-minecraft-mods

Repository for my open source Minecraft Mods
https://atomicstryker.github.io/
180 stars 96 forks source link

[Dynamic Lights] 1.18.1 Deadlock on World Creation #424

Closed Darkosto closed 2 years ago

Darkosto commented 2 years ago

Hey AtomicStryker,

I've run across a bit of an issue involving Dynamic Lights. This is a bit of a difficult issue to replicate, but after spending quite some time we've identified what's going on.

After loading up a MC client and creating a new world, the game will deadlock and won't proceed. You have to kill the instance and start again to get into a world. The issue will not be on every world load, so it's tough to debug right away. After narrowing it down to Dynamic Lights, we did a thread dump and discovered that it's most likely Dynamic Lights accessing Entity#level#getBlockState before worker threads are available and before the chunk is finished.

It should be fixable by inserting: if(!entity.level.isLoaded(pos)) return; at https://github.com/AtomicStryker/atomicstrykers-minecraft-mods/blob/1.18/DynamicLights/src/main/java/atomicstryker/dynamiclights/server/modules/DroppedItemsLightSource.java#L135

MC: 1.18.1 Forge: 39.1.0 Dynamic Lights: 1.18.3

Let me know if I can provide any further info to help out or test. Thank you! Darkosto

AtomicStryker commented 2 years ago

Thanks for the report, i've pushed 1.18.4 to curse (for MC 1.18.2) https://github.com/AtomicStryker/atomicstrykers-minecraft-mods/commit/59c114e6259ee9db29878a51238cfd7a6006b24a If you need a build for 1.18.1, i guess i could make one

GirafiStudios commented 2 years ago

1.18.2 breaks a lot of things for a lot of mods, manually due to major changes to how registration is loaded now. So a lot of mods is still "stuck" on 1.18.1, so a 1.18.1 version would be much appreciated :)

AtomicStryker commented 2 years ago

Sure thing. 1.18.1 version built and sent to curse

Shadows-of-Fire commented 2 years ago

So that didn't actually appear to fix it... It's passing the check and then still deadlocking, which should be technically impossible since passing isLoaded means the chunk is already present.

Do you think you could possibly drop the getBlockState entirely and defer to entity#isInWater like you do here https://github.com/AtomicStryker/atomicstrykers-minecraft-mods/blob/1.18/DynamicLights/src/main/java/atomicstryker/dynamiclights/server/modules/DroppedItemsLightSource.java#L168

AtomicStryker commented 2 years ago

https://github.com/AtomicStryker/atomicstrykers-minecraft-mods/commit/7cbf42dce800953cefacd4b4b8e371c8bc86e121

OK, pushed 1.18.5 for MC 1.18.1 to curse

Darkosto commented 2 years ago

Everything is working great after the update. Thank you very much!