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

Last Message Before Death Webhook. #30235

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

Conversation

VelonacepsCalyxEggs
Copy link
Contributor

@VelonacepsCalyxEggs VelonacepsCalyxEggs commented Jul 21, 2024

About the PR

This PR introduces an optional webhook feature that sends the last in-character (IC) messages of dead players before their death at the end of the round.

Why / Balance

This feature is added mainly for fun, I have taken inspiration from a simillar webhook on a SS13 server.
It doesn’t have a specific reason, but could add an amusing element to the game by capturing players’ final words.

Technical details

This is my first PR involving C#. So, if I did anything wrong, I will be glad to learn and improve.

The implementation works as follows:

  • When TrySendInGameICMessage is called in ChatSystem.cs, it checks if the player sent the message.
  • The message is then added or updated in the dictionary located in LastMessageBeforeDeath.cs.
  • At the end of the round, onRoundEnd is called from GameTicker.RoundFlow to send the message using a webhook.

This feature is only active if the webhook link is specified in server_config.toml.

Media

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

изображение
I know it says After Death on the image... I am dumb.

Changelog

🆑 func_kenobi

  • add: Added an optional Webhook that will send messages said by players before their death at the end of the round.

Signed-off-by: VelonacepsCalyxEggs <ivankuvaev777@gmail.com>
Signed-off-by: VelonacepsCalyxEggs <ivankuvaev777@gmail.com>
@github-actions github-actions bot added the S: Needs Review Status: Requires additional reviews before being fully accepted label Jul 21, 2024
@Emisse
Copy link
Contributor

Emisse commented Jul 21, 2024

very fun

@Emisse Emisse requested a review from Chief-Engineer July 21, 2024 18:45
@VelonacepsCalyxEggs VelonacepsCalyxEggs changed the title Last Message After Death Webhook. Last Message Before Death Webhook. Jul 22, 2024
- Added conditional compilation for Debugging.
- Webhook is no longer exposed to VV.
- Now using an ordered dict.
- Message is now formatted.
- Time is now trunucated.
- Now checking for maximum size and splitting the message (2000).
- Prevention of a ratelimit if more than 30 messages are sent.
@Chief-Engineer Chief-Engineer dismissed their stale review July 22, 2024 13:38

addressed

VelonacepsCalyxEggs and others added 4 commits July 22, 2024 18:15
Remove the leftover [ViewVariables] (I am blind).

Co-authored-by: Chief-Engineer <119664036+Chief-Engineer@users.noreply.github.com>
- All hardcoded variables are now constants. (well not all, all that needed to be)
- If the message exceeds the MaxICLength, it will be cut off at a random interval.
Example: Someone was giving a speech and got blown to bits.
IC: "I am giving a very interesting 1000
character long spee-"
- Made sure the messages don't get split in half.
- Added a delay between messages.
@VelonacepsCalyxEggs
Copy link
Contributor Author

I have encountered a problem, IGameTiming.CurTime seems to be the time of how long the server has been up, and not the round time.
Where can I get the round time from instead?

@EmoGarbage404
Copy link
Member

I have encountered a problem, IGameTiming.CurTime seems to be the time of how long the server has been up, and not the round time. Where can I get the round time from instead?

GameTicker has it

VelonacepsCalyxEggs and others added 2 commits July 22, 2024 18:43
Signed-off-by: VelonacepsCalyxEggs <ivankuvaev777@gmail.com>
fixed a place where a constant was not used.
@VelonacepsCalyxEggs
Copy link
Contributor Author

I have only one concern as of now.

What if a player says something inapropriate, and gets kicked / banned from the game? Player's message will still stay in the pool.
I not sure if I should be worried about this or not.
But if I need to implement this, I would probably go about it like this:
Remove the EntityUid of the player from the dict when he gets kicked/banned.

But I didn't really look at any admin code, so... not sure how I would go about implementing that without touching some sacred code.

@Chief-Engineer
Copy link
Contributor

When someone is banned or kicked you could just call a method that finds and removes their message from the message list

@VelonacepsCalyxEggs
Copy link
Contributor Author

When someone is banned or kicked you could just call a method that finds and removes their message from the message list

Oh, so I suppose there is an event listener for that, that also returns the player's uid when someone is kicked/banned?
I will look into it.

@Chief-Engineer
Copy link
Contributor

I'm not sure if there's an existing event for those or not, but it should be fairly easy to make an event if it's needed

@VelonacepsCalyxEggs
Copy link
Contributor Author

VelonacepsCalyxEggs commented Jul 28, 2024

I'm not sure if there's an existing event for those or not, but it should be fairly easy to make an event if it's needed

So... uhh... I've asked around and looked around, and I don't think there are event listeners for that.
Should I open a separate PR for this or... just jam it in here?

@Chief-Engineer
Copy link
Contributor

You can just add them with this if they'll be used by this PR

I had to add an event listener for bans, it seems to work.
But I am not sure I did it properly.

Also some minor fixes, like whisper and emote not registering properly.


Signed-off-by: VelonacepsCalyxEggs <ivankuvaev777@gmail.com>
@PJB3005 PJB3005 self-assigned this Oct 30, 2024
Copy link
Member

@PJB3005 PJB3005 left a comment

Choose a reason for hiding this comment

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

Haven't fully reviewed this but well here's enough to keep you busy.

private static LastMessageBeforeDeathSystem? _instance;
private static readonly object _lock = new object();

private static readonly Random _random = new Random();
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 still not fixed.

@github-actions github-actions bot added S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted labels Nov 13, 2024
Copy link
Contributor

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

1 similar comment
Copy link
Contributor

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

@SlamBamActionman SlamBamActionman added the S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. label Nov 14, 2024
@PJB3005 PJB3005 added T: New Feature Type: New feature or content, or extending existing content S: Awaiting Changes Status: Changes are required before another review can happen A: Chat Area: Chat-related features and changes, most likely technical size/L Denotes a PR that changes 1000-4999 lines. and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. S: Needs Review Status: Requires additional reviews before being fully accepted labels Nov 17, 2024
Copy link
Contributor

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

@VelonacepsCalyxEggs
Copy link
Contributor Author

I'll get back to this soon. Just in case.

# Conflicts:
#	Content.Server/Administration/Managers/IBanManager.cs
#	Content.Shared/CCVar/CCVars.cs

Added a webhook related CCVar in Content.Shared/CCVar/CCVars.Discord.cs
@github-actions github-actions bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Nov 25, 2024
VelonacepsCalyxEggs and others added 2 commits December 15, 2024 23:44
Now using mind to get the Character Name to avoid obfuscation.
Added all constants as CVars.
Webhook CVar is now tracked fully in the LastMessage system.
Removed unneeded junk.
@VelonacepsCalyxEggs
Copy link
Contributor Author

I think I did everything specified in the review, but I am not sure about some things, specifically not storing ICommonSession, I need to store it to check whenever the player was banned or kicked during their gameplay, which would disqualify them from getting into the webhook, but, that also could mean that if a player looses connection and reconnects, they would be disqualified too, I need a way to check directly by NetUserId, because it is persistent, I was thinking of checking if there is a mind linked to this NetUserId, but I am not sure what happens to a player's mind when they disconnect.

I need some advice on this.

@github-actions github-actions bot added S: Needs Review Status: Requires additional reviews before being fully accepted and removed S: Awaiting Changes Status: Changes are required before another review can happen labels Dec 16, 2024
@github-actions github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Jan 15, 2025
Copy link
Contributor

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

@github-actions github-actions bot added size/M Denotes a PR that changes 100-999 lines. and removed S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted size/L Denotes a PR that changes 1000-4999 lines. labels Jan 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: Chat Area: Chat-related features and changes, most likely technical Merge Conflict S: Needs Review Status: Requires additional reviews before being fully accepted 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
Development

Successfully merging this pull request may close these issues.

7 participants