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, adding SweepAttackEvent #1496

Merged
merged 10 commits into from
Sep 24, 2024

Conversation

lcy0x1
Copy link
Contributor

@lcy0x1 lcy0x1 commented Aug 29, 2024

Rework of PR #1314
Closes Issue #1313

This PR does the following:

  1. Adds disableSweep in CriticalHitEvent to track if sweep attack should be disabled. Legacy methods that enables critical hit will disable sweep attack.
  2. Adds a new method in CriticalHitEvent to enable critical hit without disabling sweep attack.
  3. Adds a new method to set if the critical hit should disable sweep attack.
  4. Adds SweepAttackEvent to determine if player should sweep attack, considering both vanilla conditions and CriticalHitEvent.disableSweep()

The vanilla logic for critical hit and sweep attack is:

  • Check critical hit conditions, including:
    • attack is full strength
    • player is not sprinting
    • player is falling
    • player is not on ground
    • player is not on climbable blocks
    • player is not in water
    • player is not blinded
    • player is not a passenger
    • target is a living entity
  • Check sweep attack conditions, including
    • attack is full strength
    • player is not sprinting
    • player is not reaching its full speed
    • player is on ground
    • this attack is not critical hit
  • Lastly, check if the weapon support critical hit

This PR propose the following logic:

  • flag1 would represent all 9 vanilla critical hit conditions
  • allowSweep would represent first 4 vanilla sweep conditions (excluding critical hit check)
  • CriticalHitEvent determines if the attack would disable sweep
  • SweepAttackEvent determines if the attack would be a sweep attack, considering allowSweep and CriticalHit.disableSweep

@neoforged-pr-publishing
Copy link

  • Publish PR to GitHub Packages

@sciwhiz12 sciwhiz12 added enhancement New (or improvement to existing) feature or request 1.21.1 Targeted at Minecraft 1.21.1 labels Sep 1, 2024
@KnightMiner
Copy link
Contributor

I asked in the other PR, what is the behavior when that property is set on axes or other weapons with no sweep, I assume just ignored? Or am I able to force sweeps onto any weapon?

Personally I think it should just be ignored but wanted to double check so I know I dont have to deal with my weapons suddenly gaining sweep attacks.

@lcy0x1
Copy link
Contributor Author

lcy0x1 commented Sep 17, 2024

I asked in the other PR, what is the behavior when that property is set on axes or other weapons with no sweep, I assume just ignored? Or am I able to force sweeps onto any weapon?

Personally I think it should just be ignored but wanted to double check so I know I dont have to deal with my weapons suddenly gaining sweep attacks.

Per Shadow’s request, it allows bypassing item ability check. Do you think I should have enableSweep and enableSweepBypassItemAbilityCheck separately?

@Shadows-of-Fire Shadows-of-Fire merged commit 8ec872a into neoforged:1.21.x Sep 24, 2024
6 checks passed
@neoforged-releases
Copy link

🚀 This PR has been released as NeoForge version 21.1.61.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.21.1 Targeted at Minecraft 1.21.1 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