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

Random Announcer System [PORT] #2652

Open
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

Monotheonist
Copy link
Contributor

@Monotheonist Monotheonist commented Jan 8, 2025

About the PR

Lord Deltanedas is gonna scream at me for this one because FUCK!! IT IS A MESS!!

This PR is for the Random Announcer System that EE has, originally from Parkstation. It is picked from Impstation who has fixed it up, so therefore a good amount of things are labeled as being from that.
To demonstrate this functionality, N.E.I.L. (Neil Breen, the ancient pre-rebase announcer voice) has been added back.
This touches a lot of upstream files and may be hell to maintain. It makes announcements a boolean, and makes using yaml to create announcers + add them to events maybe even a little easier.
I'm planning on adding announcers from Impstation (Sinister, Vox Mesa,) and other ones that eventually may crop up (The obama one that Lyndo keeps shitposting to me about, and Miku / Kasane Teto are some ideas that can be floated) in seperate sequential PRs, for ease of access if anyone would like to revert them.

Why / Balance

I have been a proponent of this for a long time; it adds more flexibility and customization to the server, and selects the voice that all the announcements are in at roundstart. It's cool, and more announcers = more fun, to me. There's little balance reason to it, it's just cool to have.

Technical details

Touches a load of C#, audio, events yaml, if there's an announcement, you name it, it touches it. Like, okay. In order of the tag labels:

  • Adds a shit ton of audio / reorganizes it. Leaves all the normal announcement stuff from upstream relatively intact, to my knowledge.
  • C# for the actual systems for the announcements, and also to inject announcements about various station events into existing events. All that stuff.
  • Locale for the announcement strings.
  • Settings XAML (so, UI code) to change the volume of the announcer in case nobody wants to hear them ;-;
  • YAML for the probabilities and such of what announcers will play.

Media

image
Actual audio files are in the PR and Direction can inspect those. Michael is our current announcer, Neil is not.
But the gist is, player side:

  • creates an announcement at the beginning of the round "welcome to the station crew, enjoy your stay" to show what announcer it selects.
  • Replaces "Station" in "Station Announcement" with the name of said announcer (so, Station Announcement, if Michael is selected, becomes "Michael Announcement.") Name can be tweaked via locstring.
  • The template has loads of commented lines in the yaml that you can uncomment when you get someone to voice the rest of that announcer's lines, otherwise it will fallback onto "Attention."
  • Allows us to tweak what sound is sent when announcements are sent (if we want this to be the old announcements noise, we can change that, and I might do so anyways - currently for Michael it's just him saying "attention.")

Requirements

  • I have tested all added content and changes. (I am testing every couple commits.)
  • I have added media to this PR or it does not require an ingame showcase.

Breaking changes

Review notes for the maintainers that look at this - this shit needs a bunch of comments. Point out where I should put them, please. I mean it because this is a frankly fucking massive PR.

Otherwise, here's the most notable breaking change: this is going to break nearly all upstream files with a string for the announcement being said for that event; there are examples of booleans in the files changed you can replace these with (implying you can even load that.)

Changelog

🆑 Monotheonist, Einstein Engines, Impstation

  • add: Added functionality for multiple announcers. Now, you don't have to hear just one guy over the PA.
  • add: Added Neil Breen back as an announcer.

@github-actions github-actions bot added size/XL Over 1024 lines Changes: YML Changes any yml files Changes: UI Changes: C# Changes any cs files Changes: Localization Changes any ftl files Changes: Audio Changes any audio files labels Jan 8, 2025
@hivehum
Copy link
Contributor

hivehum commented Jan 8, 2025

hello! thanks for porting a feature from imp! it's an honor!

couple notes on our announcers

@github-actions github-actions bot added size/L 256-1023 lines and removed size/XL Over 1024 lines labels Jan 9, 2025
@Monotheonist Monotheonist marked this pull request as ready for review January 14, 2025 17:22
@Monotheonist Monotheonist requested review from a team as code owners January 14, 2025 17:22
@Monotheonist
Copy link
Contributor Author

Monotheonist commented Jan 14, 2025

Reason the PR is open is that it works on my machine, and I'm able to test. It's still kinda broken. Here's some takeaways from playing with it for a little while;

  • Michael (current announcer) and Neil (old announcer) seem to have all their respective lines. At some point when I get to it I'll probably consider a do-over of all the lines that Neil has if I can; I have no idea how the original person who made 'em even got them, and a cursory look seems to reveal it possibly having been AI from a Hugging Face thing that no longer works...? (At least this is my conspiracy, so don't take it as fact?) So if you wanna have a discussion about the ethics of AI voices, be my guest; I'll replace them in this PR with either Sinister or someone else if that is true, which Colin seems to say it isn't.
  • Sometimes it straight up won't select an announcer (Well, it actually does. It's Intern. I nuked him.)
  • Both VoxFem and Medibot don't got much, so I'm gonna remove them from this PR and inject even more impstation (namely Sinister and the other two hivehum mentioned if possible,) and maybe make that Obama TTS announcer Lyndo keeps bothering me about.💀

I think I'll do that in future PRs, and not all at once, so that when you or if you need to revert this you can do it sequentially (announcer by announcer) instead of having to like excise the announcers added here from the repo

@Monotheonist
Copy link
Contributor Author

Other than Build and Test throwing a fit about the xeno vent critters localization pointing to {$location}, and also the lack of sound attribution (yes I do know - it's coming,) this is basically ready for actual review now.

@Monotheonist
Copy link
Contributor Author

Build and Test finally passed on this, yippee! It Works!

Copy link
Contributor

Choose a reason for hiding this comment

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

Add a EoF newline.

Copy link
Contributor

Choose a reason for hiding this comment

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

Add EoF newline.

Copy link
Contributor

Choose a reason for hiding this comment

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

Add EoF newline.

Copy link
Contributor

Choose a reason for hiding this comment

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

Add EoF newline.

Copy link
Contributor

Choose a reason for hiding this comment

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

Two things.
First, the nitpick: Place the added comment after the upstream ones.
Second: Comment what the locale was before turning into a boolean.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same as before, comment the change out, don't delete it. Also state what the locale was before the change.

Copy link
Contributor

Choose a reason for hiding this comment

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

IDs should be on PascalCase.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same as before, IDs should be on PascalCase.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fix this template for the IDs to be on PascalCase please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: Audio Changes any audio files Changes: C# Changes any cs files Changes: Localization Changes any ftl files Changes: UI Changes: YML Changes any yml files S: Needs Review size/L 256-1023 lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants