Skip to content

Exception now meows when an error occurs on the server #39734

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Exception now meows when an error occurs on the server #39734

wants to merge 19 commits into from
+169 −2

Conversation

beck-thompson
Copy link
Member

@beck-thompson beck-thompson commented Aug 18, 2025 •
edited
Loading

About the PR

Title! There are a few things to note:
1.) There is a 2-3 minute cool down (Depending on the animal).
2.) They will say a fake error message ~0.7-1% of the time without an accent (Again depending on the animal).

Why / Balance

Its a cute feature that can give some life into otherwise pretty boring cats (Besides when they out II because of that glitch 😆)!

Technical details

To test, add Spawn("randomgibberish") to the gun system when you shoot and run in Tools mode so it doesn't crash.

image

Media

2025-08-18.00-55-11.mp4

Requirements

Breaking changes

The TransformSpeechEvent can now be canceled

Changelog

🆑

  • add: Both Exception and Runtime now meow when an error occurs on the server.

@PJBot PJBot added S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. size/M Denotes a PR that changes 100-999 lines. labels Aug 18, 2025
@beck-thompson beck-thompson added P3: Standard Priority: Default priority for repository items. T: New Feature Type: New feature or content, or extending existing content D2: Medium Difficulty: A good amount of codebase knowledge required. A: Chat Area: Chat-related features and changes, most likely technical A: General Interactions Area: General in-game interactions that don't relate to another area. and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Aug 18, 2025
@lzk228
Copy link
Contributor

lzk228 commented Aug 18, 2025

it's hilarious

@VerinSenpai
Copy link
Contributor

good thinking on that cooldown. The oh so common error spam causing the cat to never shut the hell up lmao

@iaada
Copy link
Member

iaada commented Aug 18, 2025

Do you think it might be good to add this to real mouse? Getting the occasional error message while in dev feels appropriate. (It owns)

@TeenSarlacc
Copy link
Contributor

finally, fourth wall breaks

@beck-thompson
Copy link
Member Author

Do you think it might be good to add this to real mouse? Getting the occasional error message while in dev feels appropriate. (It owns)

Yeah it sounds fun! Probably wont come up too often because most errors crash in debug but sometimes I run on tools so 🤷

@slarticodefast
Copy link
Member

Some error messages contain PII if I remember correctly. So are we legally allowed to show them in-game?
Also how long are the error messages? I assume it doesn't log the full stack trace.

@beck-thompson
Copy link
Member Author

Some error messages contain PII if I remember correctly. So are we legally allowed to show them in-game?
Also how long are the error messages? I assume it doesn't log the full stack trace.

The most it would show would be (I guess) its username / maybe admin notes but I'm not quite sure how exactly those could happen. Usernames are already shown in OOC chat so it can't be too big of a deal? It does seem really sketchy to do it raw but the fun factor cannot be denied... I'm OK with either

image

(This is what it looks like)

@slarticodefast
Copy link
Member

Ok, that seems pretty harmless

@Luxzhv
Copy link

Luxzhv commented Aug 18, 2025

Throwing in a ideaguy thing: Have this interact with Cogni some how, like errorprint ability or something, rather then just confusing the controlling player that your randomly meowing.

Love this PR though, very cute. We need something for Runtime.

@ArtisticRoomba
Copy link
Member

can you also add the ability to throw and catch exception as well

VasilisThePikachu
VasilisThePikachu reviewed Aug 18, 2025
View reviewed changes
Copy link
Member

@VasilisThePikachu VasilisThePikachu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I believe censoring should be enabled by default outside of dev environments. (PII is possible to appear outside of the stack trace in the message. I am not taking the risk.)

Also the SpeakOnException component should be removed when the entity becomes cogni'd or otherwise controlled by a player. Especially since it could confuse an admin that the controlling player said that. (And in general be annoying if you are just playing and you suddenly scream "UNABLE TO SPAWN PROTOTYPE")

VasilisThePikachu
VasilisThePikachu requested changes Aug 18, 2025
View reviewed changes
Copy link
Member

@VasilisThePikachu VasilisThePikachu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Above should be required IMO

Also, this cat WILL meow a lot, there are A LOT of errors on live servers currently.... I think a cvar to disable this function as a whole will be good for downstreams that may not like it.

Also dont log the speach in chat

I just checked lizard and there are about 35 errors in the past 15 minutes as of writing. This cat will meow a lot and it will eventually become annoying.

@PJBot PJBot added S: Awaiting Changes Status: Changes are required before another review can happen and removed S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. labels Aug 18, 2025
@iaada
Copy link
Member

iaada commented Aug 18, 2025

If you don't like the meows, contribute to fixing the errors :godo:

I don't think a cvar is necessary. If downstreams don't like it they can comment out the component.

@VasilisThePikachu
Copy link
Member

If you don't like the meows, contribute to fixing the errors :godo:

I don't think a cvar is necessary. If downstreams don't like it they can comment out the component.

A downstream should not have to do this, it will just cause annoying conflicts down the line for them

Princess-Cheeseballs
Princess-Cheeseballs requested changes Oct 18, 2025
View reviewed changes
@PJBot PJBot added S: Awaiting Changes Status: Changes are required before another review can happen and removed S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. labels Oct 18, 2025
Simyon264
Simyon264 requested changes Oct 18, 2025
View reviewed changes
@PJBot PJBot added S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. and removed S: Awaiting Changes Status: Changes are required before another review can happen labels Oct 18, 2025
@github-actions github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Oct 19, 2025
@github-actions 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 removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Oct 19, 2025
@beck-thompson beck-thompson removed the S: Concern A maintainer has raised concerns regarding the idea. The PR may be closed by another maintainer. label Oct 19, 2025
Simyon264
Simyon264 requested changes Oct 21, 2025
View reviewed changes
Copy link
Member

@Simyon264 Simyon264 left a comment

Choose a reason for hiding this comment

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

looks peak than this

@PJBot PJBot added S: Awaiting Changes Status: Changes are required before another review can happen and removed S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. labels Oct 21, 2025
@PJBot PJBot added S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. and removed S: Awaiting Changes Status: Changes are required before another review can happen labels Oct 21, 2025
PJB3005
PJB3005 requested changes Oct 21, 2025
View reviewed changes
Content.Server/RuntimeFun/SpeakOnExceptionComponent.cs
/// The next time the entity can say another error.
/// </summary>
[DataField]
public TimeSpan? NextTimeCanSpeak;
Copy link
Member

Choose a reason for hiding this comment

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

This needs to use TimeOffsetSerializer.

Copy link
Member Author

Choose a reason for hiding this comment

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

I also added the auto pause fields which I think are also needed - let me know if they aren't (The documentation kina makes them sound like they should be added?)

Content.Server/RuntimeFun/SpeakOnExceptionSystem.cs
{
if (_random.Prob(ent.Comp.ChanceSpeakNoAccent))
args.Cancel();
}
Copy link
Member

Choose a reason for hiding this comment

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

This will apply to all speech, not just error messages, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep I think it will! Changed it due to #39734 (comment) - but apparently I did that for a reason but I got myself confused and got rid of it 😆

Copy link
Member

Choose a reason for hiding this comment

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

This is not mentioned in the PR description and I do not think this is a good feature to have.

Copy link
Member Author

Choose a reason for hiding this comment

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

It now works proplerly - I personally think its fun.

@PJBot PJBot added S: Awaiting Changes Status: Changes are required before another review can happen and removed S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. labels Oct 21, 2025
@beck-thompson beck-thompson requested a review from PJB3005 October 22, 2025 02:30
@PJBot PJBot added S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. and removed S: Awaiting Changes Status: Changes are required before another review can happen labels Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ScarKy0 ScarKy0 ScarKy0 left review comments

@Princess-Cheeseballs Princess-Cheeseballs Princess-Cheeseballs requested changes

@metalgearsloth metalgearsloth Awaiting requested review from metalgearsloth

@VasilisThePikachu VasilisThePikachu Awaiting requested review from VasilisThePikachu

@sowelipililimute sowelipililimute Awaiting requested review from sowelipililimute sowelipililimute is a code owner

@Simyon264 Simyon264 Awaiting requested review from Simyon264

@PJB3005 PJB3005 Awaiting requested review from PJB3005

Requested changes must be addressed to merge this pull request.

Assignees

@SlamBamActionman SlamBamActionman

Labels

A: Chat Area: Chat-related features and changes, most likely technical A: General Interactions Area: General in-game interactions that don't relate to another area. D2: Medium Difficulty: A good amount of codebase knowledge required. P3: Standard Priority: Default priority for repository items. S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. S: Conceptual Approval Status: Discussed by maintainers and has conceptual approval, but needs code review S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. S: Undergoing Discussion Status: Currently going through an extended discussion, as per procedure. size/M Denotes a PR that changes 100-999 lines. T: New Feature Type: New feature or content, or extending existing content

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.