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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Setting to automatically restart server based on hours played #11142

Merged
merged 1 commit into from Jan 26, 2024

Conversation

2TallTyler
Copy link
Member

@2TallTyler 2TallTyler commented Jul 16, 2023

Motivation / Problem

We have a setting for network games to automatically restart upon reaching a certain year. That will never happen if calendar time is frozen.

Description

Add a new setting for a timer that counts down hours played, and resets after that.

Limitations

This was originally a change before @TrueBrain suggested we have both, so blame him for more code to maintain. 馃槈

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR touches english.txt or translations? Check the guidelines
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@TrueBrain
Copy link
Member

I like this PR, but not in the way you might think. Btw, for settings conversion, see #11143. That is, if that gets accepted. I tried to work out there how to do that.

Anyway, for this PR:

I think it is the wrong approach, but accidentally shows a good improvement. Let me explain.

I think we have two types of users:

  1. users that just want to restart the map every NN hours, like this PR does. Like: every 24h it is reset. That sounds like a total awesome things to do.
  2. users that want to play till in-game year 2050, because there the scoreboard shows up. After that, the server restarts (or a few days after, or what-ever).

The first is more for long-term servers like reddit etc. I totally see this PR being of added value to them.
The second is more for competitive play, where people start about on the same time. This PR would make that gametype more difficult.

So ... and I know this is a big ask .. shouldn't we have both settings? With day-length patch, the setting of this PR is going to make much more sense for that first group. But there will always be people in that second group .. and they will have to understand that if you freeze the calendar, the setting will never work. But this new one does. Best of both worlds?

What do you think?

@2TallTyler
Copy link
Member Author

Sure, that makes sense. This becomes a Feature: PR, to add new functionality for when frozen calendar time makes the existing setting useless.

I think I'll also change the time units to hours. Minutes is probably too much precision, given than an hour is only five years in-game. On the other hand, it doesn't really hurt to use minutes, except that users desiring a longer time have to do some math. Anyone feel strongly that it should stay minutes?

@2TallTyler 2TallTyler changed the title Change: Use minutes instead of dates for automatic server restarts Feature: Setting to automatically restart dedicated server based on hours played Jul 27, 2023
@2TallTyler

This comment was marked as outdated.

Copy link
Member

@TrueBrain TrueBrain left a comment

Choose a reason for hiding this comment

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

Overall this looks nice; just a few bits and pieces :)

src/network/network_server.cpp Show resolved Hide resolved
src/network/network_server.cpp Outdated Show resolved Hide resolved
src/settings_type.h Outdated Show resolved Hide resolved
src/network/network_server.cpp Outdated Show resolved Hide resolved
src/network/network_server.cpp Show resolved Hide resolved
@TrueBrain

This comment was marked as resolved.

@TrueBrain
Copy link
Member

Owh, and one more: title says dedicated server, but the code says it works for just server. Do we want to emphasis dedicated here, or just omit it?

@TrueBrain TrueBrain added the candidate: yes This Pull Request is a candidate for being merged label Sep 13, 2023
@2TallTyler 2TallTyler marked this pull request as draft October 4, 2023 18:27
@2TallTyler 2TallTyler changed the title Feature: Setting to automatically restart dedicated server based on hours played Feature: Setting to automatically restart server based on hours played Oct 15, 2023
Copy link
Member Author

@2TallTyler 2TallTyler left a comment

Choose a reason for hiding this comment

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

I don't see any changes to anything autoclean related? What am I missing?

You're missing my lack of reading comprehension. Indeed, this has nothing to do with autoclean. 馃檮

Owh, and one more: title says dedicated server, but the code says it works for just server. Do we want to emphasis dedicated here, or just omit it?

...Hmm, I guess I don't know what difference there is between the two. I don't intend any emphasis, and have only tested on a standard client hosting a game, so I've amended the title.

src/settings_type.h Outdated Show resolved Hide resolved
@2TallTyler 2TallTyler marked this pull request as ready for review October 15, 2023 21:02
@TrueBrain
Copy link
Member

TrueBrain commented Jan 26, 2024

The only thing I wonder about, but I think it is fine:

  • The calendar based restart works even on a loaded savegame.
  • This restart counts from the moment you start the application; not from the start of the game.

An alternative would be to hook it onto the economy timer, and use that for restart. That way it would be a restart after N hours of play.

In reality, I do not think it matters at all, as I suspect only servers use this, where the owner just wants a new game every 23 hours. And they never load in a map, and expect the restart to continue counting.

@2TallTyler
Copy link
Member Author

I agree that servers are probably the only users of this, since it is only accessible from the command line. And once they do restart using this function, the timer starts from 0 as expected and it's a new game, not a savegame.

@2TallTyler 2TallTyler merged commit 2871654 into OpenTTD:master Jan 26, 2024
20 checks passed
@2TallTyler 2TallTyler deleted the network-timer branch January 26, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
candidate: yes This Pull Request is a candidate for being merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants