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

[Scaling/Bug Fix] Scaling where min and max damage was bugged #3514

Merged
merged 2 commits into from
Aug 1, 2023

Conversation

noudess
Copy link
Contributor

@noudess noudess commented Jul 28, 2023

This is a refinement of a fix by @Kinglykrab in the following PR:

#3351

@Kinglykrab addressed an issue where min_dmg was 0 in db and max_dmg was not. This caused the npc_scale_manager to only scale the min_dmg side, which can result in the min_dmg being bigger than the max_dmg.

This PR fixes a side effect of that PR wherein when min_dmg and max_dmg are both 0 in the db, setting min_dmg 1st results in min_dmg getting put into max_dmg (due to his attempts to keep max_dmg greater than min_dmg inside of ModifyNPCStat()) at all times.

My belief is that if _either min or max dmg is 0 in the db, both need to be processed. His earlier PR assures that, so long as min_dmg is 0, but not when min_dmg is non-zero and max_dmg is 0. Admittedly an unlikely case, but it simplifies the code tremendously to simply scale both if either is 0.

This was discovered because several special attacks like bash and kick (for scaled mobs) were saying the opponent was invulnerable. This was due to math from the changes to npc.cpp which left min_damage (GetMinDamage()) negative and sometimes that value was -5 (DMG_INVULNERABLE) or -6 (DMG_RUNE).

eqlog_Ebola_Rolath.txt:[Tue Jun 13 15:14:40 2023] A wan ghoul knight tries to bash YOU, but are INVULNERABLE!
eqlog_Ebola_Rolath.txt:[Tue Jun 13 15:15:12 2023] A wan ghoul knight tries to bash YOU, but are INVULNERABLE!
eqlog_Ebola_Rolath.txt:[Tue Jun 13 15:49:58 2023] A wan ghoul knight tries to bash YOU, but are INVULNERABLE!
eqlog_Ebola_Rolath.txt:[Tue Jun 13 16:06:20 2023] A zol ghoul knight tries to bash YOU, but are INVULNERABLE!

@noudess noudess requested review from Kinglykrab and Aeadoin July 28, 2023 20:25
@noudess
Copy link
Contributor Author

noudess commented Jul 28, 2023

Please hold merge even if approved until Sunday as I'll be out of town tomorrow and want to be available if you have any issues.

@Akkadius
Copy link
Member

I'm for this change but we still need to be clamping values

@noudess
Copy link
Contributor Author

noudess commented Jul 29, 2023

I'm for this change but we still need to be clamping values

Can you explain? If you mean in ModifyNPCStat() it would have to differ from what I removed. The existing code there to make sure that min < max actually throws min away completely in this case:

min_dmg and max_dmg 0 in db
scale manager sets min, and then max

The setting of min actually puts min into max and min never gets touched again.

Leaving out the code in ModifyNPCStat works fine since npc_scale_manager always sets both.

But yeah, some quest could only set one and they could be hosed. (always been the case).

Really we should not allow min or max to be changed individually - one function to set both at once before the math... But I don't want to break existing scripts.

@noudess
Copy link
Contributor Author

noudess commented Jul 31, 2023

I took a shot at what @Akkadius wants. This should make sure min is never > max and max is never < min.

@Akkadius
Copy link
Member

Akkadius commented Aug 1, 2023

I took a shot at what @Akkadius wants. This should make sure min is never > max and max is never < min.

Thank you sir

@Akkadius Akkadius merged commit 6cff433 into EQEmu:master Aug 1, 2023
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.

3 participants