Conversation
…ates in create sound data cross save
…ntervention The custom sound system correctly handles save/load without needing to clear and rebuild the custom sound bank. Sound indices are properly saved and restored with the Game struct. Creatures maintain their spells and research is preserved. Tested and verified working with custom campaigns.
c0bf553 to
a5e6cd7
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces infrastructure for configurable/named sound IDs and runtime-loaded custom sounds, then updates gameplay code to use those named IDs and a unified sample-ID space (effects/speech/custom) rather than hard-coded numbers and explicit bank IDs.
Changes:
- Add
sounds.cfgloading (config_sounds.*) and replace many hard-coded sound IDs with cachedsnd_*globals. - Remove
SoundBankIDfrom the low-level sound API and switch to unified sample IDs across sound backends. - Add a C++
SoundManager(plus Lua bindings) to load/register/play custom sounds and support creature sound overrides.
Reviewed changes
Copilot reviewed 46 out of 47 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/bflib_sound.h |
Updates sound API signatures to remove bank IDs and expose unified-ID helpers. |
src/bflib_sound.c |
Adjusts emitter/sample operations to the new API and unified sample IDs. |
src/bflib_sndlib.cpp |
Implements unified ID resolution (effects/speech/custom) and runtime custom bank loading. |
src/sounds.c |
Adds negative-ID → custom unified-ID translation in thing_play_sample and updates some walking/ambient sound usage. |
src/config_sounds.c / src/config_sounds.h |
Adds sounds.cfg parsing + cached snd_* globals. |
config/fxdata/sounds.cfg |
Introduces default sound name → ID mappings. |
src/sound_manager.h / src/sound_manager.cpp |
Adds SoundManager + C API wrappers + creature-config bridge functions. |
src/lua_api_sound.c / src/lua_api_sound.h |
Exposes SoundManager functionality to Lua scripts. |
src/config_crtrmodel.c |
Adds creature cfg support for custom sound file paths and CustomSound syntax. |
src/dungeon_stats.c |
Loads sounds.cfg as part of stats/config loading. |
src/config_campaigns.c |
Loads campaign/mod sound overrides during campaign changes. |
src/game_saves.c |
Alters save-load config initialization sequence. |
src/* (many) |
Replaces numeric sound IDs with snd_* variables and updates API calls to new signatures. |
Makefile |
Adds new objects and bumps C++ standard to C++20. |
.gitignore, keeperfx.code-workspace |
Dev-environment housekeeping updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| TbBool sound_manager_init(void) { | ||
| return KeeperFX::SoundManager::getInstance().initialize(); | ||
| } | ||
|
|
||
| SoundEmitterID sound_manager_minimal_play_effect(SoundSmplTblID sample_id, long priority, SoundVolume volume) { | ||
| return KeeperFX::SoundManager::getInstance().playEffect(sample_id, priority, volume); | ||
| } | ||
|
|
||
| void sound_manager_minimal_play_creature_sound(struct Thing* thing, long sound_type, long priority) { | ||
| KeeperFX::SoundManager::getInstance().playCreatureSound(thing, sound_type, priority); | ||
| } | ||
|
|
||
| void sound_manager_minimal_stop_effect(SoundEmitterID emitter_id) { | ||
| KeeperFX::SoundManager::getInstance().stopEffect(emitter_id); | ||
| } | ||
|
|
||
| TbBool sound_manager_minimal_play_music(int track_number) { | ||
| return KeeperFX::SoundManager::getInstance().playMusic(track_number); | ||
| } | ||
|
|
||
| void sound_manager_minimal_stop_music(void) { | ||
| KeeperFX::SoundManager::getInstance().stopMusic(); | ||
| } | ||
|
|
||
| SoundSmplTblID sound_manager_minimal_load_custom_sound(const char* name, const char* filepath) { | ||
| return KeeperFX::SoundManager::getInstance().loadCustomSound(name, filepath); | ||
| } | ||
|
|
||
| SoundSmplTblID sound_manager_minimal_get_custom_sound_id(const char* name) { | ||
| return KeeperFX::SoundManager::getInstance().getCustomSoundId(name); | ||
| } | ||
|
|
||
| TbBool sound_manager_minimal_set_creature_sound(const char* creature_model, const char* sound_type, const char* custom_sound_name) { | ||
| return KeeperFX::SoundManager::getInstance().setCreatureSound(creature_model, sound_type, custom_sound_name); | ||
| } | ||
|
|
||
| TbBool sound_manager_minimal_is_custom_sound_loaded(const char* name) { | ||
| return KeeperFX::SoundManager::getInstance().isCustomSoundLoaded(name); | ||
| } | ||
|
|
||
| void sound_manager_minimal_print_stats(void) { | ||
| KeeperFX::SoundManager::getInstance().printStats(); | ||
| } | ||
|
|
||
| void sound_manager_clear_custom_sounds(void) { | ||
| KeeperFX::SoundManager::getInstance().clearCustomSounds(); | ||
| } |
There was a problem hiding this comment.
The C API wrappers here are named sound_manager_minimal_*, but the public header declares sound_manager_play_effect, sound_manager_load_custom_sound, sound_manager_get_custom_sound_id, etc. Call sites (eg. Lua API) use the non-minimal_ names, so this will fail to link. Rename/export the wrappers to match the header (or update the header + all call sites consistently).
| // Get custom sound ID | ||
| SoundSmplTblID SoundManager::getCustomSoundId(const std::string& name) const { | ||
| auto it = custom_sounds_.find(name); | ||
| if (it == custom_sounds_.end()) { | ||
| printf("[SoundManager] Custom sound '%s' not found\n", name.c_str()); | ||
| return 0; | ||
| } | ||
|
|
||
| printf("[SoundManager] Found custom sound '%s' as sample %d\n", | ||
| name.c_str(), it->second.sample_id); | ||
| return it->second.sample_id; | ||
| } | ||
|
|
||
| // === Named Sound Registry === | ||
|
|
||
| // Get sample ID for a named sound (built-in or custom) | ||
| SoundSmplTblID SoundManager::getSoundId(const char* name) const { | ||
| if (name == nullptr || name[0] == '\0') { | ||
| return 0; | ||
| } | ||
|
|
||
| std::string name_str(name); | ||
|
|
||
| // First check the built-in sound registry | ||
| auto it = sound_registry_.find(name_str); | ||
| if (it != sound_registry_.end()) { | ||
| SoundSmplTblID id = it->second.sample_id; | ||
| // If count > 1, pick a random sample from consecutive IDs | ||
| if (it->second.count > 1) { | ||
| id = it->second.sample_id + (UNSYNC_RANDOM(it->second.count)); | ||
| } | ||
| return id; | ||
| } | ||
|
|
||
| // Fall back to custom sounds | ||
| auto cit = custom_sounds_.find(name_str); | ||
| if (cit != custom_sounds_.end() && cit->second.loaded) { | ||
| return cit->second.sample_id; | ||
| } | ||
|
|
||
| printf("[SoundManager] Sound '%s' not found in registry\n", name); | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
Custom sounds are stored as bank indices (CustomSoundEntry.sample_id), but getCustomSoundId() / the custom-sound fallback in getSoundId() return that raw bank index rather than a unified sample ID (custom offset + index). Passing that value into play_non_3d_sample()/playEffect() will be interpreted as an effect sample and can play the wrong sound or error. Return unified IDs from these getters (or clearly separate “bank index” vs “unified sample id” in the API).
ec546e3 to
547f756
Compare
547f756 to
530ba22
Compare
530ba22 to
aa24d0f
Compare
aa24d0f to
a100043
Compare
a100043 to
794f136
Compare
No description provided.