Better Trims

Better Trims

4M Downloads

[Bug Report] server & client crash at start up with latest update

dadoirie opened this issue ยท 11 comments

commented

as the title says

here is the server log: https://mclo.gs/PxAJ5AX
client log: https://mclo.gs/G0sNgVJ

looks like Friends&Foes has a mixin with same priority as upon deactivating F&F this issue (crash) doesn't occur.
I will link this post in the Friends & Foes discord. Hope this helps

commented

wow - nice one - fix and compat with F&F ๐Ÿ‘
working - nice & thanks

commented

Yeah that's a compatibility issue, I'll look into it

commented

Well looks like, F&F overrides so many methods all over the place making their mixins very invasive (bad practice as it makes mods incompatible). I'll patch it anyway but I wish some developers would use mixins properly.

commented

can't say much as I'm just trying to make a modpack for my small whitelisted server - any critical methods?

commented

nah, just minor rant lol

commented

Fixed in 1.2.1

commented

I will try to do some proper mixins with next release of F&F.

commented

@Faboslav Nice, basically, rule of thumb is don't Override or Overwrite if possible, the exception being overriding from supermixins which is fine.

An example of this in F&F, the one that caused this incompatibility is this method:

@Override
public void initGoals() {
    super.initGoals();
    this.goalSelector.add(0, new SwimGoal(this));
    this.goalSelector.add(1, new SpellcastingIllagerEntity.LookAtTargetGoal(this));
    this.goalSelector.add(2, new FleeEntityGoal(this, IronGolemEntity.class, 8.0F, 0.6, 1.0));
    if (!FriendsAndFoes.getConfig().enableIllusioner || !this.isIllusion()) {
        this.goalSelector.add(3, BlindTargetGoalFactory.newBlindTargetGoal((IllusionerEntity)this));
    }

    this.goalSelector.add(4, new BowAttackGoal(this, 0.5, 20, 15.0F));
    this.goalSelector.add(5, new WanderAroundGoal(this, 0.6));
    this.goalSelector.add(6, new LookAtEntityGoal(this, PlayerEntity.class, 3.0F, 1.0F));
    this.goalSelector.add(7, new LookAtEntityGoal(this, MobEntity.class, 8.0F));
    this.targetSelector.add(1, (new RevengeGoal(this, new Class[]{RaiderEntity.class})).setGroupRevenge(new Class[0]));
    this.targetSelector.add(2, (new ActiveTargetGoal(this, PlayerEntity.class, true)).setMaxTimeWithoutVisibility(300));
    this.targetSelector.add(3, (new ActiveTargetGoal(this, IronGolemEntity.class, false)).setMaxTimeWithoutVisibility(300));
    this.targetSelector.add(4, (new ActiveTargetGoal(this, MerchantEntity.class, false)).setMaxTimeWithoutVisibility(300));
}

By overriding you're making any mixin that tries to target the original method fail, instead, you should try to be finer with your approach, such that you're only affecting exactly what you need and nothing else.

Just for the sake of demonstration, I wanted to see what a fine-tuning approach would look like for this method and this does the same thing but doesn't cause incompatibilities with other mods targetting the initGoals method out the gate:

@ModifyArg(
        method = "initGoals",
        at = @At(
                value = "INVOKE",
                target = "Lnet/minecraft/entity/ai/goal/GoalSelector;add(ILnet/minecraft/entity/ai/goal/Goal;)V",
                ordinal = 1
        ),
        slice = @Slice(
                from = @At(
                        value = "INVOKE",
                        target = "Lnet/minecraft/entity/mob/IllusionerEntity$GiveInvisibilityGoal;<init>(Lnet/minecraft/entity/mob/IllusionerEntity;)V"
                )
        ),
        index = 1
)
private Goal replaceBlindTargetGoal(Goal original) {
    return BlindTargetGoalFactory.newBlindTargetGoal((IllusionerEntity)(Object)this);
}

@WrapWithCondition(
        method = "initGoals",
        at = @At(
                value = "INVOKE",
                target = "Lnet/minecraft/entity/ai/goal/GoalSelector;add(ILnet/minecraft/entity/ai/goal/Goal;)V",
                ordinal = 1
        ),
        slice = @Slice(
                from = @At(
                        value = "INVOKE",
                        target = "Lnet/minecraft/entity/mob/IllusionerEntity$GiveInvisibilityGoal;<init>(Lnet/minecraft/entity/mob/IllusionerEntity;)V"
                )
        )
)
private boolean shouldReplaceBlindTargetGoal(GoalSelector instance, int priority, Goal goal) {
    return !FriendsAndFoes.getConfig().enableIllusioner || !this.isIllusion();
}

@ModifyArg(
        method = "initGoals",
        at = @At(
                value = "INVOKE",
                target = "Lnet/minecraft/entity/ai/goal/GoalSelector;add(ILnet/minecraft/entity/ai/goal/Goal;)V",
                ordinal = 2
        ),
        slice = @Slice(
                from = @At(
                        value = "INVOKE",
                        target = "Lnet/minecraft/entity/ai/goal/SwimGoal;<init>(Lnet/minecraft/entity/mob/MobEntity;)V"
                )
        ),
        index = 1
)
private Goal replaceWithFleeGoal(Goal original) {
    return new FleeEntityGoal<>((IllusionerEntity)(Object)this, IronGolemEntity.class, 8.0F, 0.6, 1.0);
}

It's more code to write but it improves compatibility.

The output of this method with fine-tuning and my own mixin:

public void initGoals() {
    super.initGoals();
    this.goalSelector.add(0, new SwimGoal(this));
    this.goalSelector.add(1, new SpellcastingIllagerEntity.LookAtTargetGoal(this));
    GoalSelector var10000 = this.goalSelector;
    Goal injectorAllocatedLocal1 = new GiveInvisibilityGoal(this);
    var10000.add(4, this.modify$bbf000$bettertrims$replaceWithFleeGoal(injectorAllocatedLocal1));
    // this.goalSelector.add(4, new FleeEntityGoal(this, IronGolemEntity.class, 8.0F, 0.6, 1.0));
    var10000 = this.goalSelector;
    Goal injectorAllocatedLocal1 = new BlindTargetGoal(this);
    Goal injectorAllocatedLocal3 = this.modify$bbf000$bettertrims$replaceBlindTargetGoal(injectorAllocatedLocal1);
    int injectorAllocatedLocal2 = 5;
    Goal injectorAllocatedLocal1 = var10000;
    if (this.wrapWithCondition$bbf000$bettertrims$shouldReplaceBlindTargetGoal(injectorAllocatedLocal1, injectorAllocatedLocal2, injectorAllocatedLocal3)) {
    // if (!FriendsAndFoes.getConfig().enableIllusioner || !this.isIllusion()) {
        injectorAllocatedLocal1.add(injectorAllocatedLocal2, injectorAllocatedLocal3);
        // this.goalSelector.add(5, BlindTargetGoalFactory.newBlindTargetGoal((IllusionerEntity)this)); 
    }

    this.goalSelector.add(6, new BowAttackGoal(this, 0.5, 20, 15.0F));
    this.goalSelector.add(8, new WanderAroundGoal(this, 0.6));
    this.goalSelector.add(9, new LookAtEntityGoal(this, PlayerEntity.class, 3.0F, 1.0F));
    this.goalSelector.add(10, new LookAtEntityGoal(this, MobEntity.class, 8.0F));
    this.targetSelector.add(1, (new RevengeGoal(this, new Class[]{RaiderEntity.class})).setGroupRevenge(new Class[0]));
    var10000 = this.targetSelector;
    Goal injectorAllocatedLocal1 = (new ActiveTargetGoal(this, PlayerEntity.class, true)).setMaxTimeWithoutVisibility(300);
    var10000.add(2, this.modify$bbg000$bettertrims$replaceWithConditionalTargetGoal(injectorAllocatedLocal1));
    this.targetSelector.add(3, (new ActiveTargetGoal(this, MerchantEntity.class, false)).setMaxTimeWithoutVisibility(300));
    this.targetSelector.add(3, (new ActiveTargetGoal(this, IronGolemEntity.class, false)).setMaxTimeWithoutVisibility(300));
}
commented

I know all this, thanks for the demonstration anyway, I was probably just lazy in some cases. Is there a way to target methods like tick which are not present in the target class? I think this was the main problem behind that, so i used Overrides for the whole mixin.

commented

Yes, you can use supermixins for that.
Create a mixin targetting the parent class where the tick method is present, then inject into that method, at head or tail, or wherever you want the call to be. Then make the injection method protected with an empty body and then have your target class mixin extend the parent class mixin and override the injected method there.
When dealing with multiple layers of inheritance you may have to create multiple supermixins for the classes you're extending.

Here is an example in this project:
Target class:
https://github.com/Benjamin-Norton/BetterTrims/blob/main/src/main/java/com/bawnorton/bettertrims/mixin/friendsandfoes/IceologerMixin.java#L36-L39
Parent class:
https://github.com/Benjamin-Norton/BetterTrims/blob/main/src/main/java/com/bawnorton/bettertrims/mixin/LivingEntityMixin.java#L64-L68

commented

Great then, thank you for your time, will try to correct all overrides :).