Skip to content

Expose more data for MusicInstrument #12415

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Doc94
Copy link
Contributor

@Doc94 Doc94 commented Apr 12, 2025

This PR take the base of #11925 for expose the data in MusicInstrument for keep the old PR just for create new ones when support exists.

@Doc94 Doc94 requested a review from a team as a code owner April 12, 2025 20:17
@github-project-automation github-project-automation bot moved this to Awaiting review in Paper PR Queue Apr 12, 2025
@Athar42
Copy link

Athar42 commented Apr 12, 2025

Thanks @Doc94 to still looking into this, still waiting for it to get into Paper so I can finally get those custom goat horn working like I want them 🤣

*
* @return a sound
*/
public abstract Sound getSoundEvent();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just call it getSound instead of SoundEvent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm just the method not?
I set this name for avoid confusing for what this get and make consist with the create instrument PR.

Copy link
Member

@Owen1212055 Owen1212055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Otherwise please change the method name of sound.
Consistency with the instrument PR is not needed in this case since those represent the concrete schema types.

@github-project-automation github-project-automation bot moved this from Awaiting review to Awaiting final testing in Paper PR Queue May 1, 2025
@Doc94
Copy link
Contributor Author

Doc94 commented May 1, 2025

LGTM Otherwise please change the method name of sound. Consistency with the instrument PR is not needed in this case since those represent the concrete schema types.

okay this and with the Kenny comment i rename the method

@lynxplay lynxplay moved this from Awaiting final testing to PR Party candidate in Paper PR Queue May 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: PR Party candidate
Development

Successfully merging this pull request may close these issues.

4 participants