From 1924a686715af14bcd4a7f70a54bc47fcfb211d4 Mon Sep 17 00:00:00 2001
From: Eric Yancey Dauenhauer <eric@ericyd.com>
Date: Thu, 8 Dec 2022 18:02:24 -0600
Subject: [PATCH] Metronome garbage cleanup (#31)

* await permissions before enumerating devices!

* encourage garbage cleanup
---
 roadmap.md              |  3 ++-
 src/Metronome/index.tsx | 56 ++++++++++++++++++++++++++---------------
 src/Start/index.tsx     |  8 +++---
 3 files changed, 41 insertions(+), 26 deletions(-)

diff --git a/roadmap.md b/roadmap.md
index b4a0c4f..48cb7b1 100644
--- a/roadmap.md
+++ b/roadmap.md
@@ -30,6 +30,7 @@ The `Metronome` is the heart of the app. The BPM, measure, current tick, and tim
 - [x] Metronome must initialize as "stopped", and can be "started" by user input https://github.com/ericyd/loop-supreme/pull/4
 - [x] Metronome can be muted, while still running https://github.com/ericyd/loop-supreme/pull/11
 - [x] ~move "playing" state into MetronomeControls; there is no obvious need for it to live in Metronome~ irrelevant after https://github.com/ericyd/loop-supreme/pull/28
+- [ ] allow changing tempo by typing value into an input
 
 ## Scene
 
@@ -92,7 +93,7 @@ A `Track` is a single mono or stereo audio buffer that contains audio data. A `T
 
 ## Misc
 
-- [ ] `Bug`: using keyboard shortcuts is causing weird recording artifacts... 😭
+- [x] `Bug`: using keyboard shortcuts is causing weird recording artifacts... 😭 https://github.com/ericyd/loop-supreme/pull/30
 - [x] clean up "start" button/view
 - [x] Allow user to change inputs https://github.com/ericyd/loop-supreme/pull/25
 - [ ] clean up TODOs
diff --git a/src/Metronome/index.tsx b/src/Metronome/index.tsx
index ef63933..d73d441 100644
--- a/src/Metronome/index.tsx
+++ b/src/Metronome/index.tsx
@@ -1,3 +1,9 @@
+/**
+ * Metronome provides controls for the common metronome settings:
+ * BPM, measures per loop, and time signature.
+ * It also controls whether or not the click track makes noise,
+ * and the global "playing" state of the app.
+ */
 import React, { useCallback, useEffect, useRef, useState } from 'react'
 import { useAudioContext } from '../AudioProvider'
 import { BeatCounter } from './BeatCounter'
@@ -56,43 +62,53 @@ export const Metronome: React.FC<Props> = ({ clock }) => {
    * Set up metronome gain node.
    * See Track/index.tsx for description of the useRef/useEffect pattern
    */
-  const gainNode = useRef(
-    new GainNode(audioContext, { gain: muted ? 0.0 : gain })
-  )
+  const gainNode = useRef<GainNode | null>()
+  useEffect(() => {
+    gainNode.current = new GainNode(audioContext, { gain: 0.5 })
+    gainNode.current.connect(audioContext.destination)
+    return () => {
+      gainNode.current?.disconnect()
+    }
+  }, [audioContext])
   useEffect(() => {
-    gainNode.current.gain.value = muted ? 0.0 : gain
+    if (gainNode.current) {
+      gainNode.current.gain.value = muted ? 0.0 : gain
+    }
   }, [gain, muted])
 
+  // I don't think this is an ideal use for a ref,
+  // but this is the easiest way to be able to "disconnect" on each loop.
+  // This isn't strictly necessary afaik, but I think it will help with garbage cleanup
+  const source = useRef<AudioBufferSourceNode | null>(null)
+
   /**
+   * Add clock event listeners.
    * On each tick, set the "currentTick" value and emit a beep.
    * The AudioBufferSourceNode must be created fresh each time,
    * because it can only be played once.
    */
-  const clockMessageHandler = useCallback(
-    (event: MessageEvent<ClockControllerMessage>) => {
+  useEffect(() => {
+    const clockMessageHandler = (
+      event: MessageEvent<ClockControllerMessage>
+    ) => {
       // console.log(event.data) // this is really noisy
-      if (event.data.message === 'TICK') {
-        const source = new AudioBufferSourceNode(audioContext, {
+      if (event.data.message === 'TICK' && gainNode.current) {
+        if (source.current) {
+          source.current.disconnect()
+        }
+        source.current = new AudioBufferSourceNode(audioContext, {
           buffer: event.data.downbeat ? sine380 : sine330,
         })
-
-        gainNode.current.connect(audioContext.destination)
-        source.connect(gainNode.current)
-        source.start()
+        source.current.connect(gainNode.current)
+        source.current.start()
       }
-    },
-    [audioContext, sine330, sine380]
-  )
+    }
 
-  /**
-   * Add clock event listeners
-   */
-  useEffect(() => {
     clock.addEventListener('message', clockMessageHandler)
     return () => {
       clock.removeEventListener('message', clockMessageHandler)
     }
-  }, [clockMessageHandler, clock])
+  }, [audioContext, sine330, sine380, clock])
 
   /**
    * When "playing" is toggled on/off,
diff --git a/src/Start/index.tsx b/src/Start/index.tsx
index 3ff2b7c..6894f6a 100644
--- a/src/Start/index.tsx
+++ b/src/Start/index.tsx
@@ -95,11 +95,9 @@ export const Start: React.FC = () => {
   }
 
   async function handleClick() {
-    await Promise.all([
-      grantDevicePermission(),
-      enumerateDevices(),
-      initializeAudioContext(),
-    ])
+    await grantDevicePermission()
+    await enumerateDevices()
+    await initializeAudioContext()
   }
 
   return defaultDeviceId && audioContext && devices?.length ? (