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

Fix harpy singing #292

Merged
merged 16 commits into from
Oct 27, 2023
Merged

Conversation

luringens
Copy link
Contributor

@luringens luringens commented Oct 24, 2023

About the PR

Some changes to when my new favourite morphotype is allowed to sing:

  • Muzzled: Stops singing, prevents singing
  • Zombified: Stops singing, prevents singing
  • Stunned: Stops singing
  • Knocked down: Stops singing
  • Sleeping: Stops singing, prevents singing
  • Crit/Dead: Stops singing, prevents singing
  • Glimmer mute event: Stops singing, prevents singing
  • Enough damage to interrupt speech: Stops singing.
  • Any time you can't speak, including mime's vow or mute characters: Prevents singing.

This should fix some of the bugs listed in #180.

Why / Balance

N/A

Technical details

HarpySingerSystem now does two sets of things to enforce these rules. First it subscribes to a lot of events to close the Instrument UI when a muzzle is equipped, they're knocked down, etc. Second, it subscribes to OpenUiActionEvent and explicitly goes before ActivatableUISystem in order to intercepts the MIDI UI opening action. This allows it to run a bunch of checks to see if the harpy should be allowed to sing, and if not, stop it from opening. It also warns the player when this happens.

Media

image
image

  • I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes

N/A

Changelog
🆑

  • fix: Harpies can now sing while cuffed, but not while muzzled, mute, zombified, or otherwise incapable of talking.
  • add: You can now make a harpy stop singing by slipping, stunning, or hurting them.

Stops harpies from opening the MIDI UI while muzzled. Also reworks how
harpies are stopped from singing when incapacitated, so that harpies are
allowed to sing while cuffed. And for UX sake, tells the player why they
can't sing when applicable. This should fix some of DeltaV-Station#180.
@luringens luringens changed the title Fix/harpy singing Fix harpy singing Oct 24, 2023
DebugOk
DebugOk previously approved these changes Oct 24, 2023
Copy link
Contributor

@DebugOk DebugOk left a comment

Choose a reason for hiding this comment

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

Very cool

@DebugOk DebugOk dismissed their stale review October 24, 2023 21:55

Just want to confirm something before this is merged

@luringens
Copy link
Contributor Author

Tested the little refactor and it looks good still, but I should probably give it another round of testing tomorrow when it's not midnight and I'm liable to miss something..

@VMSolidus
Copy link
Contributor

Yea, we'll want to verify that this doesn't reintroduce any new old bugs.
The following conditions need to STOP a singing harpy

  • Stun
  • Gagged
  • Critted
  • Dead
  • Knocked down

The following conditions need to prevent a harpy from starting a new song

  • Stunned
  • Gagged
  • Critted
  • Dead
  • Knocked Down

@luringens
Copy link
Contributor Author

luringens commented Oct 24, 2023

@evilexecutive Thank you for the list! I suspect this PR breaks the "Knocked down" part of it, but it should be easy to add another condition to the system now that the framework for interrupting the UI call is there. I will have a look tomorrow.

@VMSolidus
Copy link
Contributor

@luringens It's more important that Knocked Down stops them from singing, rather than preventing them from starting a new song. It's intended that an annoyed player should be able to shove a harpy to get them to stop.

@luringens luringens marked this pull request as draft October 25, 2023 06:45
@luringens
Copy link
Contributor Author

luringens commented Oct 25, 2023

Alright, putting a checklist of things I need to add checks for and test. Let me know if anything should be added or removed from the list.
Conditions that should interrupt and/or prevent singing:

  • Gagged/muzzled
  • Incapacitated (crit or dead)
  • Zombified
  • Knocked down
  • Asleep
  • Muted trait
  • Psionic glimmer event mute

@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the S: Merge Conflict Fix your PR! label Oct 25, 2023
@github-actions github-actions bot removed the S: Merge Conflict Fix your PR! label Oct 25, 2023
@VMSolidus
Copy link
Contributor

The players apparently want to be able to play spooky zombie music and/or Thriller as zombie birds. I'm inclined to suggest having zombification change their instrument options to something cursed. Thats not a requirement, just a funny suggestion. Personally I lean towards zombie birds not being able to sing

@luringens
Copy link
Contributor Author

luringens commented Oct 25, 2023

@evilexecutive on a similar note, currently if you're typing in text chat and then you take damage from a set of categories and above a set amount, the game immediately appends something like -OOF! to the message and sends it automatically. I could reuse some of that code to stop singing when punched or otherwise taking a solid chunk of brute damage. This could be funny along with a sound effect, though for that we'd need an appropriate bird screech. Or just reuse a scream. Thoughts?

@DebugOk
Copy link
Contributor

DebugOk commented Oct 25, 2023

The players apparently want to be able to play spooky zombie music and/or Thriller as zombie birds. I'm inclined to suggest having zombification change their instrument options to something cursed. Thats not a requirement, just a funny suggestion. Personally I lean towards zombie birds not being able to sing

Zombie harpies shouldn't have the ability to sing anymore

Now stops singing if knocked down, stunned, or asleep. Also prevents
singing if the player for any reason can not talk.
@luringens
Copy link
Contributor Author

Current implementation:

  • Muzzled: Stops singing, prevents singing
  • Zombified: Stops singing, prevents singing
  • Stunned: Stops singing
    • Lasts so short it'll take longer to reopen the window and pick a file than the effect lasts anyways. And for consistency, stun doesn't prevent talking or cause stutter either.
  • Knocked down: Stops singing
    • As above. Also means that harpies that have lost their wheelchair can sing even if they can't stand up.
  • Sleeping: Stops singing, prevents singing (through !CanSpeak)
  • Crit/Dead: Stops singing, prevents singing (through !CanSpeak)
  • !CanSpeak: Prevents singing.

Turns out CanSpeak is a thing. Anytime you can't speak in chat, you now can't sing. This covers crit, dead, asleep, or being mute, simplying the code quite a bit. This should mean you can't sing while glimmer muted, and that mimes can't sing without breaking their wov of silence. Whether or not the latter makes sense I guess depends on how that vow is interpreted.

@VMSolidus
Copy link
Contributor

The latter example of Mime harpies not being able to sing is absolutely correct. Mimes should not he making ANY noise at all, as its part of their act. Being able to sing or imitate sounds would ruin the act, so I will say this counts for intended behavior.

My last question regarding implementation, does the !CanSpeak also stop a song in the case of the mute glimmer event? If it does, I wholeheartedly approve of these changes, you are doing amazingly good work!

@VMSolidus
Copy link
Contributor

Also as an aside, since !CanSpeak applies to Mute characters, yes that is also an intended feature that Mute harpies cannot sing. Which its good that we're covering that base too.

@luringens
Copy link
Contributor Author

luringens commented Oct 25, 2023

I've added a handler that should stop singing when a "Muted" status effect is applied. PsionicCatGotYourTongueRule should trigger this, but I could not figure out how to force a station event to test it.

Do you want harpies to stop singing when taking 10+ brute/burn damage, the same way DamageForceSaySystem does it?

@VMSolidus
Copy link
Contributor

Yea, I will also agree that taking a chunk of damage like that should interrupt them. If it interrupts speech, it should also interrupt singing. Im all for having more options for players to get birds to shut up. This would also add some interesting emergent gameplay options. Such as a harpy salvager singing so that their team knows they're okay, and if something stops them, their team would know they're in trouble.

Design wise, this is their main mechanical positive. They have no other mechanical benefits to weigh against their downsides, so adding mechanical complexity to this feature helps distinguish them more.

@luringens luringens marked this pull request as ready for review October 25, 2023 21:26
@luringens
Copy link
Contributor Author

luringens commented Oct 25, 2023

Alright! Singing is now interrupted by the same criteria that speech is interrupted. I have updated the changelog and PR description, I think this is ready for review again now.

@DebugOk
Copy link
Contributor

DebugOk commented Oct 25, 2023

I've added a handler that should stop singing when a "Muted" status effect is applied. PsionicCatGotYourTongueRule should trigger this, but I could not figure out how to force a station event to test it.

Do you want harpies to stop singing when taking 10+ brute/burn damage, the same way DamageForceSaySystem does it?

addgamerule PsionicCatGotYourTongue, but it's broken anyways

@luringens
Copy link
Contributor Author

Rewrote (force-pushed) those two commits to fix the committer metadata, no change to prior history. Sorry about that.

@DebugOk DebugOk enabled auto-merge (squash) October 27, 2023 18:07
@DebugOk DebugOk merged commit d8f893d into DeltaV-Station:master Oct 27, 2023
10 checks passed
DeltaV-Bot pushed a commit that referenced this pull request Oct 27, 2023
@luringens
Copy link
Contributor Author

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants