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

[Feature Request] Toggle suggestions for the RCON backup messages to server #272

Open
3 tasks done
Jadiction opened this issue Jun 28, 2024 · 8 comments
Open
3 tasks done
Assignees
Labels
enhancement New feature or request needs-more-information Not enough information provided to help waiting-for-feedback Waiting on feedback

Comments

@Jadiction
Copy link

Have you read the Important information text above

  • Yes i did

Describe the feature

I am suggesting an environment variable to toggle on/off the RCON backup messages. If not many people are talking, it gets pretty "spammy".
For example, this will be repeated each time:
image
I do want rcon enabled for the player messages (join/leave/name change), but my players don't need to know when a world is saving or being backed up. It's unnecessary information/spam for them to see constantly.

Additional information

  • Would you be willing to help implement this feature?

Final checks

  • I understand that this is a feature request function. I also understand that if i use this to report a bug report, that my issue might be un-answered closed.
@Jadiction Jadiction added the enhancement New feature or request label Jun 28, 2024
@jammsen
Copy link
Owner

jammsen commented Jun 28, 2024

Hey @Jadiction we added that with no toggle for "informing" the players that a lag or a slow-server reaction might be coming in the next 5 seconds. We did that to keep the player informed when stuff happens, when its done and when its over. We would hate it, that someone is in a bossfight and dies because of lags that were not informed and based on backups.

Could you please explain in more detail how a solution for opt-in or opt-out and the idea behind the FR is from your point-of-view please?

Thanks for taking the time and caring!

@jammsen jammsen added waiting-for-feedback Waiting on feedback needs-more-information Not enough information provided to help labels Jun 28, 2024
@Jadiction
Copy link
Author

Jadiction commented Jun 29, 2024

Hey @Jadiction we added that with no toggle for "informing" the players that a lag or a slow-server reaction might be coming in the next 5 seconds.

Yeah, that's a valid reason to have it and I'm sure some people appreciate it.
I would just prefer not to have it and an option to toggle it would be nice. Currently, my backups and saves have arguably unnoticeable server performance. I know this will vary based on how many players, how large the maps are server specs, etc.

I'd prefer to have a cleaner and less spammed chat than to keep informed by a potential small lag spike.

My suggestion is to add an environment variable to make it optional.
if RCON_NOTIFICATIONS is set to false, it for example, won't use the RCON broadcasts from these functions in rcon.sh:

save_and_shutdown_server
broadcast_automatic_restart
broadcast_backup_start
broadcast_backup_success
broadcast_backup_failed

Or if you wanted, you could do 2 variables such as RCON_BACKUP_NOTIFICATIONS and RCON_SAVE_NOTIFICATIONS to further the optimization.

@jammsen
Copy link
Owner

jammsen commented Jun 30, 2024

Hey @Jadiction

After readind your answer and some thinking about it, i would be open to extend it like this:

We already added BACKUP_ENABLED to change if you like backups or not. A valid and reasonable extension on to this would something like BACKUP_NOTIFICATIONS_ENABLED directly after the setting BACKUP_ENABLED to give context to the user/admin and also to help make more informed decisions.

BUT the value would be on/true by default and for backwards-compability. I really dont like thousands of server to break in an instant, just because the have auto-updates enabled and had not chance to read the patch-notes. Thats a no no.

What the server saves before stoping thing affects, i dont want to change that. I a sense to speak of / in a sense you could say, im open to extend the functions of the "backupmanager.sh but im happy with whats in the servermanager.sh i dont want to touch that. Again it comes down to inform the player of events. Server reboot is setup via cron you can do that every 5m or once a month, that what you can already control whats happening and the "spamming" is in the admins control. But messing with this, also does messing with automation for server-management, im not really happy to change that sorry.

Again im open to hear more feedback, but thats my current offer to extend this.

@jammsen
Copy link
Owner

jammsen commented Jul 14, 2024

@Jadiction ping

@jammsen jammsen self-assigned this Jul 14, 2024
@Jadiction
Copy link
Author

A valid and reasonable extension on to this would something like BACKUP_NOTIFICATIONS_ENABLED directly after the setting BACKUP_ENABLED to give context to the user/admin and also to help make more informed decisions.

This would work, but I was also looking for this to include the save message notifications, unless you're thinking about including those for this variable.

BUT the value would be on/true by default and for backwards-compability.

I agree, having it inform players is a good default and let it be an option for server admins on where they want it to or not.

But messing with this, also does messing with automation for server-management, im not really happy to change that sorry.

I'm confused on what you mean by this. Adding an if statement for a new variable (defaulted to how it is now) wouldn't affect the automation process other than allowing customization to the notifications.

@jammsen
Copy link
Owner

jammsen commented Jul 27, 2024

A valid and reasonable extension on to this would something like BACKUP_NOTIFICATIONS_ENABLED directly after the setting BACKUP_ENABLED to give context to the user/admin and also to help make more informed decisions.

This would work, but I was also looking for this to include the save message notifications, unless you're thinking about including those for this variable.

That would be covered if i understood you correctly.

I'm confused on what you mean by this. Adding an if statement for a new variable (defaulted to how it is now) wouldn't affect the automation process other than allowing customization to the notifications.

What i meant was regarding to the first request you wrote "i want to reduce spamming regarding backups" im open to that, thats in a single file called https://github.com/jammsen/docker-palworld-dedicated-server/blob/develop/scripts/backupmanager.sh

BUT the other rcon-messages are in the server-manager-part ( https://github.com/jammsen/docker-palworld-dedicated-server/blob/develop/scripts/servermanager.sh ), meaning join/leave/start/stop and i dont consider them spamming.

@Jadiction
Copy link
Author

BUT the other rcon-messages are in the server-manager-part ( https://github.com/jammsen/docker-palworld-dedicated-server/blob/develop/scripts/servermanager.sh ), meaning join/leave/start/stop and i dont consider them spamming.

Oh yeah, I agree. It's just the backup messages that I consider "spammy." I imagine most people have it backup relatively short, especially with a retention option, it makes sense to have recent backups in the case of world corruption (which has been an issue since beta release). The other notifications are helpful.

@jammsen
Copy link
Owner

jammsen commented Aug 12, 2024

Hey @Jadiction i finally got around to work on this, sorry it took so long.

Its no on develop for testing, feel free to load this image tag to try it out jammsen/palworld-dedicated-server:develop

Changes and explainers are here: c1704b3

I decided to announce "ERRORS ONLY" still to the ingame chat, no matter what. I think that is one thing we didnt talk about, but it still counts as important to get an important info that you need to research whats happening.

Please let me know if that works for you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-more-information Not enough information provided to help waiting-for-feedback Waiting on feedback
Projects
None yet
Development

No branches or pull requests

2 participants