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 couple stat issues #2442

Merged
merged 6 commits into from
Sep 16, 2024
Merged

Fix couple stat issues #2442

merged 6 commits into from
Sep 16, 2024

Conversation

nguyentvan7
Copy link
Collaborator

Describe your changes

  • Remove -1 from things that have no levels
  • Adjust char dm util to reduce nesting needed to access values
  • Adjust heal util for future heals that don't have a flat value
  • Rename brEff_ -> brEffect_ to avoid collision with brEfficiency_ later

Issue or discord link

Testing/validation

Checklist before requesting a review (leave this PR as draft if any part of this list is not done.)

  • I have commented my code in hard-to understand areas.
  • I have made corresponding changes to README or wiki.
  • For front-end changes, I have updated the corresponding English translations.
  • I have run yarn run mini-ci locally to validate format and lint.
  • If I have added a new library or app, I have updated the deployment scripts to ignore changes as needed

Copy link
Contributor

github-actions bot commented Sep 15, 2024

[sr-frontend] [Sun Sep 15 03:53:58 UTC 2024] - Deployed 88698cf to https://genshin-optimizer-prs.github.io/pr/2442/sr-frontend (Takes 3-5 minutes after this completes to be available)

[Mon Sep 16 00:14:12 UTC 2024] - Deleted deployment

Copy link
Owner

@frzyc frzyc left a comment

Choose a reason for hiding this comment

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

I am okay with the stat renaming, just need to make sure scanners continue to work. (do we need to raise an issue with scanner devs?)

@nguyentvan7
Copy link
Collaborator Author

Scanners been broke plus we don't even have support from reliquary scanner which is the primo scanner imo. Might be better for us to support their format instead of other way around maybe?

@nguyentvan7 nguyentvan7 merged commit df0679e into master Sep 16, 2024
7 checks passed
@nguyentvan7 nguyentvan7 deleted the van/sro/statFix branch September 16, 2024 00:13
@frzyc
Copy link
Owner

frzyc commented Sep 16, 2024

Scanners been broke plus we don't even have support from reliquary scanner which is the primo scanner imo. Might be better for us to support their format instead of other way around maybe?

sure, we'd just add a conversion layer between.

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