Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CI Skip] Allow server owners to fully customize sounds #2844

Merged
merged 30 commits into from
Jun 29, 2023

Conversation

TheBusyBiscuit
Copy link
Member

@TheBusyBiscuit TheBusyBiscuit commented Feb 26, 2021

Description

This pull request adds the SoundService and the SoundEffect enum which allow us to easily play sounds
but also allow server owners to easily configure and customize sounds.
Note here that I opted for the minecraft-ids of sounds rather than the bukkit constants because this will allow
server owners to use completely custom sounds in combination with resource packs.
If a sound does not exist, the player will simply not hear anything, no error will be thrown.

When values are misconfigured, a helpful warning will appear in the console.
This will also protect us against Mojang sound changes in the future, as we don't have to worry about the support of changed enum constants or such.

Here is an example of the new config file:

PORTABLE_CRAFTER_OPEN_SOUND:
  sound: entity.wolf.howl
  volume: 1.0
  pitch: 0.123456789
SPLINT_CONSUME_SOUND:
  sound: entity.skeleton.hurt
  volume: 1.0
  pitch: 1.0
TRASH_CAN_OPEN_SOUND:
  sound: i.dont.exist.btw.spinach.sucks.and.pineapple.on.pizza.rules
  volume: 1.0
  pitch: 1.0

ToDo (Why this is still a draft)

  • MockBukkit does not support String-sounds atm, so Unit Tests will fail, I will need to update MockBukkit for that first
  • I would like to add Unit Tests for this sound service too
  • Not all sounds have been converted yet

Checklist

  • I have fully tested the proposed changes and promise that they will not break everything into chaos.
  • I have also tested the proposed changes in combination with various popular addons and can confirm my changes do not break them.
  • I followed the existing code standards and didn't mess up the formatting.
  • I did my best to add documentation to any public classes or methods I added.
  • I have added Nonnull and Nullable annotations to my methods to indicate their behaviour for null values
  • I added sufficient Unit Tests to cover my code.

@github-actions github-actions bot added the 🎈 Feature This Pull Request adds a new feature. label Feb 26, 2021
@github-actions
Copy link
Contributor

Your Pull Request was automatically labelled as: "🎈 Feature"
Thank you for contributing to this project! ❤️

@TheBusyBiscuit TheBusyBiscuit changed the title Allow server owners to fully customize sounds WIP - Allow server owners to fully customize sounds Feb 26, 2021
@TheBusyBot TheBusyBot added the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Mar 9, 2021
feature/sound-manager

Conflicts:
	src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/androids/MinerAndroid.java
	src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/electric/machines/IronGolemAssembler.java
@TheBusyBiscuit TheBusyBiscuit removed the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Mar 13, 2021
@TheBusyBot TheBusyBot added the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Mar 21, 2021
@TheBusyBiscuit TheBusyBiscuit removed the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Mar 21, 2021
@TheBusyBot TheBusyBot added the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Mar 25, 2021
@TheBusyBiscuit TheBusyBiscuit removed the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Mar 31, 2021
@TheBusyBot TheBusyBot added the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Apr 5, 2021
feature/sound-manager

Conflicts:
	CHANGELOG.md
@TheBusyBiscuit TheBusyBiscuit removed the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Apr 8, 2021
@TheBusyBot TheBusyBot added the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Apr 10, 2021
@TheBusyBiscuit TheBusyBiscuit removed the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Apr 10, 2021
@TheBusyBot TheBusyBot added the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Apr 22, 2021
feature/sound-manager

Conflicts:
	src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/multiblocks/AbstractSmeltery.java
@TheBusyBiscuit TheBusyBiscuit removed the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Apr 23, 2021
@TheBusyBot TheBusyBot added the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Apr 23, 2021
@TheBusyBot TheBusyBot removed the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Oct 17, 2022
@TheBusyBot TheBusyBot added the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Jan 22, 2023
@J3fftw1
Copy link
Member

J3fftw1 commented Jan 30, 2023

Can someone pull latest changes?

@WalshyDev WalshyDev removed the ⚡ Merge Conflicts This Pull Request has merged conflicts which need to be resolved! label Jun 12, 2023
@WalshyDev
Copy link
Member

With the addition of #3694 this is now 2/3 done. Merging in both (and the followup with tests) into master independently to avoid one massive PR.

@WalshyDev WalshyDev marked this pull request as ready for review June 21, 2023 17:25
@WalshyDev WalshyDev requested a review from a team as a code owner June 21, 2023 17:25
@WalshyDev WalshyDev changed the title WIP - Allow server owners to fully customize sounds Allow server owners to fully customize sounds Jun 21, 2023
CHANGELOG.md Outdated Show resolved Hide resolved
@J3fftw1
Copy link
Member

J3fftw1 commented Jun 29, 2023

i was gonna spam all comments but nvm
I resolved all requested changes in #3694

@J3fftw1
Copy link
Member

J3fftw1 commented Jun 29, 2023

please pull the latest changes in this branch and then it should be ready to merge

@github-actions
Copy link
Contributor

Slimefun preview build

A Slimefun preview build is available for testing!

https://preview-builds.walshy.dev/download/Slimefun/2844/5411919802

Note: This is not a supported build and is only here for the purposes of testing.
Do not run this on a live server and do not report bugs anywhere but this PR!

@sonarcloud
Copy link

sonarcloud bot commented Jun 29, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 5 Code Smells

65.9% 65.9% Coverage
0.0% 0.0% Duplication

@WalshyDev WalshyDev changed the title Allow server owners to fully customize sounds [CI Skip] Allow server owners to fully customize sounds Jun 29, 2023
@WalshyDev WalshyDev merged commit 811933b into master Jun 29, 2023
11 checks passed
@WalshyDev WalshyDev deleted the feature/sound-manager branch June 29, 2023 20:08
@WalshyDev WalshyDev restored the feature/sound-manager branch June 29, 2023 20:09
@WalshyDev WalshyDev deleted the feature/sound-manager branch June 29, 2023 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎈 Feature This Pull Request adds a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants