Skip to content

Conversation

@shivaganesht
Copy link

This PR adds essential keyboard controls that improve user experience:

Features

  • SPACE - Pause/resume the sequencer (freezes the scanning)
  • M - Mute/unmute audio output using Tone.Destination.mute
  • H - Toggle a help overlay showing available shortcuts

Visual Feedback

  • Pause indicator (⏸) in top-right corner when paused
  • "MUTED" label in top-left when audio is muted
  • Clean centered help overlay with keyboard shortcuts

Why This Matters

The original project had no way to pause or mute the audio without refreshing the page. This makes it much more usable for presentations, demos, or just general use.

Technical Notes

  • Uses p5.js built-in keyPressed() function
  • Audio muting uses Tone.js native Tone.Destination.mute API
  • Minimal code footprint (~100 lines)
  • No new dependencies
  • Follows existing code style

- Add SPACE key to pause/resume the sequencer
- Add M key to mute/unmute audio output
- Add H key to show/hide keyboard shortcuts help
- Display visual indicators for paused and muted states
- Uses Tone.Destination.mute for proper audio control
Copilot AI review requested due to automatic review settings December 26, 2025 18:29
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds keyboard controls to improve user experience by allowing pause/resume, mute/unmute, and help overlay functionality. The implementation uses p5.js keyPressed() for input handling and Tone.js for audio muting.

  • Adds pause (SPACE), mute (M), and help (H) keyboard shortcuts
  • Implements visual indicators for paused and muted states
  • Adds a help overlay displaying available keyboard shortcuts

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +285 to +289
if (isMuted) {
Tone.Destination.mute = true;
} else {
Tone.Destination.mute = false;
}
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

The if-else statement can be simplified by directly assigning the isMuted value to Tone.Destination.mute. Instead of checking the value and setting true or false separately, you can assign isMuted directly to Tone.Destination.mute.

Suggested change
if (isMuted) {
Tone.Destination.mute = true;
} else {
Tone.Destination.mute = false;
}
Tone.Destination.mute = isMuted;

Copilot uses AI. Check for mistakes.
}

pathLineDraw();
ellipseMoving();
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

The visual indicators (pause, mute, help overlay) are drawn even when paused, but the ellipseMoving() function still updates animation positions (linesXPos) in the paused state. This creates an inconsistency where the visual state appears frozen but the underlying animation positions continue to update. Consider also skipping ellipseMoving() when paused, or at least skip the position update inside it.

Suggested change
ellipseMoving();
if (!isPaused) {
ellipseMoving();
}

Copilot uses AI. Check for mistakes.

pathLineDraw();
ellipseMoving();
waveLineDraw();
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

The waveLineDraw() function continues to update animation state (waveMovingFactor and waveMovingDec) even when paused. This means the wave animation will continue to progress invisibly while paused, which may cause unexpected visual jumps when resuming. Consider also skipping these updates when isPaused is true.

Suggested change
waveLineDraw();
if (!isPaused) {
waveLineDraw();
}

Copilot uses AI. Check for mistakes.
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