Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incompatibility with Lithium #10

Closed
Wesley1808 opened this issue Dec 26, 2023 · 16 comments
Closed

Incompatibility with Lithium #10

Wesley1808 opened this issue Dec 26, 2023 · 16 comments
Labels
bug Something isn't working compatibility Compatibility with another mod

Comments

@Wesley1808
Copy link

Hey! I've come across some strange chunk section related issues when running Noisium together with Lithium.
An example of this is that entities will be completely unable to detect if they're in water in certain chunk sections.
Video showing this issue: https://streamable.com/4t71lf

What I'm suspecting is the cause of the issue here is that Lithium has this optimization to shortcut fluid checks based on whether or not a chunk section contains any block of a certain flag. These values are maintained here, but because of the below optimization in Noisium I believe there might be a chance this doesn't always get updated properly.

// Set the blockstate in the palette storage directly to improve performance
var blockStateId = chunkSection.blockStateContainer.data.palette.index(blockState);
chunkSection.blockStateContainer.data.storage().set(
chunkSection.blockStateContainer.paletteProvider.computeIndex(chunkSectionBlockPosX,
chunkSectionBlockPosY, chunkSectionBlockPosZ
), blockStateId);

It might also be worth noting that this isn't always easy to reproduce and can take a while to find, and that it only really seems to happen in newly generated chunks (though that would probably make sense if this is the cause of it).

These blockstate flags are also used for other optimizations, so there's a good chance it may sometimes be breaking those aswell.

@Steveplays28 Steveplays28 added bug Something isn't working compatibility Compatibility with another mod labels Dec 26, 2023
@Steveplays28
Copy link
Owner

Hi Wesley, thanks for the report!
I'll take a look and see if I can reproduce/fix the issue.

@Steveplays28
Copy link
Owner

I see the issue, there's a mixin injection into setBlockState, which I bypass by setting blocks in the palette directly.
I'll have to call that manually from Noisium's end if Lithium is present, or make a PR to Lithium to add compatibility.

@NotADinosaur
Copy link

Hi, I/some friends have noticed this bug on my server after I added this mod to test its improvements, and with the water mobs dying it has started a "fish cult" lmao
I know this isn't useful to the issue but I thought it was funny :P

@Steveplays28
Copy link
Owner

Hi NotADinosaur, that's a funny story. Last I heard, this issue was being worked on by a Lithium contributor.

@embeddedt
Copy link

I'm curious - what is the performance delta of your microoptimization to setBlockState? At a cursory glance, the only logic you are skipping is decrementing the counters for the old block (presumably because you know it's always air), and I'm skeptical this saves enough time to be worth integrating with Lithium for.

@Steveplays28
Copy link
Owner

Steveplays28 commented Mar 12, 2024

@embeddedt Hi embeddedt,

This mod does indeed do a few micro-optimizations, though they aren't as good as I'd like them to be. Not even the main optimization is as good as I'd like it to be. That's for the future to improve on my end.
I did some exact testing with Uniter sometime ago, which showed performance benefits range between basically nothing and a few % per some big amount of chunks generated (iirc), and it's quite hardware-dependent.
In the grand scheme of things this does save a bit of time, which is better than nothing imo.

And, this bug is slightly critical for BuilderB0y's Big Globe, since it has the same issues. I don't currently have time to create a PR for this myself, but when I do have time I'll look into solutions in more detail. Lithium compat is quite important to me (and Builder is interested as well).
I appreciate you taking the time to investigate this issue.

@Steveplays28 Steveplays28 pinned this issue Apr 7, 2024
@Steveplays28 Steveplays28 added the waiting for release This issue has been fixed, but the fix hasn't been released yet label Apr 14, 2024
@Steveplays28
Copy link
Owner

Steveplays28 commented Apr 14, 2024

I've released a fix for this in v2.0.2.

@Steveplays28 Steveplays28 removed the waiting for release This issue has been fixed, but the fix hasn't been released yet label Apr 14, 2024
@GopsRay
Copy link

GopsRay commented Apr 19, 2024

1,Hlo Steveplays,does this new release of noisium mod 2.0.2 (1.20-1.20.1) completely fix this issue [ Incompatibilities
Lithium: submerged entities (such as fish and squids) occasionally suffocate in water and/or fall to the ground in water. All other features will work fine, just need to occasionally sacrifice a fish for performance.]?? 2,And now Noisium is fully compatibile with lithium?? 3,Does your mod {CHANGE} or {ALTER} World generation?? Functions?? 4,please clarify my questions mainly q:3.

@Steveplays28
Copy link
Owner

Hi GopsRay, the issue is completely fixed now. I forgot to update the readme. Thanks for reminding me about that.

@GopsRay
Copy link

GopsRay commented Apr 19, 2024

Its OK u made my potato laptop run smoother But can u pls answer this question Does your mod {CHANGE} or {ALTER} World generation?? Functions??

@Steveplays28
Copy link
Owner

No, it doesn't alter world generation. Worlds generated with Noisium installed will look the exact same as without it.

@GopsRay
Copy link

GopsRay commented Apr 19, 2024

Thx Can I join ur discord

@Steveplays28
Copy link
Owner

Sure, feel free to stop by. Link can be found at my GitHub profile and at my website.

@GopsRay
Copy link

GopsRay commented Apr 19, 2024

Noisium has full 1:1 parity with vanilla worldgen (worldgen without Noisium). oh ok I did't read this soory for wasting ur time vanilla and noisium world generation are the same soory for disturbing you.

@Steveplays28
Copy link
Owner

@GopsRay No worries, have fun playing.

@XandarNull
Copy link

@GopsRay No worries, have fun playing.

What a chad mod dev, thanks for being nice and making this great mod

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compatibility Compatibility with another mod
Projects
None yet
Development

No branches or pull requests

6 participants