Patch: Rating in Town Label [v13-r18684]

Forum for technical discussions regarding development. If you have a general suggestion, problem or comment, please use one of the other forums.

Moderator: OpenTTD Developers

User avatar
SirXavius
Transport Coordinator
Transport Coordinator
Posts: 302
Joined: 28 Jun 2006 18:25
Location: Florida

Re: Patch: Town sign color shows ratings[v0.62]

Post by SirXavius »

Thanks for that up-to-date binary. With that, i'll be glad to test it (something tells me i'm gonna love this). :wink:

Terkhen
OpenTTD Developer
OpenTTD Developer
Posts: 1034
Joined: 11 Sep 2008 07:32
Location: Spain

Re: Patch: Town sign color shows ratings[v0.63]

Post by Terkhen »

Another update. I implemented all of Hirundo's suggestions and updated the code to trunk.

v0.63: Update to current trunk. Coding style corrected, including trailing whitespaces (I bet I'm missing something :D). Statements to check current rating changed to a better solution that is already used in other parts of OpenTTD code. TownRatingColour implemented as a member of the Town class. Unified some duplicated code.

CommanderZ: I intend to do that, but only when I'm completely sure that I'm not missing something. I want to get this as good as I can :)

Hirundo: I followed your suggestions and inspected closely every line of the diff, and I think I got every coding style error. The first versions of the patch sticked to the colours in that enum, but I had to either include colours that broke the intuitive red-yellow-green progression (I tried blue, gold, silver...) or to use fewer colours and let half of the ratings sharing colours. Finally I decided to include more colours: I'm pretty sure that the results look good now, but I'm still not happy with the way I implemented it. Thank you for checking my patch!

SirXavius: Thank you, I hope you like it.

Here is an updated binary for windows. Since the behaviour of the patch hasn't been altered, the only "visible" change is the version bump.

Edit: Deleted, see the first post for an updated binary.
Last edited by Terkhen on 24 Jun 2009 14:33, edited 1 time in total.

Hirundo
Transport Coordinator
Transport Coordinator
Posts: 298
Joined: 27 Jan 2008 13:02

Re: Patch: Town sign color shows ratings[v0.63]

Post by Hirundo »

Since you seem to have little success in eradicating the whitespace (no offence intended at all, I know from experience how hard it can be) I shall present you the Whitespace Detector (r). It is butt-ugly and coded in around five minutes. e.g. there isn't even a check to see whether the file you try to open actually exists. However, for me it does the job of detecting trailing spaces / tabs nicely. Feel free to do or do not use it. Source code (Actionscript) is available upon request.
Attachments
whitespace.zip
(25.08 KiB) Downloaded 142 times
Create your own NewGRF? Check out this tutorial!

Terkhen
OpenTTD Developer
OpenTTD Developer
Posts: 1034
Joined: 11 Sep 2008 07:32
Location: Spain

Re: Patch: Town sign color shows ratings[v0.63]

Post by Terkhen »

None taken, I'm glad that you are helping me. I thought that Notepad++'s trim trailing whitespaces command would do the job, but it seems like it didn't.

The whitespace detector wasn't able to detect any trailing whitespaces, even in files that I know for sure that had them (like the release of the patch for stable 0.7, which I removed until I have time to prepare a new one for 0.7.1). I think it may be because I'm in windows right now: I'll give it another try later in ubuntu.

Edit: It didn't worked in ubuntu. Perhaps I'm doing something wrong?. I open the whitespace remover with firefox (tried with explorer too), put the route of the file at the <file> field and press load. When I put a wrong route on purpose, there's no output. When I put the correct route, I get a "no whitespaces found" message.

Hirundo
Transport Coordinator
Transport Coordinator
Posts: 298
Joined: 27 Jan 2008 13:02

Re: Patch: Town sign color shows ratings[v0.63]

Post by Hirundo »

Did you enter the correct line-ending in the top right box? For anything vaguely related to unix, including your patch file, %0A (=\n) is usually a better choice than windows's %0D%0A (=\r\n)
Create your own NewGRF? Check out this tutorial!

Terkhen
OpenTTD Developer
OpenTTD Developer
Posts: 1034
Joined: 11 Sep 2008 07:32
Location: Spain

Re: Patch: Town sign color shows ratings[v0.63]

Post by Terkhen »

That was my mistake: the whitespace remover worked perfectly. I uploaded a version free of trailing whitespaces in the first post. Thank you for your patience and help again, Hirundo.

xZise
Engineer
Engineer
Posts: 57
Joined: 19 Mar 2009 16:52
Location: Bad Schwartau

Re: Patch: Town sign color shows ratings[v0.63]

Post by xZise »

Hi,

this is a great patch, but now I couldn't hide the population of a town. Is it possible to add an option "Show town names colored to the rating" which also dye the name (whether the population appears)?

Sincerely
xZise

Terkhen
OpenTTD Developer
OpenTTD Developer
Posts: 1034
Joined: 11 Sep 2008 07:32
Location: Spain

Re: Patch: Town sign color shows ratings[v0.63]

Post by Terkhen »

The original behaviour of the patch was to show both strings coloured (hence the outdated name of the patch that I'll correct some day). It added a new setting (show ratings in town sign or something like that) instead of modifying the already existent setting for population. This behaviour was abandoned in favor of the present one because of four motives.

- I think that there's already too much advanced settings, and showing population and ratings can be joined in the same option because they do mostly the same (show extra info about the town).

- Since station signs are coloured too, the coloured town name generated confusion. More important: if transparency is set, is impossible to tell apart the town name and stations with the same color than the town rating.

- The colours made the town name a bit more unreadable.

- The code is more undestandable now, and easier to mantain.

User avatar
SirXavius
Transport Coordinator
Transport Coordinator
Posts: 302
Joined: 28 Jun 2006 18:25
Location: Florida

Re: Patch: Town sign color shows ratings[v0.63]

Post by SirXavius »

I love this patch! I can immediately see my standing with each town, esp. when i'm bulldozing their trees!! :twisted:

Now let us pray to the gods of Trunk Inclusion that they may find favor with your sacrifice....

(Anyone bring the incense? And where's the virgin??!) :P

Terkhen
OpenTTD Developer
OpenTTD Developer
Posts: 1034
Joined: 11 Sep 2008 07:32
Location: Spain

Re: Patch: Town sign color shows ratings[v0.63]

Post by Terkhen »

The patch has not been submitted to flyspray for dev revision, as I still need to reimplement some quick hacks I did and add comments to my code to make it cleaner. When I'm sure that the patch is clean and correctly implemented, it will be submitted. Then we will know if this patch can be added to trunk or not.

Terkhen
OpenTTD Developer
OpenTTD Developer
Posts: 1034
Joined: 11 Sep 2008 07:32
Location: Spain

Re: Patch: Town sign color shows ratings[v0.64]

Post by Terkhen »

Another new version has arrived. Check the first post for the diff file and a win32 build.

v0.64: Update to current trunk. Critical parts of the patch have been remade to be both faster and easier to read.

Right now what I had in mind for this patch is almost finished, and there's a single problem to be solved (unless we find another one, of course). As you may know, right now the colour scheme uses colours that are not part of the TextColour enum. Without using custom colours, the colour scheme would be reduced to something like Red, Orange, Yellow, White and Green; losing information for some rating levels and it would look worse. But the changes to the code needed to show these extra colors are kind of ugly and some hacks are needed. There's two options: I could take the new colours out and submit it to flyspray in a cleaner state, or try to undo the hacks and implement my changes in a well designed way (which would require to rewrite string functions that are used a lot through all of OpenTTD's code). Personally I prefer the simpler approach (patches should stay as simple as possible) but I want to check your feedback before taking it. Of course if I end making a version with a reduced color scheme, the old patch would stay somewhere for everybody who prefers it.

Edit: This is the patch for v0.64. Note that this is the last patch with a different colour for each rating
Attachments
rating_town_label_v0.64_r16727.diff
(10.41 KiB) Downloaded 146 times

Terkhen
OpenTTD Developer
OpenTTD Developer
Posts: 1034
Joined: 11 Sep 2008 07:32
Location: Spain

Re: Patch: Rating in Town Label[v0.7]

Post by Terkhen »

Another new version. The patch was renamed because the last name wasn't very correct. As always, diff and binary in the first post.

v0.7: Patch renamed, and almost completely remade. The patch uses only standard colours, and because of that now the code is way more simple. Added comments and fixed a situation in which the correct ratings weren't shown.

As you can see, this version reduces the number of colours to make the patch more simple. I don't think that anybody will miss the last version, since there was no feedback about the planned changes and I posted different files by mistake and it seems that nobody noticed. Anyways the old version can be found in my previous post. I'd like to ask for a final review of the code, because this is the version I'm going to submit to flyspray.

Terkhen
OpenTTD Developer
OpenTTD Developer
Posts: 1034
Joined: 11 Sep 2008 07:32
Location: Spain

Re: Patch: Rating in Town Label[v0.71]

Post by Terkhen »

Updated patch and binary at the first post. No new features.

v0.71: Update to current trunk. Fixed an incorrect string.

To avoid quintuple posting, I will not announce future updates to this patch.

xZise
Engineer
Engineer
Posts: 57
Joined: 19 Mar 2009 16:52
Location: Bad Schwartau

Re: Patch: Rating in Town Label[v0.71]

Post by xZise »

Hi,
the germans textstring for this Patch:

Code: Select all

STR_CONFIG_SETTING_POPULATION_IN_LABEL                          :{LTBLUE}Zeige die die Haltung von Städten in ihrem Namensschild: {ORANGE}{STRING}
Sincerely
xZise

Terkhen
OpenTTD Developer
OpenTTD Developer
Posts: 1034
Joined: 11 Sep 2008 07:32
Location: Spain

Re: Patch: Rating in Town Label[v0.71]

Post by Terkhen »

Thank you, xSize. I am not including translations in my patches (besides english) to keep them as clean and trunk-friendly as I can, but german-speaking testers of this patch could find it useful if it was made as an additional diff file.

User avatar
Vaulter
Traffic Manager
Traffic Manager
Posts: 185
Joined: 21 Dec 2004 05:35
Skype: andrey-zaharov
Location: St. Petersburg, Russia
Contact:

Re: Patch: Rating in Town Label [v13-r18283]

Post by Vaulter »

Check this out
Just patch against trunk
Attachments
rating_town_label_r18684.patch
(7.17 KiB) Downloaded 126 times

Terkhen
OpenTTD Developer
OpenTTD Developer
Posts: 1034
Joined: 11 Sep 2008 07:32
Location: Spain

Re: Patch: Rating in Town Label [v13-r18684]

Post by Terkhen »

First post updated with a patch against current trunk (r18684).

User avatar
Lord Aro
Tycoon
Tycoon
Posts: 2366
Joined: 25 Jun 2009 16:42
Location: Location, Location
Contact:

Re: Patch: Rating in Town Label [v13-r18684]

Post by Lord Aro »

Bump Question: why is this not in trunk yet?
AroAI - A really feeble attempt at an AI

It is practically impossible to teach good programming to students that have had a prior exposure to BASIC: as potential programmers they are mentally mutilated beyond hope of regeneration. --Edsger Dijkstra

Terkhen
OpenTTD Developer
OpenTTD Developer
Posts: 1034
Joined: 11 Sep 2008 07:32
Location: Spain

Re: Patch: Rating in Town Label [v13-r18684]

Post by Terkhen »

Read the full thread to see all the tried options to show the town rating. The final solution is no good either: it is confusing because colours are already used for stations. There are not enough "standard" colours to show all ratings and as a result you lose some information. People with colour blindness can't distinguish some town rating colour / background combinations.

Showing changes in town rating via a text effect (as already done for costs/profits) is not good either. Knowing that the rating has changed (which requires paying attention to the town while building) is not that useful.

In short: suggestions will be helpful to finish this feature, bumps without content will not.

User avatar
Lord Aro
Tycoon
Tycoon
Posts: 2366
Joined: 25 Jun 2009 16:42
Location: Location, Location
Contact:

Re: Patch: Rating in Town Label [v13-r18684]

Post by Lord Aro »

perhaps have the option to turn it off, or off by default
and people with colour blindness can't distinguish between half of what's in OTTD
AroAI - A really feeble attempt at an AI

It is practically impossible to teach good programming to students that have had a prior exposure to BASIC: as potential programmers they are mentally mutilated beyond hope of regeneration. --Edsger Dijkstra

Post Reply

Return to “OpenTTD Development”

Who is online

Users browsing this forum: No registered users and 7 guests