[Feature Request] allow CriticalHitEvent to not disable sweep when set to criticalHit

[lcy0x1]

Vanilla logic:

1. check if player can perform critical hit (flag1)
2. apply critical hit bonus
3. check if player can sweep attack (flag2), which requires player to not be able to critical hit in vanilla sense

Current NeoForge logic:
Directly modifies flag1 when mods enables critical hit. This disables sweep attack unintentionally. The suggested behavior is to use the vanilla critical hit flag for sweep attack check, which means changing the following code:

var critEvent = net.neoforged.neoforge.common.CommonHooks.fireCriticalHit(this, target, flag1, flag1 ? 1.5F : 1.0F);
flag1 = critEvent.isCriticalHit();
if (flag1) {
    f *= critEvent.getDamageMultiplier();
}

float f3 = f + f1;
boolean flag2 = false;
double d0 = (double)(this.walkDist - this.walkDistO);
if (flag4 && !flag1 && !flag && this.onGround() && d0 < (double)this.getSpeed()) {
    ItemStack itemstack1 = this.getItemInHand(InteractionHand.MAIN_HAND);
    flag2 = itemstack1.canPerformAction(net.neoforged.neoforge.common.ItemAbilities.SWORD_SWEEP);
}

to

var critEvent = net.neoforged.neoforge.common.CommonHooks.fireCriticalHit(this, target, flag1, flag1 ? 1.5F : 1.0F);
boolean disableSweep = flag1;
flag1 = critEvent.isCriticalHit();
if (flag1) {
    f *= critEvent.getDamageMultiplier();
}

float f3 = f + f1;
boolean flag2 = false;
double d0 = (double)(this.walkDist - this.walkDistO);
if (flag4 && !disableSweep && !flag && this.onGround() && d0 < (double)this.getSpeed()) {
    ItemStack itemstack1 = this.getItemInHand(InteractionHand.MAIN_HAND);
    flag2 = itemstack1.canPerformAction(net.neoforged.neoforge.common.ItemAbilities.SWORD_SWEEP);
}

Alternative solutions such as adding whether to disable sweep as part of CriticalHitEvent would also address this issue.

[ChampionAsh5357]

I feel like I'm misunderstanding the issue. If a critical hit occurs, then the sweep attack is disabled, as that is vanilla behavior.

Is this supposed to be a bug report, or a feature request to add the ability to crit and handle a sweep attack? The way it's currently being described is as a bug when it is not.

[lcy0x1]

This is a feature request to allow crit to not disable sweep in some cases. I will edit the title and description

[neoforged-releases]

🚀 This issue has been resolved in NeoForge version 21.1.61, as part of #1496.