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

[BUGFIX] Fix folding timer #87

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

nivivon
Copy link
Contributor

@nivivon nivivon commented Dec 25, 2024

In #83, I seem to have accidentally enabled the folding timer always after clicking on a tile. My apologies.

This pull request does three things:

  • After clicking a tile, it only applies a timer if it was selected
  • When setting the time, it removes the 2s bonus to ensure the actual timer & label match
  • It no longer auto starts the timer when changing settings to match the behaviour of the Ukeire quiz. This because the actual timer & labels were mismatched for the first go around, which is unnecessarily confusing.

How to test

  • Set the timer. See that the time doesn't move until you click a tile.
Timer.doesn.t.move.mov
  • Click a tile when the timer is selected. See the timer count down. When it hits 0, see the tile get discarded.
Timer.works.mov
  • Uncheck the timer. Click a tile. Wait long enough to show that the timer is no longer being applied.
Timer.disabled.doesn.t.move.mov

@nivivon nivivon force-pushed the patch-fix-folding-timer branch from 9bef944 to a1a111c Compare December 25, 2024 00:52
(this.state.settings.time + this.state.currentBonus) * 1000
);
this.timerUpdate = setInterval(this.updateTime, 100);
if (this.state.settings.useTimer) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the main issue, missing protecting re-initializing the timer only when it's selected.

@@ -65,17 +65,6 @@ class DefenseState extends React.Component {
clearTimeout(this.timer);
clearInterval(this.timerUpdate);
}
} else {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was removing auto-starting the timer when changing settings due to the mismtch between currentTime/currentBonus & settings.time/extraTime.

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.

1 participant