[Quilt] Crash with Balm
SplendidAlakey opened this issue ยท 5 comments
MC 1.19.2
QSL 4.0.0-beta12
Balm 4.5.2+0
Terrestria 5.0.1
When Terrestria and Balm are installed together on Quilt, the game crashes on startup. The issue doesn't happen on Fabric.
Let me know if this needs to be reported to Balm or Quilt instead.
Logs: https://gist.github.com/SplendidAlakey/dde2c2bdf95113aa01eb866725e0ee53
I can reproduce this but I do not think the issue is in Terrestria. On a hunch I removed the included copies of FAPI libs from Terrestria but nothing changed.
- I can't find any way in which Terrestria mixes in or otherwise modifies the Minecraft method in question.
- Balm already has a bunch of reports of mixin failures including some quite similar to this one.
- Their mixin failures are apparently sporadic and likely depend on mod load order so adding or removing a mod could very easily trigger the problems without the added mod being directly involved in the failure.
- Terrestria does not officially support Quilt at the moment (but this is just a nail in the coffin).
I will leave this issue open for the moment but I don't intend to look into it further unless somebody is able to demonstrate how Terrestria is responsible for the Balm crash.
@gniftygnome It looks like Terrestria does a redirect to getAvailableMoisture which is what Balm is trying to modify variable at.
https://github.com/TerraformersMC/Terraform/blob/1.19.3/terraform-dirt-api-v1/src/main/java/com/terraformersmc/terraform/dirt/mixin/MixinCropBlock.java
Redirects are not stackable and are an unsafe mixin. A push should be done to remove or replace all redirects with better alternatives that do not break other mod's mixins. You can even look into using MixinExtras if needed to wrap calls easier or try something similar to what balm is doing. https://github.com/LlamaLad7/MixinExtras/wiki/ModifyExpressionValue
Unless the crash can still be reproduced without a redirect, I would say the redirect here is the issue and cause. ShetiPhan also does a redirect to the same method spot and will conflict with Terrestria definitely so this is an issue.
Ahh, thanks for tracking that down. So, this is basically TerraformersMC/Terraform#61 as it turns out. I, too, prefer to avoid replacement mixins of all types, but old code has a way of being special. Terraform API needs to have all its mixins audited and modernized if applicable. Wouldn't it be nice if I could quit my day job and spend all day rewriting code for free... In any case, knowing what is going on here, I'll try and look into it when I've got some available time.
Just wanted to inform that this continues to happen in latest versions of Balm, Terrestria and Quilt
Terrestria as of our 1.19.4 betas no longer redirects. It is ... non-trivial to JiJ MixinExtras in a library mod ... so I hope that will get easier in the future. We remain incompatible with MoreTags because of their redirects, so we have added a breaks relationship for that mod.