feat: add block win audio base on tiers #1533
Conversation
📝 WalkthroughWalkthroughThis pull request integrates comprehensive audio functionality into the application. It updates build configurations, adds audio elements in HTML, and introduces a new dependency. Localization files are enhanced with an "audio-enabled" entry, and Tauri configuration files are updated with new features, permissions, and resource entries. New backend commands, configuration fields, and UI components handle audio settings and playback. Additionally, type definitions and state management files receive updates to support the audio features. Changes
Possibly related PRs
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (14)
src/containers/floating/Settings/sections/general/AudioSettings.tsx (2)
14-14: Consider adding fallback translation.The
useSuspense: falseoption prevents suspense during translation loading, but you should also consider adding a fallback text in case the translation fails to load.- const { t } = useTranslation(['settings'], { useSuspense: false }); + const { t, ready } = useTranslation(['settings'], { useSuspense: false }); + const audioEnabledText = ready ? t('audio-enabled') : 'Audio enabled';
21-34: Consider adding aria-label for accessibility.The implementation looks good, but could be enhanced with accessibility attributes.
- <ToggleSwitch checked={audioEnabled} onChange={(event) => setAudioEnabled(event.target.checked)} /> + <ToggleSwitch + checked={audioEnabled} + onChange={(event) => setAudioEnabled(event.target.checked)} + aria-label={t('audio-enabled')} + />src/glApp.d.ts (1)
27-27: Add JSDoc comments for better documentation.The new method should be documented with parameter descriptions and example usage.
+ /** + * Initializes audio functionality with callback handlers. + * @param notificationCB - Callback function triggered for notification sounds + * @param blockWinCB - Callback function triggered when a block is won + * @param blockWinCB.tier - The tier level of the block win (1-3) + */ initAudio: (notificationCB: () => void, blockWinCB: (tier: number) => void) => void;src/store/useBlockchainVisualisationStore.ts (1)
56-67: Consider using an enum for success tiers.Using an enum for success tiers would improve type safety and maintainability.
+enum SuccessTier { + Level1 = 1, + Level2 = 2, + Level3 = 3, +} -function selectAudioAssetOnSuccessTier(tier: number) { +function selectAudioAssetOnSuccessTier(tier: SuccessTier) { switch (tier) { - case 1: + case SuccessTier.Level1: return 'assets/Success_Level_01.wav'; - case 2: + case SuccessTier.Level2: return 'assets/Success_Level_02.wav'; - case 3: + case SuccessTier.Level3: return 'assets/Success_Level_03.wav'; default: throw new Error('Invalid tier'); } }src-tauri/src/commands.rs (1)
1402-1426: Fix error message in telemetry logging.The error message incorrectly refers to "get_audio_enabled" instead of "set_audio_enabled".
- "error at sending telemetry data for get_audio_enabled {:?}", + "error at sending telemetry data for set_audio_enabled {:?}",src-tauri/capabilities/default.json (1)
21-28: Consider scoping directory permission more precisely.While the file permission is correctly scoped to block_win.mp3, the directory permission for the entire audio directory might be broader than necessary if only one audio file is needed.
If only block_win.mp3 is needed, you could remove the directory permission:
- { - "identifier": "fs:allow-read-dir", - "allow": [{ "path": "$RESOURCE/audio" }] - }public/locales/cn/settings.json (1)
7-7: Localization Entry for Audio Feature
The new key"audio-enabled": "Audio enabled"has been added to the Chinese localization file. Consider verifying whether the value should be translated—for example, to"音频已启用"—to better suit a native Chinese-speaking audience.public/locales/ko/settings.json (1)
7-7: Localization Update for Audio Feature
The added key"audio-enabled": "Audio enabled"in the Korean locale file currently retains the English phrase. It may be worthwhile to check if a localized Korean translation would provide a more consistent user experience (for example, using an appropriate Korean phrase).public/locales/ja/settings.json (1)
7-7: New Localization Key Added (Japanese).
The key"audio-enabled": "Audio enabled"has been added to support the new audio feature. Consider using a Japanese translation (e.g. "オーディオが有効") to maintain full localization consistency.public/locales/tr/settings.json (1)
7-7: New Localization Key Added (Turkish).
The new key"audio-enabled": "Audio enabled"appears in the Turkish locale; however, it remains in English. For a consistent user experience, consider providing a Turkish translation (e.g. "Ses etkin").public/locales/id/settings.json (1)
7-7: New Localization Key Added (Indonesian).
The key"audio-enabled": "Audio enabled"is now included for Indonesian users. To improve localization quality, you might consider translating it (e.g. "Audio diaktifkan").public/locales/ru/settings.json (1)
7-7: New Localization Key Added (Russian).
The key"audio-enabled": "Audio enabled"has been added; consider translating it into Russian (for example, "Аудио включено") to enhance the localization.public/locales/pl/settings.json (1)
7-7: New Audio Setting Entry Added
The key"audio-enabled": "Audio enabled"has been added for the Polish locale. Verify that the English phrasing is intentional for this locale; if a translated value is preferred for consistency with the rest of the file, consider updating it accordingly.public/locales/de/settings.json (1)
7-7: New Audio Setting Entry in German Locale
A new entry"audio-enabled": "Audio enabled"has been introduced. Please confirm whether this English text is intentional for the German locale. If a localized translation is desired, you might consider using a phrase such as"Audio aktiviert".
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/assets/Notification.wavis excluded by!**/*.wavpublic/assets/Success_Level_01.wavis excluded by!**/*.wavpublic/assets/Success_Level_02.wavis excluded by!**/*.wavpublic/assets/Success_Level_03.wavis excluded by!**/*.wavsrc-tauri/Cargo.lockis excluded by!**/*.locksrc-tauri/audio/block_win.mp3is excluded by!**/*.mp3
📒 Files selected for processing (29)
.github/workflows/release.yml(1 hunks)index.html(1 hunks)package.json(1 hunks)public/locales/af/settings.json(1 hunks)public/locales/cn/settings.json(1 hunks)public/locales/de/settings.json(1 hunks)public/locales/en/settings.json(2 hunks)public/locales/fr/settings.json(1 hunks)public/locales/hi/settings.json(1 hunks)public/locales/id/settings.json(1 hunks)public/locales/ja/settings.json(1 hunks)public/locales/ko/settings.json(1 hunks)public/locales/pl/settings.json(1 hunks)public/locales/ru/settings.json(1 hunks)public/locales/tr/settings.json(2 hunks)src-tauri/Cargo.toml(3 hunks)src-tauri/capabilities/default.json(1 hunks)src-tauri/src/app_config.rs(7 hunks)src-tauri/src/commands.rs(3 hunks)src-tauri/src/main.rs(2 hunks)src-tauri/tauri.conf.json(4 hunks)src/App/AppEffects.tsx(2 hunks)src/containers/floating/Settings/sections/general/AudioSettings.tsx(1 hunks)src/containers/floating/Settings/sections/general/GeneralSettings.tsx(2 hunks)src/glApp.d.ts(1 hunks)src/store/useAppConfigStore.ts(3 hunks)src/store/useBlockchainVisualisationStore.ts(3 hunks)src/types/app-status.ts(1 hunks)src/types/invoke.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- public/locales/fr/settings.json
- public/locales/af/settings.json
- public/locales/hi/settings.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: tauri-build
- GitHub Check: cargo checks (fmt, clippy, check)
🔇 Additional comments (18)
src/containers/floating/Settings/sections/general/AudioSettings.tsx (1)
16-19: LGTM! Clean state management implementation.Good use of selector function to minimize re-renders by only subscribing to required state.
src/types/invoke.ts (1)
99-101: LGTM!The new function signatures for audio-related features are well-defined and follow the existing pattern in the file.
src/types/app-status.ts (1)
51-51: LGTM!The new
audio_enabledproperty is correctly typed and well-placed within theAppConfiginterface.src/store/useAppConfigStore.ts (1)
42-44: LGTM!The audio settings implementation follows the established patterns in the codebase:
- Proper state management with initial values
- Error handling with state rollback
- Clear separation of concerns between feature flag and user preference
Also applies to: 74-74, 79-79, 313-321, 324-327
src-tauri/src/app_config.rs (2)
120-121: LGTM! Audio configuration field added.The audio_enabled field is properly added with serde serialization and appropriate default value.
551-559: LGTM! Audio configuration methods implemented.The getter and setter methods are well-implemented, following the established pattern in the file and ensuring proper error handling.
src-tauri/src/main.rs (2)
1069-1069: LGTM! File system plugin initialization added.The tauri_plugin_fs is properly initialized in the application builder.
1293-1296: LGTM! Audio-related commands added to invoke handler.The new audio-related commands are properly registered in the invoke handler.
src-tauri/src/commands.rs (2)
247-250: LGTM! Feature flag check implemented.Simple and effective implementation to check if the audio feature is enabled.
1396-1400: LGTM! Audio enabled getter implemented.Clean implementation following the established pattern of other getters in the file.
package.json (1)
22-22: New Dependency Addition for Tauri FS Plugin
The addition of"@tauri-apps/plugin-fs": "^2.2.0"is in line with the PR’s objective to support audio functionality through enhanced file system operations. The specified version is consistent with the project’s other Tauri dependencies.src-tauri/tauri.conf.json (2)
34-35: Bundling the Audio Resource and Updater Artifacts
The new"createUpdaterArtifacts": "v1Compatible"entry and the inclusion of"resources": ["audio/block_win.mp3"]ensure that the audio asset is correctly bundled with the application. This setup facilitates proper deployment of the new audio feature.
53-56: Asset Protocol Configuration for Audio
By adding the"assetProtocol": { "enable": true, "scope": ["$RESOURCE/audio"] }block, you are enabling secure and scoped access to audio resources. Please ensure that testing covers scenarios where the asset is retrieved via the asset protocol.src-tauri/Cargo.toml (3)
71-79: Enable Protocol-Asset Feature in Tauri Dependency
Introducing the"protocol-asset"feature in the Tauri dependency is a robust approach to extend asset handling capabilities. This enhancement directly supports the integration of audio features and aligns well with existing dependency configurations.
100-102: Inclusion of Tauri Plugin FS Dependency
The addition oftauri-plugin-fs = "2"is essential for advanced file system interactions needed for managing audio assets. This dependency complements the corresponding JavaScript plugin added inpackage.jsonand solidifies the underlying architecture for audio operations.
130-130: New Audio Feature Flag
The new feature flag"audio = []"in the[features]section provides modular control over the audio functionality during builds. This approach supports feature toggling and maintains flexibility for different build environments.public/locales/en/settings.json (1)
7-7: New Localization Key Added (English).
The key"audio-enabled": "Audio enabled"is correctly added in the English locale. The text is clear and appropriate for English users..github/workflows/release.yml (1)
236-236: Include Audio Feature in Build Arguments
The build arguments have been updated to include the "audio" feature, now using:args: ${{ matrix.args }} --features "audio ${{ env.TS_FEATURES }}"This change aligns with the new audio functionality. Please verify that concatenating
"audio ${{ env.TS_FEATURES }}"behaves correctly, particularly ifTS_FEATURESmight ever be empty.
| await fetchAppConfig(); | ||
| await fetchAudioFeatureEnabled(); | ||
| await setMiningNetwork(); | ||
| initAnimationAudio(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for initialization sequence.
The initialization sequence should handle potential failures gracefully.
- await fetchAppConfig();
- await fetchAudioFeatureEnabled();
- await setMiningNetwork();
- initAnimationAudio();
+ try {
+ await fetchAppConfig();
+ await fetchAudioFeatureEnabled();
+ await setMiningNetwork();
+ initAnimationAudio();
+ } catch (error) {
+ console.error('Failed to initialize app:', error);
+ // Consider showing a user-friendly error message
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await fetchAppConfig(); | |
| await fetchAudioFeatureEnabled(); | |
| await setMiningNetwork(); | |
| initAnimationAudio(); | |
| try { | |
| await fetchAppConfig(); | |
| await fetchAudioFeatureEnabled(); | |
| await setMiningNetwork(); | |
| initAnimationAudio(); | |
| } catch (error) { | |
| console.error('Failed to initialize app:', error); | |
| // Consider showing a user-friendly error message | |
| } |
|
|
||
| export const GeneralSettings = () => { | ||
| const isAudioFeatureEnabled = useAppConfigStore.getState().isAudioFeatureEnabled; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using store selector hook instead of direct state access.
Using getState() directly might miss updates. Consider using the hook pattern for consistency.
- const isAudioFeatureEnabled = useAppConfigStore.getState().isAudioFeatureEnabled;
+ const isAudioFeatureEnabled = useAppConfigStore(state => state.isAudioFeatureEnabled);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isAudioFeatureEnabled = useAppConfigStore.getState().isAudioFeatureEnabled; | |
| const isAudioFeatureEnabled = useAppConfigStore(state => state.isAudioFeatureEnabled); |
| const playAudio = async (eleId: string, track: string) => { | ||
| try { | ||
| const { audio_enabled, isAudioFeatureEnabled } = useAppConfigStore.getState(); | ||
| if (!audio_enabled || !isAudioFeatureEnabled) { | ||
| return; | ||
| } | ||
|
|
||
| const blobUrl = URL.createObjectURL(await fetch(track).then((res) => res.blob())); | ||
| const blockWinAudioElement = document.getElementById(eleId) as HTMLAudioElement; | ||
| if (blockWinAudioElement) { | ||
| blockWinAudioElement.setAttribute('src', blobUrl); // Rquired to make it work on an AppImage bundle | ||
| } | ||
|
|
||
| if (blockWinAudioElement.currentTime !== 0) return; | ||
| blockWinAudioElement.onended = () => { | ||
| blockWinAudioElement.currentTime = 0; | ||
| }; | ||
| blockWinAudioElement.play(); | ||
| } catch (err) { | ||
| console.error(`Failed to play block win sound: ${err}`); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix issues in the audio playback logic.
- Fix the typo in the comment: "Rquired" -> "Required".
- Add cleanup for the blob URL to prevent memory leaks.
- Add error handling for missing audio elements.
const playAudio = async (eleId: string, track: string) => {
try {
const { audio_enabled, isAudioFeatureEnabled } = useAppConfigStore.getState();
if (!audio_enabled || !isAudioFeatureEnabled) {
return;
}
- const blobUrl = URL.createObjectURL(await fetch(track).then((res) => res.blob()));
const blockWinAudioElement = document.getElementById(eleId) as HTMLAudioElement;
+ if (!blockWinAudioElement) {
+ throw new Error(`Audio element with id ${eleId} not found`);
+ }
+
+ const blobUrl = URL.createObjectURL(await fetch(track).then((res) => res.blob()));
if (blockWinAudioElement) {
- blockWinAudioElement.setAttribute('src', blobUrl); // Rquired to make it work on an AppImage bundle
+ blockWinAudioElement.setAttribute('src', blobUrl); // Required to make it work on an AppImage bundle
}
if (blockWinAudioElement.currentTime !== 0) return;
blockWinAudioElement.onended = () => {
blockWinAudioElement.currentTime = 0;
+ URL.revokeObjectURL(blobUrl);
};
blockWinAudioElement.play();
} catch (err) {
console.error(`Failed to play block win sound: ${err}`);
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const playAudio = async (eleId: string, track: string) => { | |
| try { | |
| const { audio_enabled, isAudioFeatureEnabled } = useAppConfigStore.getState(); | |
| if (!audio_enabled || !isAudioFeatureEnabled) { | |
| return; | |
| } | |
| const blobUrl = URL.createObjectURL(await fetch(track).then((res) => res.blob())); | |
| const blockWinAudioElement = document.getElementById(eleId) as HTMLAudioElement; | |
| if (blockWinAudioElement) { | |
| blockWinAudioElement.setAttribute('src', blobUrl); // Rquired to make it work on an AppImage bundle | |
| } | |
| if (blockWinAudioElement.currentTime !== 0) return; | |
| blockWinAudioElement.onended = () => { | |
| blockWinAudioElement.currentTime = 0; | |
| }; | |
| blockWinAudioElement.play(); | |
| } catch (err) { | |
| console.error(`Failed to play block win sound: ${err}`); | |
| } | |
| }; | |
| const playAudio = async (eleId: string, track: string) => { | |
| try { | |
| const { audio_enabled, isAudioFeatureEnabled } = useAppConfigStore.getState(); | |
| if (!audio_enabled || !isAudioFeatureEnabled) { | |
| return; | |
| } | |
| const blockWinAudioElement = document.getElementById(eleId) as HTMLAudioElement; | |
| if (!blockWinAudioElement) { | |
| throw new Error(`Audio element with id ${eleId} not found`); | |
| } | |
| const blobUrl = URL.createObjectURL(await fetch(track).then((res) => res.blob())); | |
| blockWinAudioElement.setAttribute('src', blobUrl); // Required to make it work on an AppImage bundle | |
| if (blockWinAudioElement.currentTime !== 0) return; | |
| blockWinAudioElement.onended = () => { | |
| blockWinAudioElement.currentTime = 0; | |
| URL.revokeObjectURL(blobUrl); | |
| }; | |
| blockWinAudioElement.play(); | |
| } catch (err) { | |
| console.error(`Failed to play block win sound: ${err}`); | |
| } | |
| }; |
index.html
Outdated
|
|
||
| <audio id="notification-player" preload="auto"> | ||
| <source src="/assets/Notification.wav" type="audio/mpeg" /> | ||
| </audio> | ||
| <audio id="success1-player" preload="auto"> | ||
| <source src="/assets/Success_Level_01.wav" type="audio/mpeg" /> | ||
| </audio> | ||
| <audio id="success2-player" preload="auto"> | ||
| <source src="/assets/Success_Level_02.wav" type="audio/mpeg" /> | ||
| </audio> | ||
| <audio id="success3-player" preload="auto"> | ||
| <source src="/assets/Success_Level_03.wav" type="audio/mpeg" /> | ||
| </audio> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Audio Elements Implementation – MIME Type Mismatch
Four new <audio> elements have been added for notification and success sounds. However, each <source> tag specifies type="audio/mpeg" while the file extensions are .wav. For WAV files, the correct MIME type is typically "audio/wav". Please update the MIME types to ensure proper playback across browsers.
Suggested Diff:
- <audio id="notification-player" preload="auto">
- <source src="/assets/Notification.wav" type="audio/mpeg" />
- </audio>
- <audio id="success1-player" preload="auto">
- <source src="/assets/Success_Level_01.wav" type="audio/mpeg" />
- </audio>
- <audio id="success2-player" preload="auto">
- <source src="/assets/Success_Level_02.wav" type="audio/mpeg" />
- </audio>
- <audio id="success3-player" preload="auto">
- <source src="/assets/Success_Level_03.wav" type="audio/mpeg" />
- </audio>
+ <audio id="notification-player" preload="auto">
+ <source src="/assets/Notification.wav" type="audio/wav" />
+ </audio>
+ <audio id="success1-player" preload="auto">
+ <source src="/assets/Success_Level_01.wav" type="audio/wav" />
+ </audio>
+ <audio id="success2-player" preload="auto">
+ <source src="/assets/Success_Level_02.wav" type="audio/wav" />
+ </audio>
+ <audio id="success3-player" preload="auto">
+ <source src="/assets/Success_Level_03.wav" type="audio/wav" />
+ </audio>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <audio id="notification-player" preload="auto"> | |
| <source src="/assets/Notification.wav" type="audio/mpeg" /> | |
| </audio> | |
| <audio id="success1-player" preload="auto"> | |
| <source src="/assets/Success_Level_01.wav" type="audio/mpeg" /> | |
| </audio> | |
| <audio id="success2-player" preload="auto"> | |
| <source src="/assets/Success_Level_02.wav" type="audio/mpeg" /> | |
| </audio> | |
| <audio id="success3-player" preload="auto"> | |
| <source src="/assets/Success_Level_03.wav" type="audio/mpeg" /> | |
| </audio> | |
| <audio id="notification-player" preload="auto"> | |
| <source src="/assets/Notification.wav" type="audio/wav" /> | |
| </audio> | |
| <audio id="success1-player" preload="auto"> | |
| <source src="/assets/Success_Level_01.wav" type="audio/wav" /> | |
| </audio> | |
| <audio id="success2-player" preload="auto"> | |
| <source src="/assets/Success_Level_02.wav" type="audio/wav" /> | |
| </audio> | |
| <audio id="success3-player" preload="auto"> | |
| <source src="/assets/Success_Level_03.wav" type="audio/wav" /> | |
| </audio> |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/App/AppEffects.tsx (1)
28-31:⚠️ Potential issueAdd error handling for initialization sequence.
The initialization sequence should handle potential failures gracefully.
Apply this diff to improve error handling:
- await fetchAppConfig(); - await fetchAudioFeatureEnabled(); - await setMiningNetwork(); - initAnimationAudio(); + try { + await fetchAppConfig(); + await fetchAudioFeatureEnabled(); + await setMiningNetwork(); + initAnimationAudio(); + } catch (error) { + console.error('Failed to initialize app:', error); + // Consider showing a user-friendly error message + }src/store/useBlockchainVisualisationStore.ts (1)
92-118:⚠️ Potential issueFix issues in the audio playback logic.
Apply this diff to fix the issues:
const playAudio = async (eleId: string, track: string) => { try { const { audio_enabled, isAudioFeatureEnabled } = useAppConfigStore.getState(); if (!audio_enabled || !isAudioFeatureEnabled) { return; } - const blobUrl = URL.createObjectURL(await fetch(track).then((res) => res.blob())); const blockWinAudioElement = document.getElementById(eleId) as HTMLAudioElement; + if (!blockWinAudioElement) { + throw new Error(`Audio element with id ${eleId} not found`); + } + + const blobUrl = URL.createObjectURL(await fetch(track).then((res) => res.blob())); if (blockWinAudioElement) { - blockWinAudioElement.setAttribute('src', blobUrl); // Rquired to make it work on an AppImage bundle + blockWinAudioElement.setAttribute('src', blobUrl); // Required to make it work on an AppImage bundle } if (blockWinAudioElement.currentTime !== 0) return; blockWinAudioElement.onended = () => { blockWinAudioElement.currentTime = 0; + URL.revokeObjectURL(blobUrl); }; blockWinAudioElement.play(); } catch (err) { console.error(`Failed to play block win sound: ${err}`); } };
🧹 Nitpick comments (3)
src/App/AppEffects.tsx (1)
33-33: Enhance error logging.The current error logging could be more descriptive to aid in debugging.
Apply this diff to improve error logging:
- void initialize().catch((e) => console.error('Failed to initialize UI config: ', e)); + void initialize().catch((e) => { + const errorMessage = e instanceof Error ? e.message : String(e); + console.error(`Failed to initialize UI config: ${errorMessage}`, { + stack: e instanceof Error ? e.stack : undefined, + cause: e instanceof Error ? e.cause : undefined + }); + });src/store/useBlockchainVisualisationStore.ts (2)
56-80: Consider using an enum for audio tiers.The current implementation uses magic numbers for tiers. Using an enum would improve type safety and maintainability.
Add this enum at the beginning of the file:
enum AudioTier { Level1 = 1, Level2 = 2, Level3 = 3 }Then update the functions:
-function selectAudioAssetOnSuccessTier(tier: number) { +function selectAudioAssetOnSuccessTier(tier: AudioTier) { switch (tier) { - case 1: + case AudioTier.Level1: return 'assets/Success_Level_01.wav'; - case 2: + case AudioTier.Level2: return 'assets/Success_Level_02.wav'; - case 3: + case AudioTier.Level3: return 'assets/Success_Level_03.wav'; default: throw new Error('Invalid tier'); } } -function getAudioElementId(tier: number) { +function getAudioElementId(tier: AudioTier) { switch (tier) { - case 1: + case AudioTier.Level1: return 'success1-player'; - case 2: + case AudioTier.Level2: return 'success2-player'; - case 3: + case AudioTier.Level3: return 'success3-player'; default: throw new Error('Invalid tier'); } }
124-153: Add debouncing for audio playback in win handler.Multiple rapid wins could cause audio playback issues. Consider adding debouncing to prevent overlapping sounds.
Add this utility at the beginning of the file:
import debounce from 'lodash/debounce'; const debouncedPlayAudio = debounce((tier: number) => { void playBlockWinAudio(tier); }, 500, { leading: true, trailing: false });Then update the win handler:
const handleWin = async (coinbase_transaction: TransactionInfo, balance: WalletBalance, canAnimate: boolean) => { const blockHeight = Number(coinbase_transaction?.mined_in_block_height); const earnings = coinbase_transaction.amount; console.info(`Block #${blockHeight} mined! Earnings: ${earnings}`); const successTier = getSuccessTier(earnings); if (canAnimate) { useMiningStore.getState().setMiningControlsEnabled(false); setAnimationState(successTier); + debouncedPlayAudio(successTier); useBlockchainVisualisationStore.setState({ earnings }); // ... rest of the code
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
index.html(1 hunks)src/App/AppEffects.tsx(2 hunks)src/containers/floating/Settings/sections/general/GeneralSettings.tsx(2 hunks)src/store/useBlockchainVisualisationStore.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/containers/floating/Settings/sections/general/GeneralSettings.tsx
- index.html
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: tauri-build
- GitHub Check: cargo checks (fmt, clippy, check)
- GitHub Check: Check signed commits
- GitHub Check: Check i18n
| export const initAnimationAudio = () => { | ||
| window.glApp.initAudio(playNotificationAudio, (tier: number) => playBlockWinAudio(tier)); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for audio initialization.
The initAnimationAudio function should handle potential initialization failures.
Apply this diff to improve error handling:
export const initAnimationAudio = () => {
+ if (typeof window.glApp?.initAudio !== 'function') {
+ console.error('Audio initialization failed: window.glApp.initAudio is not available');
+ return;
+ }
window.glApp.initAudio(playNotificationAudio, (tier: number) => playBlockWinAudio(tier));
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const initAnimationAudio = () => { | |
| window.glApp.initAudio(playNotificationAudio, (tier: number) => playBlockWinAudio(tier)); | |
| }; | |
| export const initAnimationAudio = () => { | |
| if (typeof window.glApp?.initAudio !== 'function') { | |
| console.error('Audio initialization failed: window.glApp.initAudio is not available'); | |
| return; | |
| } | |
| window.glApp.initAudio(playNotificationAudio, (tier: number) => playBlockWinAudio(tier)); | |
| }; |
| async function playNotificationAudio() { | ||
| playAudio('notification-player', 'assets/Notification.wav'); | ||
| } | ||
|
|
||
| async function playBlockWinAudio(successTier: number) { | ||
| const asset = selectAudioAssetOnSuccessTier(successTier); | ||
| const player = getAudioElementId(successTier); | ||
| playAudio(player, asset); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add retry logic for audio playback.
The audio playback functions should handle network failures gracefully.
Apply this diff to add retry logic:
+const MAX_RETRIES = 3;
+const RETRY_DELAY = 1000;
+async function retryWithDelay<T>(fn: () => Promise<T>, retries = MAX_RETRIES): Promise<T> {
+ try {
+ return await fn();
+ } catch (error) {
+ if (retries > 0) {
+ await new Promise(resolve => setTimeout(resolve, RETRY_DELAY));
+ return retryWithDelay(fn, retries - 1);
+ }
+ throw error;
+ }
+}
async function playNotificationAudio() {
- playAudio('notification-player', 'assets/Notification.wav');
+ await retryWithDelay(() => playAudio('notification-player', 'assets/Notification.wav'));
}
async function playBlockWinAudio(successTier: number) {
const asset = selectAudioAssetOnSuccessTier(successTier);
const player = getAudioElementId(successTier);
- playAudio(player, asset);
+ await retryWithDelay(() => playAudio(player, asset));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function playNotificationAudio() { | |
| playAudio('notification-player', 'assets/Notification.wav'); | |
| } | |
| async function playBlockWinAudio(successTier: number) { | |
| const asset = selectAudioAssetOnSuccessTier(successTier); | |
| const player = getAudioElementId(successTier); | |
| playAudio(player, asset); | |
| } | |
| const MAX_RETRIES = 3; | |
| const RETRY_DELAY = 1000; | |
| async function retryWithDelay<T>(fn: () => Promise<T>, retries = MAX_RETRIES): Promise<T> { | |
| try { | |
| return await fn(); | |
| } catch (error) { | |
| if (retries > 0) { | |
| await new Promise(resolve => setTimeout(resolve, RETRY_DELAY)); | |
| return retryWithDelay(fn, retries - 1); | |
| } | |
| throw error; | |
| } | |
| } | |
| async function playNotificationAudio() { | |
| await retryWithDelay(() => playAudio('notification-player', 'assets/Notification.wav')); | |
| } | |
| async function playBlockWinAudio(successTier: number) { | |
| const asset = selectAudioAssetOnSuccessTier(successTier); | |
| const player = getAudioElementId(successTier); | |
| await retryWithDelay(() => playAudio(player, asset)); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
package.json(1 hunks)src-tauri/Cargo.toml(3 hunks)src-tauri/src/main.rs(2 hunks)src-tauri/tauri.conf.json(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- src-tauri/Cargo.toml
- src-tauri/tauri.conf.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: tauri-build
🔇 Additional comments (2)
src-tauri/src/main.rs (2)
1314-1314: Inconsistency with AI-generated summary.The
get_airdrop_tokenscommand is still present in the code, but the AI summary indicated it was removed.Likely an incorrect or invalid review comment.
1315-1318: LGTM! Audio-related commands are properly integrated.The new commands for audio functionality are well-organized and follow the existing command naming patterns.
| @@ -1066,6 +1066,7 @@ fn main() { | |||
| }; | |||
| let app_state_clone = app_state.clone(); | |||
| let app = tauri::Builder::default() | |||
| .plugin(tauri_plugin_fs::init()) | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify file system permissions for audio files.
The addition of tauri_plugin_fs requires proper configuration of file system permissions to ensure secure access to audio files.
Run the following script to check the file system permissions configuration:
🏁 Script executed:
#!/bin/bash
# Description: Check file system permissions in Tauri configuration.
# Test: Search for file system permissions in capabilities configuration.
rg -A 5 "fs" src-tauri/capabilities/Length of output: 614
Action Required: Update Inconsistent Command & Confirm Permission Scope
- Verified that the Tauri capabilities configuration (in
src-tauri/capabilities/default.json) includes proper file system permissions for audio files. The settings for"fs:allow-read-file"and"fs:allow-read-dir"correctly limit access to the audio directory/resource. - The
get_airdrop_tokenscommand is still present (line 1314 insrc-tauri/src/main.rs), which contradicts the AI summary that indicated its removal. Please review whether this command should be removed or if the summary needs updating.
Description
Implements #1514
Add multiple audio sources to the app that plays when user wins block or replays one from his wallet history. There is also notification sound that precedes before the main block win sound plays.
Motivation and Context
Give users audio feedback when new block is mined.
How Has This Been Tested?
build new
glApp.jsfrom this branch https://github.com/shanimal08/TARI_MINER_APP_VISUAL/pull/9replace the
glApp.jsinpublic/assets/glApp.jstest sound when:
What process can a PR reviewer use to test or verify this change?
same as above
Breaking Changes
Summary by CodeRabbit
New Features
Chores