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 double post on modded servers #142

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

Conversation

tommywienert
Copy link

This is a potential fix for the issue mentioned in
#141
and
#138
where on a modded server messages that are sent appear twice on the Discord channel. I added a storage variable and a function to check for the doubled message.
To not end up in a loop, the storage Variable will be cleared after it was detected it is a duplicate.

This has one downside though: on a non-modded server if someone posts the same message twice, the second message would not be sent to the Discord channel (the third time will work again, fourth not, ... and so on). But i don't think the impact is too negative. I don't know enough about the message system of 7days to counter that.

It was tested and confirmed working by the User DarqRain on the 7daystodie discord server.

Thank you LakeYS for the easy to read code and the good comments :)

@LakeYS
Copy link
Owner

LakeYS commented Apr 27, 2024

Hello! I appreciate your work and your time writing this PR.

I do have one concern with this for vanilla servers - take a conversation like the following:

Discord User: Is the horde night starting soon?
Player: yes
Discord User: Is our base ready?
Player: yes

However unlikely, I can see there being some scenarios where this would trigger and cause confusion. Users not familiar with this fix would probably have no idea why their message didn't go through. That is if they notice at all, leading to a confusing, maybe even frustrating conversation.

I'd accept these changes behind a config option so that modded servers can turn it on only if they need to. If you'd like, I'm willing to make this change for you before merging. This would happen sometime within the next week or two. Or if you'd prefer, make this change on your end - then all that's required on my end is a quick test & merge. If I push a change to this PR, it will show up here in your fork.

Let me know what you'd like to do.

@tommywienert
Copy link
Author

Hey Lake,
Funny, while working on the ServerTools regex for breeece I was thinking about a flag for modded servers too.
I didn't think of a discord reply, the only (rather unlikely) situation I was thinking of was that one user was with others in voice without microphone which could cause intended double messages, but you are totally right, replying to discord messages is a likely use-case.
I will have a look at that this evening and I guess I will also present my idea how to handle specific mod formats (even though that's sadly not working yet), so you can have a look at that too.

Add regex example for ServerTools mod (WIP).
@tommywienert
Copy link
Author

tommywienert commented Apr 27, 2024

@LakeYS hey there, i have pushed another commit, please take a look and tell me what you think.
I could imagine doing the try catch function with different target regex for different mods

@LakeYS
Copy link
Owner

LakeYS commented May 1, 2024

Hi, thanks for the updates!

Couple notes on the config option:

I think naming it server-is-modded might be too broad. We need to consider servers with different mods and all possible combinations, of which this option may or may not fix their specific issues. I would rather have each fix separated into their own config, unless it's absolutely mod-specific (such as your ServerTools fix) and can be grouped based on the mod that is being accounted for. For example, an option hide-duplicate-messages for the duplicate messages, and hide-servertools-data, or alternatively server-uses-servertools since this is a very specific fix.

A few code nitpicks:

I think the reg variable should be named a bit more specific since it's a script-wide variable and there are other possible cases where we might need to use regex. Just for future-proofing's sake!

The extra console.log's should probably be removed unless they're really necessary for server hosts to see.

For the part handling duplicate messages here, I'd like to see this moved beneath this part where hide-prefix is handled. The sanitizeMsgFromGame function is meant to only handle removing abuse-able strings from messages. Plus this way you can use the return statement (just like hide-prefix does) instead of needing to pass in an empty string to suppress the message. Basically make your duplicate message removal work consistently with hide-prefix.

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

Successfully merging this pull request may close these issues.

2 participants