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

Enforce read-side and write-side query structure #2377

Merged
merged 1 commit into from
Aug 14, 2024
Merged

Enforce read-side and write-side query structure #2377

merged 1 commit into from
Aug 14, 2024

Conversation

lantua
Copy link
Collaborator

@lantua lantua commented Aug 14, 2024

Describe your changes

The current tag db is too lenient on how the buffs get added to a formula. Some formulas are added to et:self while others to et:selfBuff. This makes the system hard to design as it needs to accommodate both possibilities. This PR enforces more structure on et: as follows.

On the read side:

  • Final query (value that includes all applicable contributions) reads from et:self,
    • self.final.atk yields the final.atk from character/weapon/artifact/etc,
  • Contribution from a particular sheet reads from et:*Buff with appropriate sheet:,
    • selfBuff.final.atk.sheet("Nahida") yields Nahida's contribution to self.final.atk.

On the write side:

  • Queries with sheet:undefined or sheet:static write to et:self,
  • Queries in a sheet write to et:selfBuff/teamBuff/notSelfBuff (i.e., et:*Buff),
  • Queries outside of a sheet never write to et:*Buff,

This structure allows the buff system to be design as

`self:` stat <= "determine contributions" <= `et:*Buff` entries

It however requires linkage from self: <= et:*Buff at all time, so we also add noTeamData in case there is no team member (and so src:dst: are unnecessary).

Issue or discord link

https://discord.com/channels/785153694478893126/1189329506842984570/1272737073522278570

Testing/validation

Unit tests are added to enforce write-side entries.

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

- Add unit test to enforce tag db `et:`
@lantua lantua requested a review from nguyentvan7 August 14, 2024 03:03
Copy link
Contributor

github-actions bot commented Aug 14, 2024

[sr-frontend] [Wed Aug 14 03:05:37 UTC 2024] - Deployed 125c281 to https://genshin-optimizer-prs.github.io/pr/2377/sr-frontend (Takes 3-5 minutes after this completes to be available)

[Wed Aug 14 15:06:56 UTC 2024] - Deleted deployment

Copy link
Collaborator

@nguyentvan7 nguyentvan7 left a comment

Choose a reason for hiding this comment

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

Possibly time to rename selfBuff? not sure if that name properly describes what it does anymore

Do we need any changes on calc consumption side for this? I'm thinking not atm since we were always reading from self for individual char/wep contributions

@lantua
Copy link
Collaborator Author

lantua commented Aug 14, 2024

One change to consumption is that individual contribution is read from *buff, instead of the previously self. If we need contribution from all selfBuff/teamBuff/notSelfBuff together, I can add a new read-side et: too.

@lantua lantua merged commit 7b45e41 into master Aug 14, 2024
8 checks passed
@lantua lantua deleted the isolate branch August 14, 2024 15:06
@lantua lantua restored the isolate branch August 14, 2024 15:06
@lantua lantua deleted the isolate branch August 14, 2024 15:06
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