ServerCore

ServerCore

7M Downloads

Incompatibility With Clumps Mod

ArcusMG opened this issue · 11 comments

commented

Describe the bug
When having Clumps and ServerCore installed together, the game will fail to load complaining that Clumps has already overridden the Mixin for experience orbs. This didn’t happen with the previous version of ServerCore.

This appears to be the relevant bits from the log but I attached the whole log just in case 2021-11-19-1.log.gz:

[19/11/2021 14:27:01 PM] Caused by: java.lang.BootstrapMethodError: java.lang.RuntimeException: Mixin transformation of net.minecraft.class_1303 failed
[19/11/2021 14:27:01 PM] at net.minecraft.class_1299.(class_1299.java:164)
[19/11/2021 14:27:01 PM] at net.minecraft.class_3103.(class_3103.java:28)
[19/11/2021 14:27:01 PM] at net.minecraft.class_3031.(class_3031.java:80)
[19/11/2021 14:27:01 PM] at net.minecraft.class_5464.(class_5464.java:184)
[19/11/2021 14:27:01 PM] at net.minecraft.class_5458.method_30571(class_5458.java:46)
[19/11/2021 14:27:01 PM] at net.minecraft.class_5458.method_30566(class_5458.java:75)
[19/11/2021 14:27:01 PM] at java.base/java.util.LinkedHashMap.forEach(LinkedHashMap.java:723)
[19/11/2021 14:27:01 PM] at net.minecraft.class_5458.(class_5458.java:74)
[19/11/2021 14:27:01 PM] at net.minecraft.class_2378.(class_2378.java:266)
[19/11/2021 14:27:01 PM] at net.minecraft.class_2966.method_12851(class_2966.java:44)
[19/11/2021 14:27:01 PM] at net.minecraft.client.main.Main.main(Main.java:139)
[19/11/2021 14:27:01 PM] at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[19/11/2021 14:27:01 PM] at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
[19/11/2021 14:27:01 PM] at java.base/jdk.internal.reflect.DelegatingMethodAccessorI
mpl.invoke(DelegatingMethodAccessorImpl.java:43)
[19/11/2021 14:27:01 PM] at java.base/java.lang.reflect.Method.invoke(Method.java:567)
[19/11/2021 14:27:01 PM] at net.fabricmc.loader.impl.game.minecraft.MinecraftGameProvider.launch(MinecraftGameProvider.java:568)
[19/11/2021 14:27:01 PM] ... 2 more
[19/11/2021 14:27:01 PM] Caused by: java.lang.RuntimeException: Mixin transformation of net.minecraft.class_1303 failed
[19/11/2021 14:27:01 PM] at net.fabricmc.loader.impl.launch.knot.KnotClassDelegate.getPostMixinClassByteArray(KnotClassDelegate.java:224)
[19/11/2021 14:27:01 PM] at net.fabricmc.loader.impl.launch.knot.KnotClassDelegate.tryLoadClass(KnotClassDelegate.java:133)
[19/11/2021 14:27:01 PM] at net.fabricmc.loader.impl.launch.knot.KnotClassLoader.loadClass(KnotClassLoader.java:155)
[19/11/2021 14:27:01 PM] at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:519)
[19/11/2021 14:27:01 PM] ... 18 more
[19/11/2021 14:27:01 PM] Caused by: org.spongepowered.asm.mixin.transformer.throwables.MixinTransformerError: An unexpected critical error was encountered
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.transformer.MixinProcessor.applyMixins(MixinProcessor.java:392)
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.transformer.MixinTransformer.transformClass(MixinTransformer.java:234)
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.transformer.MixinTransformer.transformClassBytes(MixinTransformer.java:202)
[19/11/2021 14:27:01 PM] at net.fabricmc.loader.impl.launch.knot.KnotClassDelegate.getPostMixinClassByteArray(KnotClassDelegate.java:222)
[19/11/2021 14:27:01 PM] ... 21 more
[19/11/2021 14:27:01 PM] Caused by: org.spongepowered.asm.mixin.throwables.MixinApplyError: Mixin [clumps.mixins.json:MixinExperienceOrb] from phase [DEFAULT] in config [clumps.mixins.json] from mod [clumps] FAILED during APPLY
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.transformer.MixinProcessor.handleMixinError(MixinProcessor.java:638)
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.transformer.MixinProcessor.handleMixinApplyError(MixinProcessor.java:589)
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.transformer.MixinProcessor.applyMixins(MixinProcessor.java:379)
[19/11/2021 14:27:01 PM] ... 24 more
[19/11/2021 14:27:01 PM] Caused by: org.spongepowered.asm.mixin.injection.throwables.InvalidInjectionException: InjectionPoint(Shift)[@at("INVOKE")] on net/minecraft/class_1303::merge with priority 1000 cannot inject into net/minecraft/class_1303::method_31497(Lnet/minecraft/class_1303;)V merged by org.provim.servercore.mixin.performance.ExperienceOrbEntityMixin with priority 1000 [PREINJECT Applicator Phase -> clumps.mixins.json:MixinExperienceOrb -> Prepare Injections -> -> handler$zza000$merge(Lnet/minecraft/class_1303;Lorg/spongepowered/asm/mixin/injection/callback/CallbackInfo;)V -> Prepare]
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.injection.code.Injector.findTargetNodes(Injector.java:305)
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.injection.code.Injector.find(Injector.java:240)
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.injection.struct.InjectionInfo.prepare(InjectionInfo.java:421)
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.transformer.MixinTargetContext.prepareInjections(MixinTargetContext.java:1338)
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.transformer.MixinApplicatorStandard.prepareInjections(MixinApplicatorStandard.java:1043)
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.transformer.MixinApplicatorStandard.applyMixin(MixinApplicatorStandard.java:393)
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.transformer.MixinApplicatorStandard.apply(MixinApplicatorStandard.java:325)
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.transformer.TargetClassContext.apply(TargetClassContext.java:421)
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.transformer.TargetClassContext.applyMixins(TargetClassContext.java:403)
[19/11/2021 14:27:01 PM] at org.spongepowered.asm.mixin.transformer.MixinProcessor.applyMixins(MixinProcessor.java:363)
[19/11/2021 14:27:01 PM] ... 24 more

Reproduce
Steps to reproduce the behavior: Install Server�Core and Clumps along with the Fabric API, attempt to launch game

Expected behavior
Game should launch

Versions -Latest is not a version!
ServerCore: 1.2.4-1.17.1
Minecraft: 1.17.1

Mod incompatibilities
Clumps

commented

Both mods more or less do similar things when merging experience orbs, and both (essentially) overwrite the same method, causing incompatibility issues even if it didn't crash.

ServerCore already handles the vast majority of experience orb lag, when configured correctly - reducing the amount of experience orbs in one place by a lot. You can also adjust the merging radius of xp orbs alongside it.

Which version of SC did you use before, where this didn't happen?

Edit: If you were using a version before V1.2.4 on a client, SC would not initialize at all.
Which might explain why it "worked" on older versions of this mod - even though it simply just acted as if it wasn't installed.

commented

I believe carpet's combineXPOrbs should work just fine, as long as the option fast_xp_merging is disabled.
If carpet's implementation were to actually conflict with SC's, the server wouldn't be able to boot up in the first place.

To completely fix this issue I'd have to make a separate mixin config so you can turn off specific patches, so they don't conflict with other mods. I'll consider doing that in the future, but for now I don't really have much time.

commented

Both mods more or less do similar things when merging experience orbs, and both (essentially) overwrite the same method, causing incompatibility issues even if it didn't crash.

given that, I would expect issues with Carpet's combineXPOrbs if you want to list that as incompatibility as well

commented

I'll backport this compatibility change to 1.17 later. It should work just fine though, even with clumps installed (which seems to work just fine on 1.18 pre-6 aswell). Do note that I'd recommend keeping fast_xp_merging set to the default (false) when using other mods or settings that modify experience orbs in any way.

commented

Just so you're aware @Wesley1808, your implementation of exp merging destroys vanilla mending.

Which as you can see:
https://github.com/jaredlll08/Clumps/issues?q=is%3Aissue+is%3Aclosed+mending
is a rather big issue for players apparently.

commented

Just so you're aware @Wesley1808, your implementation of exp merging destroys vanilla mending.

Which as you can see: https://github.com/jaredlll08/Clumps/issues?q=is%3Aissue+is%3Aclosed+mending is a rather big issue for players apparently.

Hmm... that is definitely a pretty big issue. Do you happen to know what causes this? I'd honestly be more than happy to just drop the xp merging part of the patch and leave that for more popular mods (like carpet for example, which has essentially the same result but hopefully shouldn't have this issue).

This patch is already nearly unnecessary due to a lot of other mods doing pretty much the same thing, if it takes a ton of time to maintain and test vanilla mechanics on it I don't think its worth keeping around.

Edit: I've done some testing on this and mending appears to give the same results as vanilla rather consistently...
Was there any other point about mending you were talking about, that this implementation destroys?
Because it doesn't seem like there is an issue, at least in 1.18 rc-3.
Although I doubt much changed between 1.17 and 1.18 in terms of mending.

commented

Imo, just disabling the max xp absorption rate is much cleaner, as we are changing max xp/second anyways. https://github.com/gnembon/fabric-carpet/wiki/Current-Available-Settings#xpnocooldown

The new solution I described (and committed) no longer changes max pickup per/s. The main goal is to reduce the amount of experience orbs in all cases, not just when a player is there to pick them up (besides, as you listed, carpet already does exactly what you described).

commented
commented

Was there any other point about mending you were talking about, that this implementation destroys?

After further investigation, it seems I was a bit harsh using the word "destroys" (sorry about that), but your mod definitely has differences compared to vanilla.

So from what I understand from your mod, if the feature is enabled you increment the value, instead of the count, so you will end up with an orb with {Value: 500, Count: 1}.

One of the issues is that you don't mixin into everywhere, so the count is still incremented by other values.
After throwing some experience on the ground for a bit:

image

I now have an orb with {Value: 104, Count: 5}.

Killing the ender dragon spawns an orb with {Value: 617, Count: 1}:

image

I copied the data so I can experiment more, so the UUID will change, in screenshots, but my summoning command is just:

/summon minecraft:experience_orb 20.00 62 2 {Health: 5s, Invulnerable: 0b, Air: 300s, OnGround: 1b, Count: 1, PortalCooldown: 0, FallDistance: 0.0f, Fire: -1s, Value: 617s, Age: 1000s}

So trapping that orb and having a command block spawn bottles of enchanting on it:

/summon minecraft:experience_bottle 20.00 62 2

At some point you end up with an orb that looks like this:
image

So the original orb merged with an orb that had {Value: 3, Count: 1}, so now it has {Value: 620, Count: 2}, effectively doubling that orbs experience output (should be able to repair 2 items with 620xp worth of mending for example / give the player 1240 experience).

Due to how orbs work, only merging if they have a "good" id (when the difference of the orbs ids % 40 == 0), it isn't something that can be reproduced reliably, but a player can just kill the dragon to spawn orbs with a high value and then dump bottles of enchanting onto the orbs, now they have the potential to create massive experience orbs from what is relatively very little experience.

When orbs are merged, their age also gets reset, so a player who knows what they are doing could stockpile bottles of enchanting before killing the dragon and come prepared.

(like carpet for example, which has essentially the same result but hopefully shouldn't have this issue).

Just confirmed, carpet very much has the initial issue that I ran into with Clumps, although it isn't as bad for them since they don't merge orbs instantly, but if you let them clump into a single orb, then the issue is there.

The whole issue is that vanilla keeps a count of how many orbs they have merged together, and then instead of killing the orb, they just:

  1. Use the value of the experience to apply mending or give the experience to the player
  2. --count;
  3. if count == 0, kill the orb

This works great for vanilla because they only merge orbs of the same value.

With Carpet however, if someone throws a bottle of enchanting onto an orb with: {Value: 617, Count: 1}, it will create an orb with {Value: 617, Count: 2}, doubling the experience.

This does actually go the other way around as well, depending on how they determine which orb to use (if they use the orb that has existed longest), for example, if you killed an Enderman underneath where the dragon dies, it's xp with {Value: 3, Count: 1} will sit there, then when you kill the dragon and it drops an orb with {Value: 617, Count: 1}, that new orb will merge with the old orb, causing: {Value: 3, Count: 2}, losing out on 614 experience.

As far as I'm aware, Clumps is the only mod that actually solves the issue and gives the correct amount of experience in all cases.

commented

Thanks a lot for the investigation on the matter! That does make a lot of sense now that I think of it...

I think what I'll do is make it so its more in line with vanilla, not merging XP orbs of different values as that seems to cause all sorts of weird issues with the count attribute. Instead, making it more likely for xp orbs to merge normally still achieves the same goal (reduce xp orb counts by a large amount) without breaking mending or xp orb sizes.

Which should be:

  • Much more compatible with other mods messing with xp orb counts / values.
  • Doesn't make xp orbs faster to pickup than vanilla (but you can still use other mods that allow for this).

This would be done by (optionally) modifying the value that decides when an xp orb can merge (or has a 'good' id), which is 40 in vanilla. I don't think that should cause any issues.

commented

That is the solution that https://www.curseforge.com/minecraft/mc-mods/fat-experience-orbs used and I don't believe they had any issues.