feat: Add AudioManager; Music/SFX modding support - #1384
Conversation
EttyKitty
changed the title
EttyKitty
changed the title
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review Roast 🔥Verdict: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
🏆 Best part: The 💀 Worst part: That header comment at line 6 is confidently wrong about both directory paths. It's like giving someone directions to the wrong city while insisting you know the way. 📊 Overall: Like a Guardsman who forgot his ammo but remembered his ceremonial sword — the intent is noble, the execution needs a mag-load. Files Reviewed (3 files)
Fix these issues in Kilo Cloud Previous Review Summaries (4 snapshots, latest commit 669a8d6)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 669a8d6)Verdict: No Issues Found | Recommendation: Merge Oh wait, this PR is actually clean. I need to sit down. I had my flamethrower warmed up and everything. 📊 Overall: Like finding a unicorn in production — I didn't think clean PRs existed anymore, but here we are. Files Reviewed (2 files)
Previous review (commit 0c57b7f)Verdict: No Issues Found | Recommendation: Merge Oh wait, this PR is actually clean. I need to sit down. I had my flamethrower warmed up and everything. 📊 Overall: Like finding a unicorn in production — I didn't think clean PRs existed anymore, but here we are. Files Reviewed (1 file)
Previous review (commit deac269)Verdict: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
🏆 Best part: The 💀 Worst part: 📊 Overall: Like a Guardsman who forgot his ammo but remembered his ceremonial sword — the intent is noble, the execution needs a mag-load. Files Reviewed (26 files)
Fix these issues in Kilo Cloud Previous review (commit 395485c)Verdict: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
🏆 Best part: The 💀 Worst part: 📊 Overall: Like a Guardsman who forgot his ammo but remembered his ceremonial sword — the intent is noble, the execution needs a mag-load. Files Reviewed (26 files)
Reviewed by step-3.7-flash · Input: 187.8K · Output: 86.7K · Cached: 3.5M |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 26 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 15 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
These help with passing arguments into callbacks.
deac269 to
0c57b7f
Compare
669a8d6 to
740537b
Compare
|
|
||
| USER_AUDIO_DIR = program_directory + "Audio/"; | ||
| USER_MUSIC_DIR = USER_AUDIO_DIR + "Music/"; | ||
| USER_SFX_DIR = USER_AUDIO_DIR + "SFX//"; |
There was a problem hiding this comment.
🔥 The Roast: USER_SFX_DIR got an extra slash — "SFX//" instead of "SFX/". This typo is the audio equivalent of putting two right socks on. The OS will probably tolerate it, but it's embarrassing.
🩹 The Fix:
| USER_SFX_DIR = USER_AUDIO_DIR + "SFX//"; | |
| USER_SFX_DIR = USER_AUDIO_DIR + "SFX/"; |
📏 Severity: nitpick
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| if (fadeout == 1) { | ||
| audio_sound_gain(snd_defeat, 0, 2000); | ||
| global.audio_manager.stop_music(2000) |
There was a problem hiding this comment.
🔥 The Roast: Missing semicolon on stop_music(2000). The old code had it, the new code dropped it. This is the coding equivalent of forgetting your keys — you might still get in the house, but it's awkward.
🩹 The Fix:
| global.audio_manager.stop_music(2000) | |
| global.audio_manager.stop_music(2000); |
📏 Severity: nitpick
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| /// crossfading, and volume control. | ||
| /// | ||
| /// Audio loading tiers (user overrides shipped): | ||
| /// 1. Shipped: `working_directory/Audio/` |
There was a problem hiding this comment.
🔥 The Roast: These header comments are lying harder than a Guardsman's recruitment poster. They claim shipped audio lives at working_directory/Audio/ and user audio at AppData/Local/ChapterMaster/Audio/, but the actual code uses working_directory + "/Custom Files/Audio/" for shipped and program_directory + "Audio/" for user. Anyone reading the comments will drop their mod files in the wrong place and wonder why nothing works.
🩹 The Fix: Update the header to match the real paths:
| /// 1. Shipped: `working_directory/Audio/` | |
| /// Audio loading tiers (user overrides shipped): | |
| /// 1. Shipped: `working_directory/Custom Files/Audio/` | |
| /// 2. User: `Audio/` (relative -> program_directory + "Audio/") |
📏 Severity: suggestion
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
1 participant
Summary by cubic
Adds
AudioManagerto centralize music and SFX with runtime modding and context playlists. Smooth crossfades replace direct audio calls, with macro-based SFX and settings-driven volume.New Features
Custom Files/Audioact as defaults. Folders auto-create.play_playlist(context),play_track(name),play_sfx(name),stop_music(fade_ms).datafiles/audio_modding.mdwith folder structure, contexts, and override priority.Refactors
audio_*calls withglobal.audio_manageracross menus, combat, creation, buttons, and UI; removedscr_musicand legacy controller fields; addedscr_predicatehelpers for fade-out callbacks.AudioManageranddiscover()on startup;SettingsManager.apply_audio()now sets master gain and forwards music/SFX volume toAudioManager.SFX_CLICK,SFX_IDENTIFY,SFX_END_TURN,SFX_STC) to avoid string typos.Screenshots/with clearer filenames; minor logging tidy-up.Written for commit 740537b. Summary will update on new commits.