feat: add volume sliders for sound control#108
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR replaces the previous tri-state (Enabled/Disabled/Infrequent) sound configuration with a per-sound volume slider (0–100%) plus an independent "infrequent" toggle. Disabled and reduced sounds are merged into a single "Configured sounds" category, and a legacy-data migration path is added.
Changes:
- Replaced
SoundStateenum and disable-only logic with asoundVolumesmap and per-sound volume scaling in the playback mixin. - Reworked the config screen to show a sub-category per sound with a volume slider and infrequent toggle, and combined "disabled"/"infrequent" tabs into "configured".
- Added migration of the legacy
soundsset into the newsoundVolumesmap and updated docs/dependency versions.
Reviewed changes
Copilot reviewed 9 out of 13 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| versions/dependencies/1.21.properties | Bumped cloth_version. |
| versions/dependencies/1.21.11.properties | Bumped cloth_version. |
| src/main/resources/assets/soundsbegone/lang/zh_cn.json | No content change (line ending). |
| src/main/resources/assets/soundsbegone/lang/ru_ru.json | No content change (line ending); not updated for new keys. |
| src/main/resources/assets/soundsbegone/lang/en_us.json | Replaced state/category keys with new volume/infrequent/configured keys. |
| src/main/resources/assets/soundsbegone/lang/de_de.json | No content change (line ending); not updated for new keys. |
| src/main/java/gg/meza/soundsbegone/client/mixin/AbstractSoundInstanceMixin.java | Added volume scaling, tracks all sounds, refactored into helper. |
| src/main/java/gg/meza/soundsbegone/client/ConfigScreen.java | Rewrote per-sound entry as sub-category with slider + toggle. |
| src/main/java/gg/meza/soundsbegone/SoundState.java | Removed enum. |
| src/main/java/gg/meza/soundsbegone/ConfigData.java | Added soundVolumes map, kept legacy sounds set. |
| src/main/java/gg/meza/soundsbegone/Config.java | New volume API, removed state API, added legacy migration. |
| README.md | Updated wording to reflect volume control. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .setSaveConsumer(newVolume -> { | ||
| int oldVolume = SoundsBeGoneClient.config.getSoundVolume(key); | ||
| if (oldVolume == 0 && newVolume > 0) { | ||
| SoundsBeGoneClient.telemetry.unmutedSound(key); | ||
| } else if (oldVolume > 0 && newVolume == 0) { | ||
| SoundsBeGoneClient.telemetry.mutedSound(key); | ||
| } | ||
| SoundsBeGoneClient.config.setSoundVolume(key, newVolume); | ||
| }) |
| int currentVolume = SoundsBeGoneClient.config.getSoundVolume(key); | ||
| boolean isInfrequent = SoundsBeGoneClient.config.isSoundInfrequent(key); | ||
|
|
||
| SubCategoryBuilder sub = builder.startSubCategory(Component.translatable(key)); |
| SoundsBeGoneConfig.LOGGER.error("Cause: {}", e.getCause().getClass().getSimpleName()); | ||
| if (e.getCause().getClass() == IllegalStateException.class) { |
| public Set<String> infrequentSounds() { | ||
| return configData.infrequent; | ||
| } |
| private void migrateLegacySounds() { | ||
| if (configData.sounds != null && !configData.sounds.isEmpty()) { |
|
Hey! I’ve been wanting to use this mod for a while, but I needed finer control over sound levels rather than just a full mute/disable toggle. I noticed there was an open issue requesting this, so I went ahead and implemented per-sound volume sliders (0–100%) along with a legacy config migration. Hope the implementation looks okay — happy to change anything that doesn’t fit the project style. Thanks for mod! |
|
Hey, this is fantastic, thanks for the PR! I don't have much time this week to pay proper attention to it, but I'll take a look as soon as I can. Also for the translations, please only modify the en_us.json. All the others get populated by the crowdin integration later. I'll get back to this as soon as I can! |
|
Thanks for taking a look! I’m going through the Copilot comments now and will push fixes for the valid ones tomorrow. |
| public Set<String> disabledSounds() { | ||
| return configData.sounds; | ||
| return configData.soundVolumes.entrySet().stream() | ||
| .filter(e -> e.getValue() == 0) | ||
| .map(Map.Entry::getKey) | ||
| .collect(Collectors.toSet()); | ||
| } | ||
|
|
||
| public Set<String> infrequentSounds() { | ||
| return configData.infrequent; | ||
| public Set<String> reducedSounds() { | ||
| return configData.soundVolumes.entrySet().stream() | ||
| .filter(e -> e.getValue() > 0 && e.getValue() < 100) | ||
| .map(Map.Entry::getKey) | ||
| .collect(Collectors.toSet()); | ||
| } | ||
|
|
||
| public SoundState getSoundState(String sound) { | ||
| if (isSoundDisabled(sound)) { | ||
| return SoundState.DISABLED; | ||
| } else if (isSoundInfrequent(sound)) { | ||
| return SoundState.INFREQUENT; | ||
| } else { | ||
| return SoundState.ENABLED; | ||
| } | ||
| public Set<String> infrequentSounds() { | ||
| return Set.copyOf(configData.infrequent); | ||
| } |
| for (String sound : configData.sounds) { | ||
| configData.soundVolumes.putIfAbsent(sound, 0); | ||
| } | ||
| configData.sounds.clear(); |
| int volumePercent = SoundsBeGoneClient.config.getSoundVolume(soundId); | ||
| if (volumePercent == 0) { | ||
| SoundsBeGoneClient.telemetry.blockedSound(soundId); | ||
| cir.setReturnValue(0.0F); | ||
| cir.cancel(); | ||
| track(soundId); | ||
| return; | ||
| } else if (volumePercent != 100) { | ||
| float newVolume = this.volume * (volumePercent / 100.0f); | ||
| cir.setReturnValue(newVolume); | ||
| cir.cancel(); | ||
| } |
| } else if (volumePercent != 100) { | ||
| float newVolume = this.volume * (volumePercent / 100.0f); | ||
| cir.setReturnValue(newVolume); | ||
| cir.cancel(); | ||
| } |
| int currentVolume = SoundsBeGoneClient.config.getSoundVolume(key); | ||
| boolean isInfrequent = SoundsBeGoneClient.config.isSoundInfrequent(key); | ||
|
|
||
| SubCategoryBuilder sub = builder.startSubCategory(Component.translatable(key)); | ||
| sub.setExpanded(false); | ||
|
|
||
| sub.add(builder.startIntSlider( | ||
| Component.translatable("soundsbegone.config.sound.volume"), | ||
| currentVolume, 0, 100) | ||
| .setDefaultValue(100) |
| for (String sound : disabledSounds) { | ||
| this.configData.soundVolumes.putIfAbsent(sound, 0); |
| public double frequencyPercentage = 10; | ||
| public Set<String> sounds = new HashSet<>(); | ||
| public Map<String, Integer> soundVolumes = new HashMap<>(); | ||
| public Set<String> sounds = new HashSet<>(); // Legacy field, migrated to soundVolumes on load |
| .filter(s -> SoundsBeGoneClient.config.getSoundVolume(s) == 100 && !SoundsBeGoneClient.config.isSoundInfrequent(s)) | ||
| .sorted() | ||
| .forEach((key) -> general.addEntry(constructOption(entryBuilder, key))); |
| @Mixin(AbstractSoundInstance.class) | ||
| public class AbstractSoundInstanceMixin { | ||
|
|
||
| @Shadow protected float volume; |
| SoundsBeGoneConfig.LOGGER.error("Cause: " + e.getCause().getClass().getSimpleName()); | ||
| if (e.getCause().getClass() == IllegalStateException.class) { | ||
| Throwable cause = e.getCause(); | ||
| SoundsBeGoneConfig.LOGGER.error("Cause: {}", cause != null ? cause.getClass().getSimpleName() : "null"); |
|
Hey @meza, hope you're having a good week! Just wanted to gently check in — no rush at all, but I wanted to make sure my PR didn't fall off your radar. When you do get a chance to merge and release, I'd love to include this mod in my modpack (SkyBlock Enhanced) once it's out, which is why I would appreciate it if the update could be soon, but absolutely no pressure, I know life gets busy! Let me know if anything needs tweaking whenever you have a free moment. |
| public int getSoundVolume(String sound) { | ||
| return configData.soundVolumes.getOrDefault(sound, 100); | ||
| } | ||
|
|
||
| public boolean isSoundDisabled(String sound) { | ||
| return configData.sounds.contains(sound); | ||
| public void setSoundVolume(String sound, int volume) { | ||
| if (volume == 100) { | ||
| configData.soundVolumes.remove(sound); | ||
| } else { | ||
| configData.soundVolumes.put(sound, volume); | ||
| } | ||
| } |
| public void initConfig() { | ||
| renameConfigFile(); | ||
| if (Files.exists(configPath)) { | ||
| try { | ||
| BufferedReader reader = Files.newBufferedReader(configPath); | ||
| configData = GSON.fromJson(reader, ConfigData.class); | ||
| reader.close(); | ||
| } catch (IOException | JsonParseException e) { | ||
| SoundsBeGoneConfig.LOGGER.error("Cause: " + e.getCause().getClass().getSimpleName()); | ||
| if (e.getCause().getClass() == IllegalStateException.class) { | ||
| Throwable cause = e.getCause(); | ||
| LOGGER.error("Failed to load config", e); | ||
| if (cause != null && cause.getClass() == IllegalStateException.class) { | ||
| convertConfig(configPath); | ||
| return; | ||
| } | ||
| throw new SerializationException(e); | ||
| } | ||
| } | ||
| migrateLegacySounds(); | ||
| } |
| "soundsbegone.config.title" : "Sounds Be Gone Config", | ||
| "soundsbegone.config.category.latest" : "Played in the last 60 seconds", | ||
| "soundsbegone.config.category.disabled" : "Disabled sounds", | ||
| "soundsbegone.config.category.infrequent" : "Infrequent sounds", | ||
| "soundsbegone.config.category.configured" : "Configured sounds", | ||
| "soundsbegone.config.category.settings" : "Settings", |
| "soundsbegone.config.infrequent.frequency.title" : "Frequency of infrequent sounds (percentage)", | ||
| "soundsbegone.config.infrequent.frequency.tooltip" : "What percentage of infrequent sounds should be played? (0.001 - 99.999)", | ||
| "soundsbegone.config.sound.state.enabled" : "Enabled", | ||
| "soundsbegone.config.sound.state.disabled" : "Disabled", | ||
| "soundsbegone.config.sound.state.infrequent" : "Infrequent", | ||
| "soundsbegone.config.sound.volume" : "Volume", | ||
| "soundsbegone.config.sound.infrequent" : "Infrequent", |
| } else if (volumePercent != 100) { | ||
| track(soundId); | ||
| float newVolume = this.volume * (volumePercent / 100.0f); | ||
| cir.setReturnValue(newVolume); | ||
| cir.cancel(); | ||
| } | ||
|
|
||
| track(soundId); | ||
| } |
|
I checked it out. The main reason this feature doesn't exist yet is the UX design of it needs to be good enough and matching the future directions. Sadly this doesn't do that just yet. The settings screen is really not what it should be and finding the right way is the difficult bit. Same with the config change, that's really not how it should be. It's a good basis, but it's not something I feel comfortable merging in just yet. I'll see if I can come up with something in the coming days though |
|
Okay, thanks for looking it at. |
|
Hey, have you had a chance to look into the feature? I'd love to have this in my modpack. If you've decided not to add it, or want to implement your own version of it, would you be okay with me forking the mod to add it myself until you get around to adding it? I'd of course credit you as the original creator and link back to your repo. |
|
yeah it's coming soon |
Closes #55
Summary
This PR changes the mod from just a simple disable/enable toggle to using volume sliders (0–100%) along with a separate Infrequent toggle. This gives players a lot more control over how loud specific sounds are!
(Note: I used some AI to help me write and structure this code, but I've tested it thoroughly to make sure it works!)
What changed
Sound -> Volume.0%volume so players don't lose their settings).AbstractSoundInstanceMixinnow grabs the sound's default volume and multiplies it by the percentage from the config. If it's set to0%, it cancels the sound completely.Testing
./gradlew test buildAndCollectpassed.disabled_sounds.jsonfiles update correctly.