Stat Ratings Refactor #883
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PseudoStat representations so that they can be peer-reviewed before effort is spent on migrating the codebase to the new schema. On branch stats_refactor Changes to be committed: modified: proto/common.proto modified: proto/ui.proto modified: sim/core/stats/stats.go modified: ui/core/individual_sim_ui.tsx
NerdEgghead
requested review from
hillerstorm,
1337LutZ,
rosenrusinov,
raethkcj and
ToxicKevinFerm
1337LutZ
reviewed
Jul 29, 2024
messages. This enables a tighter proto Stat enum that contains only essential stats that need to be saved in database files or Encounter settings, while still retaining the benefits of StatDependency structs for tertiary stats in the back-end. On branch stats_refactor Changes to be committed: modified: proto/common.proto modified: sim/core/stats/stats.go
On branch stats_refactor Changes to be committed: modified: sim/core/stats/deps.go modified: sim/core/stats/stats.go
representation, for the sake of back-end migration convenience. On branch stats_refactor Changes to be committed: modified: proto/common.proto modified: sim/core/stats/deps.go modified: sim/core/stats/stats.go
On branch stats_refactor Changes to be committed: modified: sim/core/base_stats_auto_gen.go modified: tools/base_stats_parser.py
removed incorrect / unused constants for Cata. On branch stats_refactor Changes to be committed: modified: sim/core/base_stats_auto_gen.go modified: tools/base_stats_parser.py
explicit in the name. On branch stats_refactor Changes to be committed: modified: sim/core/stats/stats.go
On branch stats_refactor Changes to be committed: modified: proto/common.proto
units for consistency with the other stats changes. On branch stats_refactor Changes to be committed: modified: sim/core/stats/stats.go
debuffs provide this. On branch stats_refactor Changes to be committed: modified: sim/core/debuffs.go modified: sim/core/stats/stats.go
percentage units for consistency with other stats changes. On branch stats_refactor Changes to be committed: modified: sim/core/spell.go
order to avoid inconsistent interpretations of percentages in the codebase. On branch stats_refactor Changes to be committed: modified: sim/core/spell_outcome.go modified: sim/core/spell_result.go
entirely, as further inspection reveals that they are never used in Cata. On branch stats_refactor Changes to be committed: modified: sim/core/stats/stats.go
stats.ToFloatArray() to stats.ToProtoArray() for clarity, since SimStatsLen and ProtoStatsLen no longer need to match. Also fixed a bug in UnitStat.EqualsPseudoStat(), which may be utilized in future stat weights refactors. On branch stats_refactor Changes to be committed: modified: sim/core/stats/stats.go
and school-specific Hit / Crit percentages. Added utility function for importing tertiary stats that are stored as PseudoStats in UnitStats messages into back-end Stats arrays. Also added support for exporting these tertiary stats to UnitStats protos during character stats requests. The net impact of these utilities is that the proto Stat enum can be kept very lean, including only essential entries for database files and NPC target stats, without compromising the ability to pass tertiary stat data between the front-end and back-end code. On branch stats_refactor Changes to be committed: modified: sim/core/character.go modified: sim/core/target.go modified: sim/core/test_suite.go modified: sim/core/unit.go modified: sim/core/utils.go
functions, which are not used anywhere in the codebase. On branch stats_refactor Changes to be committed: modified: sim/core/test_utils.go
AddHighestStat() versions. On branch stats_refactor Changes to be committed: modified: sim/core/stats/stats.go modified: sim/core/unit.go
On branch stats_refactor Changes to be committed: modified: sim/core/avoid_dr.go modified: sim/core/buffs.go modified: sim/core/consumes.go modified: sim/core/database.go modified: sim/core/energy.go modified: sim/core/focus.go modified: sim/core/major_cooldown.go modified: sim/core/mana.go modified: sim/core/professions.go modified: sim/core/racials.go modified: sim/core/runic_power.go modified: sim/core/spell.go modified: sim/core/spell_mod.go modified: sim/core/spell_result.go modified: sim/core/statweight.go modified: sim/core/test_utils.go modified: sim/core/unit.go
On branch stats_refactor Changes to be committed: modified: sim/warrior/arms/talents.go modified: sim/warrior/fury/fury.go modified: sim/warrior/fury/glyphs.go modified: sim/warrior/fury/talents.go modified: sim/warrior/glyphs.go modified: sim/warrior/items.go modified: sim/warrior/protection/glyphs.go modified: sim/warrior/protection/protection.go modified: sim/warrior/protection/talents.go modified: sim/warrior/recklessness.go modified: sim/warrior/shield_block.go modified: sim/warrior/talents.go modified: sim/warrior/warrior.go
On branch stats_refactor Changes to be committed: modified: sim/druid/druid.go modified: sim/druid/ferocious_bite.go modified: sim/druid/force_of_nature.go modified: sim/druid/forms.go modified: sim/druid/items.go modified: sim/druid/lacerate.go modified: sim/druid/pulverize.go modified: sim/druid/rake.go modified: sim/druid/ravage.go modified: sim/druid/rip.go modified: sim/druid/savage_defense.go modified: sim/druid/starfire.go modified: sim/druid/talents.go modified: sim/druid/wrath.go
On branch stats_refactor Changes to be committed: modified: sim/warlock/demon_soul.go modified: sim/warlock/demonology/hand_of_guldan.go modified: sim/warlock/doomguard.go modified: sim/warlock/ebon_imp.go modified: sim/warlock/infernal.go modified: sim/warlock/pets.go modified: sim/warlock/talents_affliction.go modified: sim/warlock/talents_destruction.go
StatCapConfig proto messages without as much boilerplate. On branch stats_refactor Changes to be committed: modified: ui/core/components/suggest_reforges_action.tsx modified: ui/core/individual_sim_ui.tsx modified: ui/core/player.ts modified: ui/core/proto_utils/stats.ts modified: ui/druid/guardian/sim.ts modified: ui/mage/arcane/sim.ts modified: ui/mage/fire/sim.tsx modified: ui/rogue/assassination/sim.ts modified: ui/rogue/combat/sim.ts modified: ui/rogue/subtlety/sim.ts modified: ui/warlock/affliction/sim.ts modified: ui/warlock/demonology/sim.ts modified: ui/warrior/fury/sim.ts
to facilitate cleaning up this code. On branch stats_refactor Changes to be committed: modified: ui/core/components/suggest_reforges_action.tsx modified: ui/core/encounter.ts modified: ui/core/individual_sim_ui.tsx modified: ui/core/proto_utils/stats.ts
On branch stats_refactor Changes to be committed: modified: ui/paladin/protection/sim.ts
feature for Cata, it will need a full overhaul to look more like Suggest Reforges, so the old code is not useful. On branch stats_refactor Changes to be committed: deleted: ui/core/components/suggest_gems_action.ts
On branch stats_refactor Changes to be committed: modified: ui/core/components/inputs/buffs_debuffs.ts modified: ui/core/components/inputs/consumables.ts
On branch stats_refactor Changes to be committed: modified: ui/core/components/individual_sim_ui/reforge_summary.tsx
On branch stats_refactor Changes to be committed: modified: ui/core/components/gear_picker/gear_picker.tsx modified: ui/core/components/gear_picker/selector_modal.tsx
On branch stats_refactor Changes to be committed: modified: ui/core/components/exporters.tsx
On branch stats_refactor Changes to be committed: modified: ui/core/components/encounter_picker.ts
On branch stats_refactor Changes to be committed: modified: ui/core/components/character_stats.tsx modified: ui/core/proto_utils/stats.ts
display stats filtering to eliminate unnecessary creation of new UnitStat objects. On branch stats_refactor Changes to be committed: modified: ui/core/components/character_stats.tsx modified: ui/core/proto_utils/names.ts modified: ui/core/proto_utils/stats.ts
from version 1 to 2. On branch stats_refactor Changes to be committed: modified: proto/common.proto modified: sim/core/TestProtoVersioning.results modified: ui/core/proto_utils/stats.ts modified: ui/core/proto_utils/utils.ts
custom clone method for StatCap objects since structuredClone was not working, and (2) removed Spirit from configurable stat caps since it does not have a sensible Rating vs. Percent representation for visualization. On branch stats_refactor Changes to be committed: modified: ui/core/components/suggest_reforges_action.tsx modified: ui/core/proto_utils/stats.ts
they are never saved to local storage or links. On branch stats_refactor Changes to be committed: modified: proto/ui.proto modified: ui/core/individual_sim_ui.tsx
display of breakpoint EP values. On branch stats_refactor Changes to be committed: modified: ui/core/components/suggest_reforges_action.tsx modified: ui/core/proto_utils/stats.ts
|
Tested in Firefox/Edge (not Chrome because I don't want to migrate my current personal Sim settings) to see if old vs new sims would migrate. Tested most spec sims and encountered no issues. |
1337LutZ
approved these changes
Aug 16, 2024
showing only the percentage component for clarity. On branch stats_refactor Changes to be committed: modified: ui/core/components/character_stats.tsx
clarity. On branch stats_refactor Changes to be committed: modified: ui/core/components/suggest_reforges_action.tsx
ToxicKevinFerm
approved these changes
Aug 16, 2024
|
For the most part links / old data seems to work properly. There seems to be one "visual" bug only as the dps are the same. This seems only to occur after opening an import link from the "old" sim. i.E. - when loading the preset from within the new sim, the stat breakdown look good.
Unfortunatley I can not reliably reproduce this but I had a couple of occasions where when I use 'Use custom stat weights' for the reforger, after closing the dialog the numbers were suddenly crazy high and just a mess for my hit rating. Had to manualy reset them. Not sure if this has anything todo with this PR though 🤣 Also checked that breakpoints align with the haste shown in the character sheet with https://docs.google.com/spreadsheets/d/17cJJUReg2uz-XxBB3oDWb1kCncdH_-X96mSb0HAu4Ko/copy?gid=497073075#gid=497073075 as reference for shadow. When checking all buffs they align nicely. Reforging also seems to work properly. So no real issues found except the wanky stat overview for old links |
Does this only happen when the gear in the link contained Bonus Stats? If so, I think I know why - I had to make a choice for how to migrate old MeleeHit and SpellHit fields into the new % PseudoStats, and I opted to multiply rather than divide by the conversion factor so that saved school-specific EP values would translate correctly, but this will cause saved bonus stats to behave wonkily. |
On branch stats_refactor Changes to be committed: modified: ui/core/proto_utils/stats.ts
|
@InDebt I think I fixed this with the latest commit, lmk if there are still any lingering migration issues you encounter. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims to clean up our internal stats representations by storing secondary stat gear Ratings (HitRating, CritRating, HasteRating) separately from fully buffed school-specific metrics (PhysicalHit / SpellHit, PhysicalCrit / SpellCrit, MeleeHaste / RangedHaste / SpellHaste). Separating these out in the codebase for cleanliness has been a stretch goal for quite some time, but the impetus for making the change now is so that Firelands trinkets like Matrix Restabilizer and Apparatus of Khaz'goroth will be modeled correctly in the sim.
At a high level, the following changes are made:
fromStatandtoStatfields of the ReforgeStat message are changed from arrays to scalars, since they can now be represented by a single Rating stat.