Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow setting if the critical hit should disable sweep attack in CriticalHitEvent #1314

Closed

Conversation

RaymondBlaze
Copy link
Contributor

Closes #1313.
Introduce a disableSweepAttack field to the CriticalHitEvent, equals to isVanillaCritical by default, matching vanilla behavior.
Now the sweep attack will only be disabled when isCriticalHit() and disableSweepAttack() both returns true.
This would change the behavior of existing subscriber of this event who set isCriticalHit to true, as now these modded critical would allow sweep attack when they are not vanilla critical. It's now up to these mod's decision to disable the sweep attack or not for their modded critical hit.

@neoforged-pr-publishing
Copy link

  • Publish PR to GitHub Packages

@Matyrobbrt Matyrobbrt added enhancement New (or improvement to existing) feature or request 1.21 Targeted at Minecraft 1.21 labels Jul 15, 2024
@Shadows-of-Fire
Copy link
Contributor

Would it make more sense to always check disableSweepAttack instead of requiring that the attack is a critical and disableSweepAttack is set to true? They seem like independent events to a degree.

@TelepathicGrunt
Copy link
Contributor

@RaymondBlaze Would you be able to respond to shadow's suggestion?

@KnightMiner
Copy link
Contributor

What is the behavior when that property is set on axes or other weapons with no sweep, I assume just ignored?

@Shadows-of-Fire
Copy link
Contributor

Superseded by #1496

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.21 Targeted at Minecraft 1.21 enhancement New (or improvement to existing) feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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