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

Fixed render crash when placing Universal Cables on ValkyrienSkies ships #8260

Open
wants to merge 2 commits into
base: release/1.20.x
Choose a base branch
from

Conversation

Andy608
Copy link

@Andy608 Andy608 commented Nov 17, 2024

Changes proposed in this pull request:

  • Added ValkyrienSkies and Eureka as runtime mods to help with mod compatibility testing. (KotlinForForge as well since VS relies on it)
  • Updated the render function in RenderUniversalCable.java to check against null refs. Previously, getTransmitterNetwork() would return null when initially placing or removing a cable from a VS ship which would cause the client to crash.

Now with this fix, the client no longer crashes and players are able to place cables on VS ships.

- Added ValkyrienSkies and Eureka as runtime mods to help with mod compatibility testing.
@Andy608
Copy link
Author

Andy608 commented Nov 17, 2024

This fix is in response to the open issue on the ValkyrienSkies github page: ValkyrienSkies/Valkyrien-Skies-2#558

@thiakil
Copy link
Member

thiakil commented Nov 17, 2024

Valkyrien Skies should be checking net.minecraft.client.renderer.blockentity.BlockEntityRenderer#shouldRender which runs through mekanism.client.render.transmitter.RenderUniversalCable#shouldRenderTransmitter and would not return true if there's no network.

@Andy608
Copy link
Author

Andy608 commented Nov 17, 2024

Sounds good, thank you! I have been trying to get ValkyrienSkies to compile with Mekanism as a runtime mod in their repo, but I continue to get this error when running the game:

Caused by: java.lang.NoSuchMethodError: 'java.lang.String net.minecraft.Util.m_137492_(java.lang.String, net.minecraft.resources.ResourceLocation)'

image

Has anyone run into this error before? I would love to investigate this issue from the VS side of things, but I can't seem to get it to compile properly. I had no issues getting ValkyrienSkies running in mekanism's build.gradle, so I'm not quite sure what I'm missing. Any help is appreciated. Thanks!

@Andy608
Copy link
Author

Andy608 commented Nov 17, 2024

Okay figured it out, and have submitted a fix on the VS side here: ValkyrienSkies/Valkyrien-Skies-2#1006

@Andy608
Copy link
Author

Andy608 commented Nov 18, 2024

After further investigation, it turns out the VS side change isn’t the ideal fix. Using shouldRender() causes beds and other uniquely shaped objects to become invisible. I believe adding a proper null check here would be sufficient in this case, though I’m definitely open to other suggestions if anyone has ideas for a better solution.

I also see render() calls shouldRender() so I'm still uncertain how the cable's render is even getting called. I'll need to do some more investigating on this.

For now, I’m using a custom-built version of Mekanism with the null check locally to prevent crashes.

@thiakil
Copy link
Member

thiakil commented Nov 18, 2024

can it actually be reproduced with only Mekanism and VS?
They're right that it shouldn't be needed, but its either not being called or there's a race condition somewhere

@Andy608
Copy link
Author

Andy608 commented Nov 18, 2024

Yee it occurs with only VS and Mekanism installed together. I found that adding shouldRender() to MekanismTileEntityRenderer fixes the crash (see most recent commit)

@thiakil
Copy link
Member

thiakil commented Nov 18, 2024

Can you send a crashlog with just Mek and VS?

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

Successfully merging this pull request may close these issues.

2 participants