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

fix: correctly reflect damage #1941

Merged
merged 5 commits into from
Dec 7, 2023
Merged

fix: correctly reflect damage #1941

merged 5 commits into from
Dec 7, 2023

Conversation

un000000
Copy link
Contributor

@un000000 un000000 commented Nov 30, 2023

Description

Behaviour

Actual

Damaging enemy with type they reflect heals you for ~42 million hp if you are using ek/rp weapons.

Expected

Damaging enemy with type they reflect damages you according to reflect map when using any damage source.

Fixes #1719

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested

I equipped fire axe and bonked gazer spectre. This resulted me receiving about 450 damage from reflect, and spectre received 80 damage. Did same with royal star - this resulted in smaller values, correct nevertheless.

Test Configuration:

  • Server Version: up to commit ce7e977
  • Client: 13.21
  • Operating System: Win 10

src/game/game.cpp Outdated Show resolved Hide resolved
@un000000 un000000 changed the title fix: correctly reflect damage on ek/rp weapons fix: correctly reflect damage Dec 3, 2023
@un000000
Copy link
Contributor Author

un000000 commented Dec 3, 2023

I fixed problem mentined by @Schiffers in 5ad00a8. Now reflected damage is based on damage after mitigation (actual dmg monster was hit for).

I changed pr name accordingly.

@Schiffers
Copy link
Contributor

I fixed problem mentined by @Schiffers in 5ad00a8. Now reflected damage is based on damage after mitigation (actual dmg monster was hit for).

I changed pr name accordingly.

I've done some tests and works fine.

Copy link
Contributor

@Schiffers Schiffers left a comment

Choose a reason for hiding this comment

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

I've done another tests... now the reflect damage by agony damage don't work as before.
bug

works with monster with alot of hp, but with rats the damage don't apply.

@un000000
Copy link
Contributor Author

un000000 commented Dec 4, 2023

I've done another tests... now the reflect damage by agony damage don't work as before.

works with monster with a lot of hp, but with rats the damage don't apply.

This is because in the line below, there is a check that guarantees that the reflected damage isn't higher than attacker damage.

Earlier it used pre-mitigation damage. So when rat rolls say 2 damage, you can reflect damage lower or equal to 2 damage.

Now its calculated based on post-mitigation damage (actual health loss), so with crazy high level, and endgame armor, rat is always gonna hit you for 0 damage, thus no reflect damage will occur.

If it's not intended, then I would just remove "damage.secondary.value" from all three places where reflect is calculated.

damageReflected.primary.value = std::ceil(damage.secondary.value * secondaryReflectPercent / 100.) + std::max(-static_cast<int32_t>(std::ceil(attacker->getMaxHealth() * 0.01)), std::max(damage.secondary.value, -(static_cast<int32_t>(secondaryReflectFlat))));

Copy link

sonarqubecloud bot commented Dec 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@luan luan merged commit a21fe2b into opentibiabr:main Dec 7, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Monster reflection heals instead of dealing damage
4 participants