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: change tick-time to its intended value (27ms instead of 30ms) #10607

Merged
merged 1 commit into from
Apr 14, 2023

Conversation

2TallTyler
Copy link
Member

@2TallTyler 2TallTyler commented Apr 7, 2023

Motivation / Problem

According to #10205, TTD ran at 27 milliseconds per tick.

OpenTTD always (since day one) has run on 30ms. But this is giving some troubles now we are splitting up timers. Mainly, the idea of 27ms and 74 ticks in a game-day, is that it makes a game-day very close to 2s.

TrueBrain also indicated he never really understood the 74 ticks a day till he realised 27ms was the intended time-per-tick, instead of the picked 30ms.

This most likely was an oversight of the very first version of OpenTTD, which makes this bug the longest unresolved issue as far :)

Additionally, in order for #10605 to support real-time units, we need to change OpenTTD to match. @TrueBrain asked me to split up that PR into smaller chunks, so here's the first one.

Description

27 milliseconds per tick together with a day length of 74 ticks makes one day 1,998 milliseconds, almost exactly 2 seconds. With a 2-second day, one standard month is 1 minute and one standard year is slightly over 12 minutes.

This makes in-game units like "game-seconds" possible, as that is simply 37 ticks. This is far easier to understand for a human over the other units, especially if we continue to decouple those clocks.

Closes #10205.

Limitations

The game runs 13.25% faster. #10605 should partially negate this problem, but it's only available with real-time units.

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')

This makes a month last about 60 seconds, allowing the use of real-time units in game.
@ldpl
Copy link
Contributor

ldpl commented Apr 8, 2023

Have you seen any player ever asking to make the game run faster in tick rate sense? Slower maybe not faster.

The game is fast enough as it is, speeding it up means less time to build things with those lame building tools and even less vehicles you can use in the game before it lags out. Yet instead of fixing those essential issues you think it's a good idea to make the situation even worse? And for what? Why does it even matter for daylength how many ticks there are in a second? All I see is you took a feature that only a few players need, stumbled on an edge case of frozen progress, for it specifically invented some weird realtime units, found out that for the convenience of implementation you need the game to run in whatever tick rate that matches real-time minutes better and decided to change that for everyone. Sorry, but I don't see how any of this is a good idea. There are so many ways to do daylength differently. I run "frozen" server and yet I'd rather have useless year ticking than deal with realtime nonsense.

So, if you want to change tick rate so much please make it configurable. It's a useful setting to have anyway that can save mp events that run into vehicle limit issue. I honestly don't understand how you can at the same time think something of this impact is alright yet be too afraid to change default hotkey in #10006.

P.S I asked ChatGPT to make it sound nicer but it just returned some mumbo jumbo so 🤷 :p

@2TallTyler
Copy link
Member Author

I suppose we could change the tick speed only when in real-time mode, since that already is a new game-only mode. Someone above my pay grade can decide if this is a wise idea or a recipe for disaster.

@nielsmh
Copy link
Contributor

nielsmh commented Apr 8, 2023

The main worry I'd have with making tick speed a variable is whether anything in the game assumes it's a compile time constant. But I think it has been tried before so it probably isn't a problem.

Personally I'd prefer it remains a real constant though, to keep things consistent. In singleplayer you can always pause the game if you need thinking time. In multiplayer, however, it can easily widen the skill gap between experienced and inexperienced players, I agree with that.
Making the tick speed variable might cause confusion in the sense of, "why is that person's game running at a different speed than mine?"
On the other hand, if the speed is made a variable you may even make it a full variable and expose it to multiplayer servers so you can make "fast" and "slow" servers too. With the more recent decoupling of UI updates and game updates that is probably not as much of a problem either?

@ldpl
Copy link
Contributor

ldpl commented Apr 8, 2023

Tick speed is already very much a variable. In single-player you can easily set whatever tick rate just by using fast-forward limiter. Faster, slower, everything works. And afiact only thing stopping it from working in mp is the lack of access to fast forward. If bypassed it seems to work fine as well. Also, even without fast forward having enough vehicles in the game will slow the tick rate.

Though currently I don't see it as a setting to change often. If set low it makes game too jerky and if set high too fast and cpu-intensive. But it would be very useful as an emergency to save a multiplayer game when clients can't keep up, like what happened in Master Hellish 2021 Christmas event. Also having server-controlled fast-forward in mp isn't that bad either if used carefully.

Having high tick rate with some kind of economy slowdown can also be an interesting venue to explore but is very limited by poor game performance unfortunately.

@TheMowgliMan
Copy link

TheMowgliMan commented Apr 8, 2023 via email

@ldpl
Copy link
Contributor

ldpl commented Apr 8, 2023

@TheMowgliMan If you use that MH event as a measure it had to be stopped at about 4k vehicles. Meaning for most players tick took more than 30ms, and vehicles were taking about half of that so that's 4k/15=266 vehicles / ms, shortening the frame by 3 ms means you can have 800 vehicles less in the game which is about 20% in this case. So, you may as well call this PR "Lower the maximum possible number of vehicles in the game by 20%". And performance of the game simulation in general doesn't seem to be improving. Openttdcoop struggled to get 5k vehicles in the game over a decade ago and it's still a challenge now despite all the hardware improvements.

@LordAro
Copy link
Member

LordAro commented Apr 8, 2023

There's a lot of anecdotal evidence and little in the way of actual facts being talked about here. Would be good to stick to measurable facts where possible - there's plenty of large saves around that we should be able to profile (on various systems) to determine the actual impact

And indeed, profile with a profiler to determine possible places for optimisation

@TheMowgliMan
Copy link

TheMowgliMan commented Apr 8, 2023 via email

@ldpl
Copy link
Contributor

ldpl commented Apr 8, 2023

It is an actual fact that MH game ended 4 or 5 hours sooner than planned because of the poor game performance. Ofc, those calculations calculations are approximate and the game was probably one of the worst cases. But no amount of profiling and statistical reasearch on the performance of players computers will make this PR look like it improves performance :P It cuts the frame by 10%, so even in the best case that means 10% less vehicles.

@JGRennison
Copy link
Contributor

Performance costs are not strictly linear. Doubling the number of vehicles may more than double the time required.

@LordAro
Copy link
Member

LordAro commented Apr 8, 2023

But what was the save being run on? (Or one of the clients that was unable to keep up or course, I don't know details of this specific case) Was it something like a Xeon with a low clock speed or otherwise bottlenecked by something else?

You can't just claim that it's poor game performance when there's so many factors at play

@ldpl
Copy link
Contributor

ldpl commented Apr 8, 2023

It ran on something fast enough to drop most of the clients. There are no other factors at play. Some clients that could keep up continued playing but it was bad to have everyone else locked out of the viewers game so it had to be restarted. Changing tick rate to like 70 or 50% could've allowed this game to last till the end of the stream.

@TrueBrain TrueBrain changed the title Change: Make tick length 27 milliseconds Fix: change tick-time to its intended value (27ms instead of 30ms) Apr 14, 2023
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.

Did some actual thinking about this, what the consequences are, and the benefits.

I updated the topic / description to reflect my opinion. Sorry for the hijack @2TallTyler .

Basically, this has been a bug as old as OpenTTD is. Does that mean we should fix it? No. But it makes a lot more things make sense. And that is worth a lot. (referring to the fact that 1 game-day now is two seconds; I now finally understand where 74 ticks come from!)
Additionally, with the upcoming changes to timers and splitting up of calendar from economy, this makes much more sense.

For most users, this will be totally invisible. The only scenario where this might hurt, is where performance was an issue. But here we are basically saying: you can't make any change to the game that has any possible negative performance impact, as that might make a few games unplayable. That is bad reasoning.

Instead, we should approach that specific problem more holistic. And the splitting up of our clocks is one step there. This means users can slow down certain clocks to gain more performance. Additionally, we should look further into both speeding up things like path-finding and making the game more multi-threading friendly. This is where the benefit comes from.

Especially as this PR makes the next steps we want to take easier, we should do it.

@PeterN PeterN merged commit 8e04cba into OpenTTD:master Apr 14, 2023
18 checks passed
@2TallTyler 2TallTyler deleted the 27-ms branch April 14, 2023 13:50
@ldpl
Copy link
Contributor

ldpl commented Apr 14, 2023

Yes, 2-second day it's an absolutely wonderful thing that no one cared about for 20 years of openttd existence.
Also, performance is not the only scenario where it might hurt. In fact, what it probably hurts the most is the ability to compare your game results between versions. Without this change you can still take, for example, over a decade old game mode from luukland or novapolis, set openttd the same way and except for some minor details it would play exactly the same so you can compare your score with those of the past and see for yourself why reaching 100k in citybuilder was such a big deal (https://www.reddit.com/r/openttd/comments/1t32a3/novapolis_community_breaks_100000_population_for/). This changes the game balance ever so slightly that it doesn't really matter for gameplay but all old scores become void. So for communities that care about scores like CityMania there are only 2 options, either erase years of score history or never to update to the new version. Luckily in CityMania case for now there seem to be a third option to just ignore this unnecessary change and patch the server to run at old tps rate but how long will it last until that's "fixed"?
And if it's so unnecessary that it can be changed arbitrarily with patch or just fast-forward why does it even matter for the planned daylenght what exact amount of ticks it has it its "minute"? It doesn't seem to care, for example, that industries produce every 256 ticks making production seem unstable when measured monthly so why should it care about tps? Wouldn't it be more logical to align economic periods with industry production? Besides, isn't the whole idea of a real-time units to be independent of game calendar? Makes no sense for it to be tied to it so closely and I'm yet to see any explanation what exact part of the game requires it. Best guess I've heard so far was "running costs" but those can be easily scaled and applied over any time period.

@JGRennison
Copy link
Contributor

If you're running your own custom server you can presumably just change the tick rate there if necessary and that will propagate to connected clients.

@ldpl
Copy link
Contributor

ldpl commented Apr 14, 2023

Yes, that's the third option I'm talking about, but it's an undocumented feature so can stop working at any time, would be nice to actually have an official support for the tick rate setting.

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.

[Bug]: MILLISECONDS_PER_TICK should be 27, not 30
8 participants