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 to pollution by stat modifications in newcharacter. #64296

Merged
merged 1 commit into from
Mar 17, 2023

Conversation

Wyghab
Copy link
Contributor

@Wyghab Wyghab commented Mar 16, 2023

Summary

Bugfixes "Fixes the situation where stat mods affect the displayed effects of your base stats."

Purpose of change

Fixes #64280

Describe the solution

avatar::reset_stats() is accumulating stat mods each passthrough because character::reset_bonuses() is never called on the object. reset_stats() should never be called without first resetting bonuses as it can reapply additional bonuses without clearing the last ones. Ideally character::reset() should be used in these situations? Regardless, I find that no mod bonuses should affect the displayed effects of your base stats, hence I've placed reset_bonuses() after reset_stats() so it only displays the effect of your base stats, though it could still be polluted by other means as I've not investigated the functions that calculate the bonuses that are affected by the stats.

Describe alternatives you've considered

Calling reset() instead of reset_stats() or extracting the calculations used to determine stat effects and calculate the bonuses in-place instead of relying on the avatar object.

Testing

Tested #64280, while also including the mutation "compound eyes" as that mutation will continually mod perception by 2 while equipped in the same manner as StatsThroughSkills or the BMI rework.

Additional context

This could likely need some refactoring later. Since this fix is closer to the core issue than #64270 , maybe the fixes from there can be removed safely. If there are other places in code that calls reset_stats() haphazardly it could cause issues in other places.

@github-actions
Copy link
Contributor

You are creating a pull request with the master branch as the head branch. This is likely a mistake unless you really know what you are doing. You may read https://docs.github.com/en/get-started/quickstart/contributing-to-projects#creating-a-branch-to-work-on for a typical workflow of contributing to a project on GitHub.

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` Character / World Generation Issues and enhancements concerning stages of creating a character or a world <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Mar 16, 2023
@dseguin dseguin merged commit c4e45f8 into CleverRaven:master Mar 17, 2023
@NetSysFire NetSysFire added Info / User Interface Game - player communication, menus, etc. 0.H backport candidate labels Mar 27, 2023
@NetSysFire
Copy link
Member

Feels like a good backport candidate.

@Wyghab
Copy link
Contributor Author

Wyghab commented Mar 28, 2023

Feels like a good backport candidate.

Just to keep the thread of change going for eventual refactor, this issue was caused by #57884. That PR assumed that using the avatar class to calculate everything and using reset_stats to update it to be less bug-prone, which seems untrue. So using the stat_max to calculate bonuses seem to be the way forward if what we want is for the stat panel to only care for your base stats when showing their effects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.H backport candidate astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Character / World Generation Issues and enhancements concerning stages of creating a character or a world Info / User Interface Game - player communication, menus, etc. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

More character creation STATS panel bugs.
3 participants