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

Station List: add an option to sort by the number of occurences in vehicles' schedules #6014

Closed
DorpsGek opened this issue May 11, 2014 · 3 comments
Labels
component: interface This is an interface issue flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)

Comments

@DorpsGek
Copy link
Member

xavery opened the ticket and wrote:

When playing OpenTTD, I occasionally have the need to find out which stations are used the most and which are sparingly used. I took the liberty of writing the patch to implement this. It adds the option to sort the station list by the "Vehicle usage rating", which will favour stations which are referenced most by all the vehicles that can stop there, according to the facilities found at a given station.

Attachments

Reported version: trunk
Operating system: All


This issue was imported from FlySpray: https://bugs.openttd.org/task/6014
@DorpsGek
Copy link
Member Author

planetmaker wrote:

The check for all vehicles in all stations is a very expensive check. How does that influence the playability (CPU usage) on a vehicle and order-rich map when the station list is open with this new sorting? You could e.g. try with http://wiki.openttdcoop.org/PublicServer:Archive_-_Games_191_-_200# gameid_200 or some other game (which?) from that list.

Also what's the use case for sorting stations by vehicle count, especially if the counts of different transport types are added? The only use cases we could find are:
* find unserviced stations
* monitor airports for crashed trains by plane count
* monitor RV stations for crashed RV on level crossings

Unserviced stations can be easier / less expensively checked via the stations alone, and abort as soon as any vehicle is found. Finding badly serviced stations is better done via cargo ratings instead of vehicle count.
Plane and RV accidents are news, so players get those already. I can see that being a cheat for single player to build and replace them in the nearest depot. Though that has issues for RV as it might not have a valid connection to the crash site, so maybe only for planes.

As to the patch itself: you're doing already well with the doxygen, but it needs to be even more :). Next to a short description and a description of every parameter, each function with a return value needs an @return doxygen line as well which describes the type and meaning of its return value. Comments in the code itself are also /* ... */ when there's no functional code in the same line(s).


This comment was imported from FlySpray: https://bugs.openttd.org/task/6014#comment13319

@DorpsGek
Copy link
Member Author

xavery wrote:

Hey, and thanks for the comments. You are of course right about this being a very expensive (too expensive, even) check : however, it seems to me that there is no other way to do it with the current codebase unless every station had a variable which would be incremented/decremented on its every add/removal from a vehicle's schedule.
I tested the function with the "Public Server Game 199" savegame. Unfortunately, it crashes in station_map.h at the line IsTileType(t, MP_STATION). This would suggest that the sorter function is called with parameters that are not necessarily stations - I obviously need to dig into the code a bit more.
As for the usecase, I find having such a thing useful when I play a game with lots of large airports, which are very far away from each other (think corners) and are serviced by the fastest airplanes. In this case, it is convenient to have a somewhat equal number of planes servicing these airports. But I'm no expert player, so that feature might not be that useful to anybody else.
I attached the crash log, in case it's useful to anyone.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6014#comment13320

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 1, 2017

andythenorth wrote:

Based on patch review from planetmaker, I think this one is unlikely to have a route forward. Suggest closing it, with thanks.


This comment was imported from FlySpray: https://bugs.openttd.org/task/6014#comment14700

@DorpsGek DorpsGek added component: interface This is an interface issue flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) enhancement labels Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: interface This is an interface issue flyspray This issue is imported from FlySpray (https://bugs.openttd.org/)
Projects
None yet
Development

No branches or pull requests

2 participants