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

[Bug Fix] ModernAAScalingEnabled() Calculation Error #4469

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

carolus21rex
Copy link
Contributor

@carolus21rex carolus21rex commented Sep 11, 2024

Description
This PR fixes an issue where the AA experience scaling only considered unspent AAs, leading to incorrect scaling. The old implementation based the scaling on unspent AAs, meaning if a player had 2000 spent AAs and 1 unspent AA, the scaling would incorrectly be based only on the 1 unspent AA instead of the total of 2001 AAs.

Here’s a log output of the issue before the fix, from the custom code inside the ModernAAScalingEnabled function:

[Wed Sep 11 14:10:19 2024] [AA] [ScaleAAXPBasedOnCurrentAATotal] AA Experience Calculation: add_aaxp = 660796, Base Bonus = 256.000000, Half-Life = 64.000000, Minimum Bonus = 1.000000, Earned AA = 1, Calculated Bonus = 253.242371

The custom logic looks like this:

uint64 totalWithExpMod = add_aaxp;
if (RuleB(AA, EnableLogrithmicClasslessAABonus)) {
    float base_bonus = RuleR(AA, InitialLogrithmicClasslessAABonus);
    float half_life = RuleR(AA, HalfLifeLogrithmicClasslessAABonus);
    float min_bon = RuleR(AA, MinimumLogrithmicClasslessAABonus);
    float bonus_expon = earnedAA / half_life;

    float bonus = base_bonus * std::pow(0.5, bonus_expon);
    Log(Logs::General, Logs::AA, 
        "AA Experience Calculation: add_aaxp = %d, Base Bonus = %f, Half-Life = %f, Minimum Bonus = %f, Earned AA = %d, Calculated Bonus = %f", 
        add_aaxp, base_bonus, half_life, min_bon, earnedAA, bonus);
    
    if (bonus < min_bon) bonus = min_bon;

    totalWithExpMod = (uint64)(totalWithExpMod * bonus);
}

After the fix, the experience calculation properly considers the total number of AAs (spent + unspent). The updated log is as follows:

[Wed Sep 11 14:30:11 2024] [AA] [ScaleAAXPBasedOnCurrentAATotal] AA Experience Calculation: add_aaxp = 660796, Base Bonus = 256.000000, Half-Life = 64.000000, Minimum Bonus = 1.000000, Earned AA = 6245, Calculated Bonus =1.000000

This log reflects the intended behavior, where AA scaling is based on the total number of AAs.

Motivation
The bugfix ensures the correct scaling of AA experience by taking into account both spent and unspent AAs. Without this change, the AA experience calculation would disproportionately favor players with very few unspent AAs.

Type of change
Bug fix (non-breaking change that resolves an incorrect behavior)
Related Issues
Fixes # (issue link, if available)

Testing
Please include any test information and evidence, including logs or screenshots that validate the correct behavior after this fix.

Clients tested:
Describe client environments or versions tested.
Logs:
Please attach or describe any relevant logs that verify the correct functionality of the fix.

Checklist
I have tested my changes.
I have performed a self-review of my code, ensuring variable and function names are clear and comments are used appropriately.
I have updated relevant documentation (if applicable).
I accept full responsibility for the impact of my changes.
If applicable, I have tested the database changes locally and updated version.h to reflect the new CURRENT_BINARY_DATABASE_VERSION.

Current version only looks at your unspent AAs, meaning if you have 2000 spent AAs and 1 unspent AA, your scaling will be based on the 1 unspent AA instead of the 2001 total AA.

Here's the original log which is custom code found in the ModernAAScalingEnabled function:

[Wed Sep 11 14:10:19 2024] [AA] [ScaleAAXPBasedOnCurrentAATotal] AA Experience Calculation: add_aaxp = 660796, Base Bonus = 256.000000, Half-Life = 64.000000, Minimum Bonus = 1.000000, Earned AA = 1, Calculated Bonus = 253.242371

Custom code looks like this:

uint64 totalWithExpMod = add_aaxp;
	if (RuleB(AA, EnableLogrithmicClasslessAABonus)) {
		float base_bonus = RuleR(AA, InitialLogrithmicClasslessAABonus);
		float half_life = RuleR(AA, HalfLifeLogrithmicClasslessAABonus);
		float min_bon = RuleR(AA, MinimumLogrithmicClasslessAABonus);
		float bonus_expon = earnedAA / half_life;

		float bonus = base_bonus * std::pow(0.5, bonus_expon);
		Log(Logs::General,
			Logs::AA,
			"AA Experience Calculation: add_aaxp = %d, Base Bonus = %f, Half-Life = %f, Minimum Bonus = %f, Earned AA = %d, Calculated Bonus = %f",
			add_aaxp, base_bonus, half_life, min_bon, earnedAA, bonus);

		if (bonus < min_bon) bonus = min_bon;

		totalWithExpMod = (uint64)(totalWithExpMod * bonus);
	}

After the fix, the log becomes:

[Wed Sep 11 14:10:19 2024] [AA] [ScaleAAXPBasedOnCurrentAATotal] AA Experience Calculation: add_aaxp = 660796, Base Bonus = 256.000000, Half-Life = 64.000000, Minimum Bonus = 1.000000, Earned AA = 1, Calculated Bonus = 253.242371

Which is much closer to the expected behavior
@Kinglykrab Kinglykrab changed the title Update exp.cpp ModernAAScalingEnabled() [Bug Fix] ModernAAScalingEnabled() Calculation Error Sep 11, 2024
@Kinglykrab Kinglykrab merged commit 913c5da into EQEmu:master Sep 11, 2024
1 check passed
Kinglykrab added a commit that referenced this pull request Sep 13, 2024
### Code

* Add IsCloseToBanker method ([#4462](#4462)) @Akkadius 2024-08-27

### Feature

* Add Rule to Limit Task Update Messages ([#4459](#4459)) @Kinglykrab 2024-08-28
* Allow NPCs to cast Sacrifice ([#4470](#4470)) @fuzzlecutter 2024-09-12
* Lazy Load Bank Contents ([#4453](#4453)) @catapultam-habeo 2024-08-27

### Fixes

* Add RULE_STRING to RuleManager::ResetRules ([#4467](#4467)) @Kinglykrab 2024-09-07
* Fix Bard Effect in Migration 9237 ([#4468](#4468)) @Kinglykrab 2024-09-09
* ModernAAScalingEnabled() Calculation Error ([#4469](#4469)) @carolus21rex 2024-09-11

### Performance

* Move Discipline Loading to Client::CompleteConnect() ([#4466](#4466)) @Kinglykrab 2024-09-09

### Rules

* Add a Bandolier Swap Delay Rule ([#4465](#4465)) @Kinglykrab 2024-09-08
@Kinglykrab Kinglykrab mentioned this pull request Sep 13, 2024
Akkadius pushed a commit that referenced this pull request Sep 13, 2024
* [Release] 22.55.2

### Code

* Add IsCloseToBanker method ([#4462](#4462)) @Akkadius 2024-08-27

### Feature

* Add Rule to Limit Task Update Messages ([#4459](#4459)) @Kinglykrab 2024-08-28
* Allow NPCs to cast Sacrifice ([#4470](#4470)) @fuzzlecutter 2024-09-12
* Lazy Load Bank Contents ([#4453](#4453)) @catapultam-habeo 2024-08-27

### Fixes

* Add RULE_STRING to RuleManager::ResetRules ([#4467](#4467)) @Kinglykrab 2024-09-07
* Fix Bard Effect in Migration 9237 ([#4468](#4468)) @Kinglykrab 2024-09-09
* ModernAAScalingEnabled() Calculation Error ([#4469](#4469)) @carolus21rex 2024-09-11

### Performance

* Move Discipline Loading to Client::CompleteConnect() ([#4466](#4466)) @Kinglykrab 2024-09-09

### Rules

* Add a Bandolier Swap Delay Rule ([#4465](#4465)) @Kinglykrab 2024-09-08

* 22.56.0
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.

2 participants