grondag / darkness

Hardcore darkness for Fabric mod loader
GNU Lesser General Public License v3.0
13 stars 27 forks source link

Lightmap updates too late and causes incompatibility with Distant Horizon #30

Open Celivalg opened 1 year ago

Celivalg commented 1 year ago

So True Darkness injects themselves inside DynamicTexture.upload at HEAD; see here https://github.com/grondag/darkness/blob/1.18/common/src/main/java/grondag/darkness/mixin/MixinDynamicTexture.java#L45

Instead of INVOKE of DynamicTexture.upload in LightTexture.class; see here https://github.com/grondag/darkness/blob/1.18/common/src/main/java/grondag/darkness/mixin/MixinLightTexture.java#L46 , where \* is injected to set a flag to the dynamic texture to make sure it's not invoked for every dynamic texture instance.

injecting at INVOKE would allow the code to be cleaner (since you wouldn't need that flag inside the DynamicTexture class) and would allow distant horizon to inject themselves after the lightmap has been updated (since update doesn't return a value (edit: still true but not relevant) Since you are modifing the data inside the DynamicTexture.upload, DH can't access the local variables inside that context, so INVOKE_ASSIGN injection wouldn't work) For reference, here is how Distant Horizon gets the lightmap data: https://gitlab.com/jeseibel/minecraft-lod-mod/-/blob/main/forge/src/main/java/com/seibel/lod/mixins/client/MixinLightmap.java

Now I'm not that familiar with the FML and mixins in their context, and I don't know how you would ensure DH reads the lightmap values after True Darkness has set them, but using an aditional flag, and flag check seems wastefull, adding a condition to be checked every time some dynamic texture is updated. So even if that doesn't fix distant horizon compatibility, it would be a (small but still there) performance upgrade imo.

jeseibel commented 1 year ago

Related Distant Horizons issue: https://gitlab.com/jeseibel/minecraft-lod-mod/-/issues/278

Aceplante commented 1 year ago

How is this OVER A YEAR old and still not fixed :(