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

Harpy Instrument Additions #184

Merged
merged 4 commits into from
Sep 16, 2024
Merged

Conversation

cynical24
Copy link
Contributor

@cynical24 cynical24 commented Sep 11, 2024

Description

Adds three new harpy instruments: Piano, Church Organ, and Harp.


Changelog

🆑

  • add: three new harpy instruments

@github-actions github-actions bot added Status: Needs Review Someone please review this Changes: C# Changes any cs files Changes: YML Changes any yml files labels Sep 11, 2024
/// When true, only the instrument entity itself can swap its sound.
/// </summary>
[DataField]
public bool OnlySetBySelf = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is true by default, meaning that ALL swappable instruments are affected by this change. You didn't modify the yaml prototypes of other instruments, meaning that all swappable instruments (synthesizers, guitars, etc) are affected by this change and cannot be swapped by anyone unless they get possessed by an admin...

You should make it false by default, and set it to true on MapInit in SingerSystem instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okie dokie

@github-actions github-actions bot removed the Changes: C# Changes any cs files label Sep 11, 2024
@cynical24 cynical24 changed the title Harpy Instrument Additions and Fix Harpy Instrument Additions Sep 11, 2024
@Fansana
Copy link
Owner

Fansana commented Sep 13, 2024

just some general advice, try writing commit messages that are a bit more on topic, later on that might help to narrow down where an important change happend.

@FoxxoTrystan FoxxoTrystan added the Priority: 3-Low Should be resolved at some point label Sep 14, 2024
@Memeji
Copy link
Collaborator

Memeji commented Sep 16, 2024

What's the status on this PR?

Copy link
Contributor

@ShatteredSwords ShatteredSwords left a comment

Choose a reason for hiding this comment

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

All 3 instruments work after playesting

@Fansana Fansana merged commit 16c6e84 into Fansana:master Sep 16, 2024
16 checks passed
Floof-Station-Bot added a commit that referenced this pull request Sep 16, 2024
@cynical24 cynical24 deleted the harpy-piano-add branch September 17, 2024 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: YML Changes any yml files Priority: 3-Low Should be resolved at some point Status: Needs Review Someone please review this
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants