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] Keep playstreak quests in sync #1111

Merged
merged 19 commits into from
Nov 14, 2024
Merged

[Fix] Keep playstreak quests in sync #1111

merged 19 commits into from
Nov 14, 2024

Conversation

BrettCleary
Copy link
Collaborator

@BrettCleary BrettCleary commented Oct 16, 2024

@BrettCleary BrettCleary self-assigned this Oct 16, 2024
@flavioislima
Copy link
Contributor

recheck

Copy link

github-actions bot commented Oct 16, 2024

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@BrettCleary BrettCleary added the PR: Ready-For-Review PR is ready to be reviewed by peers label Oct 22, 2024
useEffect(() => {
const initSync = async () => {
try {
questPlayStreakSyncState.init({
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the only place it is used, so it is simpler to just call init right before. I initially explored adding it to StoreController, but the StoreController useEffect executes after this useEffect, which breaks keepProjectQuestsInSync

Copy link
Contributor

Choose a reason for hiding this comment

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

how interesting, in my implementation of this in the store I used the StoreController approach

https://github.com/HyperPlay-Gaming/hyperplay-store/pull/228/files#diff-c4fdb34f33780698be8e19adc9cda4a65464bcd97bdc4a7ed870e888a3c5e42d

tbh I think this way is simpler and does the work too

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

maybe we should move this into @hyperplay/quests-ui if it will be used on the store as well

appQueryClient: queryClient
})

await questPlayStreakSyncState.keepProjectQuestsInSync(appName)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

clearAllTimers does not need to be called since the store and timers are destroyed when the overlay is closed, which is also when we should stop syncing play sessions

@BrettCleary BrettCleary added PR: Ready-For-Test PR is ready to be tested by a QA and removed PR: Ready-For-Review PR is ready to be reviewed by peers labels Oct 30, 2024
@nyghtstalker nyghtstalker removed their request for review November 8, 2024 22:51
@BrettCleary
Copy link
Collaborator Author

@nyghtstalker Ready for a retest

Copy link

@nyghtstalker nyghtstalker left a comment

Choose a reason for hiding this comment

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

  • Confirmed that quests are moving to the claimable section when ready to be claimed
  • Confirmed the claim button activates and is useable when quest is claimable
  • Confirmed claim button works and reward can be claimed
  • Confirmed that when the timer for the days completion is finished it will add and display the day to the count and be claimable if reaching the set day amount needed.

@BrettCleary BrettCleary merged commit 9bb76b4 into main Nov 14, 2024
11 checks passed
@BrettCleary BrettCleary deleted the fix/playstreak_sync branch November 14, 2024 23:38
@github-actions github-actions bot locked and limited conversation to collaborators Nov 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR: Ready-For-Test PR is ready to be tested by a QA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants