Restore scene rotator, tablet blog/audio tabs, and ambient audio synth#87
Restore scene rotator, tablet blog/audio tabs, and ambient audio synth#87alexwelcing wants to merge 1 commit intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR restores scene rotation functionality, adds blog/audio tabs to the tablet interface, and implements an ambient audio synthesizer to replace binary audio files. The changes enable users to cycle through panorama images and Gaussian splats, access articles and audio controls from the tablet UI, and persist audio preferences across sessions.
Key Changes:
- Implemented Web Audio API-based ambient audio with localStorage persistence for audio preferences
- Extended the background images API to support listing all available panoramas
- Added scene rotation controls with prev/next navigation and click-to-select tiles in the tablet UI
- Integrated BLOG and AUDIO tabs into the tablet terminal interface with article reading and audio controls
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
components/AmbientAudio.tsx |
New component implementing Web Audio synthesizer for ambient sound with browser autoplay handling |
pages/api/backgroundImages.ts |
Extended API to support list mode returning all available background images |
components/ThreeSixty.tsx |
Added audio state management, background image fetching, and scene rotation handler |
components/TerminalInterface.tsx |
Added BLOG and AUDIO tabs with article display, read tracking, and audio controls |
components/InteractiveTablet.tsx |
Updated tablet menu to support new tabs and pass audio/rotation props to terminal |
pages/index.tsx |
Modified image change handler to accept specific image paths or fallback to random |
pages/chat.tsx |
Updated image change handler signature to match new pattern |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| res.status(200).json({ | ||
| image: `/background/${randomImage}`, |
There was a problem hiding this comment.
The API now returns image paths with a leading slash (/background/${file}), but previously returned paths with a dot-slash prefix (./background/${randomImage}). This change in path format could break existing code that expects the old format. Ensure all consumers of this API are updated to handle the new path format correctly.
| <div style={{ | ||
| display: 'grid', | ||
| gridTemplateColumns: 'repeat(2, 1fr)', | ||
| gridTemplateColumns: isMobile ? 'repeat(2, 1fr)' : 'repeat(3, 1fr)', |
There was a problem hiding this comment.
The grid layout changes from a fixed 2-column layout to a conditional 2 or 3-column layout based on isMobile. However, with 6 action buttons (ASK AI, BLOG, GAME, SCENE, AUDIO, ABOUT), a 2-column grid on mobile will create 3 rows, while a 3-column grid on desktop creates 2 rows. Consider if this is the intended layout behavior, as having 6 items evenly divisible by both 2 and 3 might benefit from a different responsive approach.
| gridTemplateColumns: isMobile ? 'repeat(2, 1fr)' : 'repeat(3, 1fr)', | |
| gridTemplateColumns: 'repeat(auto-fit, minmax(120px, 1fr))', |
| if (!activeView || !isOpen) return; | ||
| setViewMode(activeView); | ||
| }, [activeView, isOpen]); |
There was a problem hiding this comment.
The activeView prop is passed to TerminalInterface and used to set viewMode via useEffect, but this creates a potential sync issue. If activeView changes while the terminal is open, it will switch views. However, if activeView changes while the terminal is closed, the effect won't run (due to the isOpen check), and when the terminal opens again, it might show a stale view. Consider removing the isOpen check from the dependency array or restructuring this logic.
| if (!activeView || !isOpen) return; | |
| setViewMode(activeView); | |
| }, [activeView, isOpen]); | |
| if (!activeView) return; | |
| setViewMode(activeView); | |
| }, [activeView]); |
| const attemptPlay = useCallback(async () => { | ||
| try { | ||
| if (!contextRef.current) { | ||
| contextRef.current = new AudioContext(); | ||
| } | ||
| const context = contextRef.current; | ||
| await context.resume(); | ||
|
|
||
| if (!gainRef.current) { | ||
| gainRef.current = context.createGain(); | ||
| gainRef.current.gain.value = volume; | ||
| gainRef.current.connect(context.destination); | ||
| } | ||
|
|
||
| if (!oscillatorRef.current) { | ||
| const oscillator = context.createOscillator(); | ||
| oscillator.type = 'sine'; | ||
| oscillator.frequency.value = 220; | ||
| oscillator.connect(gainRef.current); | ||
| oscillator.start(); | ||
| oscillatorRef.current = oscillator; | ||
| } | ||
|
|
||
| setBlocked(false); | ||
| onStatusChange?.('playing'); | ||
| } catch (error) { | ||
| setBlocked(true); | ||
| onStatusChange?.('blocked'); | ||
| } | ||
| }, [onStatusChange, volume]); |
There was a problem hiding this comment.
The attemptPlay callback has volume in its dependency array, but the volume is set once when gainRef.current is created. When volume changes and attemptPlay is recreated, it won't update the gain value for an existing oscillator. The volume update is handled in a separate useEffect (lines 61-65), but this means attemptPlay will be recreated on every volume change even though it doesn't need the latest volume value. Consider removing volume from the dependency array since the gain update is handled separately.
| function handleImageChange(newImage: string) { | ||
| if (newImage) { | ||
| setCurrentImage(newImage) | ||
| return | ||
| } | ||
| getRandomImage() | ||
| } |
There was a problem hiding this comment.
The handleImageChange function accepts a newImage parameter but the function signature doesn't validate or document what happens when an empty string is passed. If newImage is an empty string, the condition if (newImage) will be false and getRandomImage() will be called. Consider being more explicit about this behavior or adding validation for valid image paths.
| const fetchBackgrounds = async () => { | ||
| try { | ||
| const response = await fetch('/api/backgroundImages?mode=list'); | ||
| if (!response.ok) return; | ||
| const data = await response.json(); | ||
| if (Array.isArray(data.images)) { | ||
| setAvailableImages(data.images); | ||
| } | ||
| } catch (error) { | ||
| console.error('Failed fetching background images:', error); |
There was a problem hiding this comment.
When the API request fails or returns a non-ok response, the error is silently logged but availableImages remains as an empty array. This means users won't see panorama options if the API fails. Consider adding error handling UI or retry logic to improve the user experience when the background images API is unavailable.
| const fetchBackgrounds = async () => { | |
| try { | |
| const response = await fetch('/api/backgroundImages?mode=list'); | |
| if (!response.ok) return; | |
| const data = await response.json(); | |
| if (Array.isArray(data.images)) { | |
| setAvailableImages(data.images); | |
| } | |
| } catch (error) { | |
| console.error('Failed fetching background images:', error); | |
| const fetchBackgrounds = async (attempt: number = 1) => { | |
| try { | |
| const response = await fetch('/api/backgroundImages?mode=list'); | |
| if (!response.ok) { | |
| throw new Error(`Background images request failed with status ${response.status}`); | |
| } | |
| const data = await response.json(); | |
| if (Array.isArray(data.images)) { | |
| setAvailableImages(data.images); | |
| } | |
| } catch (error) { | |
| console.error(`Failed fetching background images (attempt ${attempt}):`, error); | |
| const maxAttempts = 3; | |
| if (attempt < maxAttempts) { | |
| const retryDelayMs = 1000 * attempt; | |
| setTimeout(() => { | |
| fetchBackgrounds(attempt + 1); | |
| }, retryDelayMs); | |
| } |
| updateStats('articlesRead', [slug]); | ||
| if (currentQuest?.id === 'read-article') { | ||
| completeQuest('read-article'); | ||
| } | ||
| window.open(`/articles/${slug}`, '_blank'); |
There was a problem hiding this comment.
The article filename is used as a fallback for the slug, but filenames may contain characters that are not URL-safe. Consider sanitizing the filename or ensuring it's properly encoded before using it in the URL path. Additionally, if the article has no filename and the title contains special characters, spaces, or slashes, this could result in a malformed URL.
| updateStats('articlesRead', [slug]); | |
| if (currentQuest?.id === 'read-article') { | |
| completeQuest('read-article'); | |
| } | |
| window.open(`/articles/${slug}`, '_blank'); | |
| const safeSlug = encodeURIComponent(slug); | |
| updateStats('articlesRead', [slug]); | |
| if (currentQuest?.id === 'read-article') { | |
| completeQuest('read-article'); | |
| } | |
| window.open(`/articles/${safeSlug}`, '_blank'); |
| if (!oscillatorRef.current) { | ||
| const oscillator = context.createOscillator(); | ||
| oscillator.type = 'sine'; | ||
| oscillator.frequency.value = 220; | ||
| oscillator.connect(gainRef.current); | ||
| oscillator.start(); | ||
| oscillatorRef.current = oscillator; | ||
| } |
There was a problem hiding this comment.
The oscillator is created and started but never stopped if attemptPlay is called multiple times while an oscillator is already running. This can lead to multiple oscillators playing simultaneously. Consider checking if an oscillator already exists and is connected before creating a new one, or stop the existing oscillator before creating a new one.
| useEffect(() => { | ||
| if (!enabled) { | ||
| stopAudio(); | ||
| return; | ||
| } | ||
| attemptPlay(); | ||
| }, [attemptPlay, enabled, stopAudio]); |
There was a problem hiding this comment.
The stopAudio and attemptPlay callbacks are included in the dependency array of the useEffect that manages audio playback (line 73). This creates a circular dependency: when stopAudio changes, the effect runs, which may call attemptPlay, causing the effect to re-run. This can lead to unnecessary re-execution. Consider using refs for these functions or restructure the logic to avoid this circular dependency.
| useEffect(() => { | ||
| return () => { | ||
| stopAudio(); | ||
| if (contextRef.current) { | ||
| contextRef.current.close(); | ||
| contextRef.current = null; | ||
| } | ||
| }; | ||
| }, [stopAudio]); |
There was a problem hiding this comment.
The cleanup effect on line 96 has stopAudio in its dependency array, which means the cleanup function will be recreated whenever stopAudio changes. This defeats the purpose of having a cleanup-only effect. Consider either removing the dependency array entirely or restructuring to avoid depending on stopAudio.
Motivation
Description
components/AmbientAudio.tsxWeb Audio-based synth and replaced the committed WAV file with a synthesized loop, and persistedambientAudioEnabledandambientAudioVolumeinlocalStorage.pages/api/backgroundImages.tsto return an image list when called with?mode=listand updatedcomponents/ThreeSixty.tsxto fetch and exposeavailableImagesand buildsceneryOptionsincluding splats and panoramas.handleRotateScenery) and wiredonRotateScenerythroughInteractiveTablet.tsx→TerminalInterface.tsx, adding prev/next buttons and click-to-select scenery tiles.BLOGandAUDIOtabs, article "READ" actions that update journey stats, and audio controls (mute/volume/status) integrated into the tablet.Testing
npm run dev) and confirmed the app compiled and served pages successfully (dev server reported Ready).curlwhich returned200 OKfor the home page.Codex Task