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

Only deduct energy when elemental burst actually fires #2424

Merged
merged 1 commit into from
Nov 5, 2023

Conversation

longfruit
Copy link
Contributor

Description

Elemental burst (Q) can misfire occasionally if done when dashing, doing elemental skill (E), or switching avatars. The client would still send EvtDoSkillSuccNotify, but the burst may not actually happen, and GC currently clears energy whether the burst happened or not.

Fix this by checking AbilityInvocationsNotify packets from the client. On successful bursts, there will be packets with information specific to the burst, like setting resistance/invincible state, AvatarSkillStart action. These can be used to decide when to clear energy and set invulnerability.

Also fix Wanderer's energy not clearing if bursting while floating.


This fix could use more testing. I can't exhaustively test through all characters and scenarios. But at least my quick swap teams in abyss no longer have any burst issue.

Preview:

  • Faruzan: Works fine
  • Bennett: E into Q causes misfire
  • Yelan: E into Q causes misfire
  • Wanderer: E into Q doesn't clear energy
Before After
gif gif

Issues fixed by this PR

#1248

Type of changes

  • Bug fix
  • New feature
  • Enhancement
  • Documentation

Checklist:

  • My code follows the style guidelines of this project
  • My pull request is unique and no other pull requests have been opened for these changes
  • I have read the Contributing note and Code of conduct
  • I am responsible for any copyright issues with my code if it occurs in the future.

@KingRainbow44
Copy link
Member

ITS THE BUG FIX

Copy link
Member

@KingRainbow44 KingRainbow44 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug fix for a bug from a year ago pog

@KingRainbow44 KingRainbow44 merged commit d224178 into Grasscutters:development Nov 5, 2023
2 checks passed
@longfruit longfruit deleted the missed-burst branch November 5, 2023 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Energy will be cleared but elemental burst will not be released
2 participants