[patch] Average Station Ratings and Total Waiting Cargo

[PouncingAnt]

I've made a nice (?) little patch to add in two statistics to the bottom of the station list view: Total waiting cargo and Average Cargo Rating.

stationStatsV0.6.diff

Patch file: compiled against r20427

(4.77 KiB) Downloaded 227 times

It should be very handy for players like myself who like providing the ultimate in customer satisfaction!

This is my first patch, and I think it needs a fair amount of testing, but hopefully someone will like it! Are there any willing volunteers?

[PouncingAnt]

OK. Would it be acceptable to post a request for volunteer testers in the general OpenTTD forum? I'm guessing it has better traffic than here, and its not good practice to leave a patch untested.

[Zuu]

Some comments after reading your patch:
* You shall not use tabs in english.txt.
* Do not add commented out code
* You have a copy-paste error in the comment for GetPlayerRatings
* Put a space before '{'. Eg "if (true) {" rather than "if (true){"
* The widget structure will break when users have tall fonts. The resize box will get the same height as the new text area, with the resize image painted at the top of the resize box. Take a look at the town window where the scrollbar and the resize box has been placed in a vertical container so that the resize box will stay small even if a tall font is selected.
* At Line 103 of your patch file you add a line with two tab characters. No trailing spaces at code lines are allowed. Unless the extra empty line is needed for logical grouping, I would not add a new empty line there as generally you shouldn't add empty lines at places where you do not make any changes.

My first though when I saw your patch was that you had made an average over time patch with some kind of sliding mean. However after having read your code I see it is not the case. Instead it is a mean over all cargoes at the current moment.

[PouncingAnt]

Thanks for those pointers, this was very helpful of you! I'll boot back into linux and correct the code in a moment.

Yes, you are correct, it gives the average rating of all cargo at all stations for the present moment in time. Since this number chances very slowly in any case (over a matter of several days it isn't likely to change by more than 2 or 3, especially later in the game), I don't think a sliding mean would really be worth it.

[PouncingAnt]

Just updated the first post with the latest version of the patch, with all the changes you suggested. If anyone picked up an old version in the short time they were there, please update!

[Zuu]

Code: Select all

 *Get total waiting cargo from all stations

Should be

Code: Select all

 * Get total waiting cargo from all stations

Notice the space after the star.

Edit: On line 50 in your version 0.4 you have a missing space after a comma:

Code: Select all

if (st->owner == owner && HasBit(st->goods[j].acceptance_pickup,GoodsEntry::PICKUP)) {

On line 56 in your version 0.4 you have a missing space after the ')' bracket:

Code: Select all

if (num_ratings > 0)cargo_rating /= num_ratings;

Otherwise your patch look good.


Edit2: You might want to consider adding a tool tip to the panel that explains your stats a bit further. You should probably include that the average rating is an average over all cargoes that are being picked up at the station.

[PouncingAnt]

Thanks yet again for your really helpful observations! I'm glad I decided to start out with a small patch!

You might want to consider adding a tool tip to the panel that explains your stats a bit further. You should probably include that the average rating is an average over all cargoes that are being picked up at the station.

I was going to argue on this point.. But I just realised that you're right. If someone has only one cargo selected, then the average doesn't update to reflect that, so I suppose I do need to state that fact explicitly somewhere.

I will look at this all tomorrow morning (I'm on JST now) and hopefully not make more of a mess of things by adding the tooltip

[Yexo]

In both GetPlayerCargo and GetPlayerRatings (bad names btw, at least do s/Player/Company/) you can skip the complete cargo loop in case the station is not owned by the given company. Also use CompanyID as parameter instead of Owner. (and the const is not needed). You do this->SetWidgetDirty(SLW_BOTTOM_TEXT); from OnPaint, so the that widget will be repainted every tick. Repainting that widget means looping over all stations twice, so it might be better to cache the result and only update it every 5 ticks or so. Add a counter variable to the window and use OnTick (or use OnHundredthTick). Also remove the newline between the comments and the start of the function.

That is for coding style. Personally I'm not convinced of the usefulness of these values (apart from "I like statistics").

[PouncingAnt]

Thanks for the tip Yexo, another thing I'll get on with tomorrow.

As for the usefulness of the values, you're right. It is all down to personal playing style. There will be some people who find it useful (or, in the very least me).

I'll try and convince you anyway:
The total waiting cargo can be used to quickly identify (ie just by clicking on the station list window) whether you have a significant of cargo not being transported (important if you're trying to increase your delivery rating to improve your score before your competitors). The av. station ratings gives you an indication of how efficiently you are getting cargo from industries and towns. If the average goes below 50, you can generally bet that you can benefit from optimising rather than expanding.
That, and the fact that "world population" exists in the town directory. Although I'm aware that is pretty mediocre point.

[Arie-]

Well, I guess it could be useful when the scoring is concerned, station rating could be a nice value. Whereas i (personally) do not like minimum profit vehicles two years old, this is a hard one when doing a lot with transfers or when playing with eg. FIRS and distributing engineering supplies.

[Dante123]

a nice add to this would be the option to make the station name get color depend on the rating (like the town name patch)
or, make the station names have a small circle in front of the name that gets the color ?
would be kinda cool

[Zuu]

only update it every 5 ticks or so. Add a counter variable to the window and use OnTick (or use OnHundredthTick).

Every 5 tick or so would still be several times a second. Every hundredth tick would be about every third second, which would still be fairly often for something that only changes from time to time.

[PouncingAnt]

Every 5 tick or so would still be several times a second. Every hundredth tick would be about every third second, which would still be fairly often for something that only changes from time to time.

Actually the waiting cargo changes all the time, as you can probably imagine. Still, I'll give the hundredth tick a go.
That said, issues with slowing the game down are going to be fairly minor. How many people keep the station list window open when fast forwarding?

If memory serves me correctly, I tried doing this->setdirty every hundredth tick, and the graphical glitch still occurred. I'm guessing the same will happen when I try setting just the widget as dirty.

[Yexo]

Indeed, that's why I suggested caching the value. You update the cached value every 100 ticks and when painting you use the cached value. You only set the widget dirty every 100th tick, but if the window is set dirty by any other means you'll still draw the cached value. That way no glitch will occur.

[PouncingAnt]

OK, I just uploaded the new version (first post), hopefully I didn't miss anything. The only thing I'm concerned about is where I put the variables to cache the results from GetCompanyRating and GetCompanyCargo, I couldn't tell from looking at the coding style page whether I got it right.

Anyway, thanks again for everyone for being so generous with their time, it is much appreciated.