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 #8155: Roadtype speed limit in toolbar dropdown in scenario editor was doubled. #8156

Merged
merged 1 commit into from May 19, 2020

Conversation

Yexo
Copy link
Contributor

@Yexo Yexo commented May 17, 2020

No description provided.

@Yexo Yexo changed the title Fix #8155: Roadtype speed limit in toolbar dropdown in scenario edito… Fix #8155: Roadtype speed limit in toolbar dropdown in scenario editor was doubled. May 17, 2020
@LordAro
Copy link
Member

@LordAro LordAro commented May 18, 2020

At the risk of sounding silly, why is / 2 necessary at all? Why is max_speed double the actual value? Perhaps there should be a helper function to get it for you, so it doesn't get forgotten elsewhere?

@Yexo
Copy link
Contributor Author

@Yexo Yexo commented May 19, 2020

At the risk of sounding silly, why is / 2 necessary at all? Why is max_speed double the actual value? Perhaps there should be a helper function to get it for you, so it doesn't get forgotten elsewhere?

This is absolutely a good point. For now I'm going to merge this first, since it is a trivial fix for an existing bug. I'll try to document and/or refactor the code in a separate PR to handle the unclarity of the code (I spend 10 minutes so far, and the reason isn't obvious to me yet).

@Yexo Yexo merged commit 587d8e7 into OpenTTD:master May 19, 2020
8 checks passed
@Yexo Yexo deleted the Yexo-patch-1 branch May 19, 2020
@Yexo Yexo added the backport requested label May 19, 2020
@frosch123
Copy link
Member

@frosch123 frosch123 commented May 19, 2020

It's NewGRF speed units. They different for all vehicle types.

src/aircraft.h: int GetDisplaySpeed() const    { return this->cur_speed; }
src/roadveh.h:  int GetDisplaySpeed() const { return this->gcache.last_speed / 2; }
src/ship.h:     int GetDisplaySpeed() const { return this->cur_speed / 2; }
src/train.h:    int GetDisplaySpeed() const { return this->gcache.last_speed; }

@James103
Copy link
Contributor

@James103 James103 commented May 19, 2020

Quoting from https://wiki.openttd.org/?title=Game_mechanics#Vehicle_speeds:

For trains and aircraft the raw speed is in km-ish/h, for road vehicles/ships in 0.5 km-ish/h. For trains and aircraft this step is done twice a tick, whereas ships and road vehicles do it once a tick.

In other words, the raw speed (the value stored in code) is doubled in code for road vehicles and ships because the physics calculations are only done once per tick on them compared to twice per tick for aircraft and trains. This is why GetDisplaySpeed() divides the speed by 2 before displaying them.

@LordAro LordAro added backported and removed backport requested labels Jul 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants