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

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

Open
Celivalg opened this issue Dec 26, 2022 · 2 comments

Comments

@Celivalg
Copy link

Celivalg commented Dec 26, 2022

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 <init>* 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
Copy link

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

@Aceplante
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants