Improve compatibility for `carpettisaddition.mixins.rule.chunkTickSpeed.ServerWorldMixin `
zly2006 opened this issue · 6 comments
Bug description
Game crashed.
This issue:
zly2006/reden-is-what-we-made/issues/39
Code Digging:
// carpettisaddition.mixins.rule.chunkTickSpeed.ServerWorldMixin
@Inject(
method = {"tick"},
at = {@At(
value = "INVOKE",
target = "Lnet/minecraft/server/world/ServerWorld;getChunkManager()Lnet/minecraft/server/world/ServerChunkManager;"
)}
)
void resetTickChunkDepth(BooleanSupplier shouldKeepTicking, CallbackInfo ci) {
this.depth = 0;
}
Suggested:Lnet/minecraft/world/chunk/ChunkManager;tick(Ljava/util/function/BooleanSupplier;Z)V
In the previous source code, the injection point is getChunkManager
. Turning it into tick
could make it more reasonable and reduce the chance of incompatibility.
I can make a PR for it if you agree to modify your source code.
Steps to reproduce
Launch the game with zly2006/reden-is-what-we-made
Expected behavior
No response
Actual behavior
No response
Relevant logs
No response
Minecraft version
1.20.1
Carpet TIS Addition version
mc1.20.1-v1.51.0
Fabric Carpet version
n/a
Other information
No response
Check list
- I have verified that the issue persists in the latest version of the mod.
- I have searched the existing issues and confirmed that this is not a duplicate.
Well I can fix this myself, but I still suggest you change the injection point because using tick
is more reasonable.
Turning it into tick could make it more reasonable and reduce the chance of incompatibility
- Please define "more reasonable"
@Inject
is one of the most harmless mixin operation, it only conflicts with@Overwrite
which rewrites the entire method and destroys everything. If this could lead to a crash, I'll suggest to look into reden, or another mod might triggers the crash in the same postion next time
Tweak the mixin inject position from getChunkManager()
to getChunkManager#tick
is doable, but I won't do it without any proper reason, or just because other mod @Overwrite
s everything
so what if another mod, with a lower mixim priority, called the getChunkManager? your depth will be set to zero and that is a side effect. I don't think a getter should have unnecessary side effects and that may be problematic.
so what if another mod, with a lower mixim priority, called the getChunkManager? your depth will be set to zero and that is a side effect
Nothing wrong will happen. The resetTickChunkDepth
method is only for resetting the depth
on exception being thrown, e.g. for update suppressors. On normal executtion, it doesn't matter how many times (0, 1, 100...) it will get invoked
If it's possible, don't use @Overwrite
, especially on such complicated and commonly used method. It's very desctructive and can break tons of mods
If you have to use @Overwrite
, please make sure the vanilla method signatures / orders are kept untouched, or mixin confliction with other mod will happen, sooner or later