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

Using emergency security alert now triggers maptext + a sound effect #8323

Closed
wants to merge 3 commits into from

Conversation

ThatFiveGuys
Copy link
Contributor

@ThatFiveGuys ThatFiveGuys commented Apr 23, 2022

About the PR

Pressing the emergency security alert button plays this sound (https://freesound.org/people/stwime/sounds/545360/), and does this maptext:
image

Why's this needed?

The security alert can easily have a massive shift in the combat dynamic, with it easily causing a rush of security officers to arrive to an area with only the touch of a single button. This gives people nearby a warning, so they can choose to retreat or stay put accordingly.

Changelog

(u)Ikea
(*)Pressing the pda security alert now alerts people nearby through the use of a sound effect and text.

@ThatFiveGuys ThatFiveGuys requested a review from a user April 23, 2022 21:19
@boring-cyborg boring-cyborg bot added the C-Sound Automatically applied on any .ogg or sound folder change. label Apr 23, 2022
@github-actions github-actions bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 23, 2022
@ThatFiveGuys ThatFiveGuys added C-Balance Balance changes, buffs and nerfs E-Input-Wanted Input and feedback are wanted. Also posts a discussion thread on the forums. labels Apr 23, 2022
@github-actions
Copy link
Contributor

Created discussion thread: https://forum.ss13.co/showthread.php?tid=18585

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Need to add the sound to the soundcache as per the audio guidelines but aside from that the sound itself is fine. Not commenting on balance or anything though, I dunno how much something like this is needed so I'll let other devs weigh in

@ThatFiveGuys
Copy link
Contributor Author

Done!

@github-actions github-actions bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 24, 2022
Copy link
Member

@frawhst frawhst left a comment

Choose a reason for hiding this comment

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

generally I'm a fan

@pali6
Copy link
Member

pali6 commented Apr 27, 2022

Hm I wonder if it could maybe have a different outline colour to distinguish it from speech more?

@Tarmunora
Copy link
Member

Hm. The sfx doesn't sound terribly alert-like to me.
Also not huge on the maptext but it's not a deal-breaker. I'd rather have it be visual-based instead of text-based if possible,
also as a possibly out-of-scope concern, sec alert module and crisis alert program should probably be unified at some point, b/c as currently implemented, sec can just invoke a sec crisis alert to avoid the notifications from this PR

@NightmareChamillian
Copy link
Contributor

Maybe an overlay, like a flash from a PDA or a glow of sorts would work better than just text? You'd have to figure out how to convey "help", maybe by pairing with text of a different font or layout than speech (like with health scans), but a visual element would aid a lot in meaning

@ThatFiveGuys
Copy link
Contributor Author

ThatFiveGuys commented Apr 28, 2022

I feel like by relying on a visual element like a flash of light, you 1. Dont tell new players what that thing means immediately, unlike text which they can just read. Any knowledge of that indicator will be based on you knowing that indicator instead of something you can quickly figure out. And 2. Would give too much visual noise, distracting people from the sec off to try and figure out what that thing was. Its important to note this maptext is both larger and has different opacity and lack of color compared to speech so while messing around found it pretty distingushable. Its basically the same font cargo price analyzer uses except a bit smaller so if you want to take a look at it just use that. Specific font/color/size is definitely changeable, when i was making this i imagined the maptext to be a robotic voice saying it (i should probably add text in sidebar to accompany it now that i mention it).

@pali6
Copy link
Member

pali6 commented Apr 28, 2022

Maybe it could be like a speech bubble but red or something? idk

@flappybatpal
Copy link
Contributor

The alert sound doesn't give me "emergency alert" vibes.

I'm a little on the fence about the one click security alerts in general at the moment, we swing between stacked sec teams where it can be infamously difficult for antags to do anything hostile and 2-3 officers who get picked off.

@ThatFiveGuys
Copy link
Contributor Author

These alerts get by far the most usage for experienced sec, with newer sec not responding and all that jazz. I think this pr will hurt new players minimally

@github-actions
Copy link
Contributor

This PR has been inactive for two weeks, and has been automatically marked as stale. This means it is at risk of being auto closed in another week. Please address any outstanding review items and ensure your PR is finished. If you are auto-staled anyway, ask developers if your PR will be merged. Once you have done any of the previous actions then you should request a developer remove the stale label on your PR, to reset the stale timer. If you feel no developer will respond in that time, you may wish to close this PR youself, while you seek developer comment, as you will then be able to reopen the PR yourself.

@github-actions github-actions bot added the S-Stale An inactive PR that has had no updates in the past two weeks label May 14, 2022
@flrsh
Copy link
Contributor

flrsh commented May 19, 2022

Echoing other commenters - a more "alert"-like sfx and some modification to the maptext (make it red and a bit more transparent?) would be good. If you're working on other things and would rather close this for now, that's fine too.

@github-actions github-actions bot removed the S-Stale An inactive PR that has had no updates in the past two weeks label May 20, 2022
@github-actions github-actions bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 31, 2022
@ThatFiveGuys
Copy link
Contributor Author

Anyways text should now be a lot more distinct then speech (updated description of pr with a new image of it.)

@github-actions
Copy link
Contributor

This PR has been inactive for two weeks, and has been automatically marked as stale. This means it is at risk of being auto closed in another week. Please address any outstanding review items and ensure your PR is finished. If you are auto-staled anyway, ask developers if your PR will be merged. Once you have done any of the previous actions then you should request a developer remove the stale label on your PR, to reset the stale timer. If you feel no developer will respond in that time, you may wish to close this PR youself, while you seek developer comment, as you will then be able to reopen the PR yourself.

@github-actions github-actions bot added the S-Stale An inactive PR that has had no updates in the past two weeks label Jun 15, 2022
@github-actions github-actions bot closed this Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Balance Balance changes, buffs and nerfs C-Sound Automatically applied on any .ogg or sound folder change. E-Input-Wanted Input and feedback are wanted. Also posts a discussion thread on the forums. S-Stale An inactive PR that has had no updates in the past two weeks size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants