Patch: Rating in Town Label [v13-r18684]

[planetmaker]

I just compiled this test and did a brief test.

While it works nicely in a zoomed-in state (most and 2nd most zoomed-in), it doesn't change the colour of the town names in the two zoomed-out states. But I'd consider a colour change at those zoom levels actually most useful as at those zoomlevels I decide where I'm going to continue building.

The solution probably is to add the colour also to the string used in those two zoom states. My guess is that it uses a different one as the font size changes - which is part of the string definition in OpenTTD.

EDIT: hm... during compilation I get a warning for many language:

Code: Select all

/Users/ingo/ottd/clientpatches/src/lang/latvian.txt:1673: warning: STR_TOWN_LABEL_POP: template string and language string have a different # of commands

[Terkhen]

The patch have been updated again: see the first post for details. After reading your opinions and working on them, there's a second version of the patch with a different way of showing ratings. As I explained, using different strings (like ++, --...) would increase the number of strings and make the patch a bit too complicated. I tried to show it with a colored symbol, but I'm not very satisfied with the result... I'll be waiting for your feedback:D

planetmaker: Since I had to remove the original color from some strings at english.txt, there's a warning about every other language because they all have an extra command (the color I removed).

[ChillCore]

For me the symbol version works better.
But i guess it is matter of taste.
What symbol to use can be tuned later.

Did something change in the towncolour version ?
If so i will try that again too in a bit.

[Terkhen]

The only change was switching red with orange. I'll make some changes based on the russian community patchpack version of the patch tonight.

I tried to use a filled circle (●), but it isn't compatible with the english font.

[Terkhen]

Version v0.5 is ready!

Screenshots here

Changes: I finally found why _current_company != _local_company. Because of this bug, when the game just commenced the town names were white. A more exact color scheme is introduced. The code have been almost completely remade to be more efficient (no longer it needs to calculate the color each time the sign is drawn, only when the rating changes).

I'd like to know your opinion about the implementation of the patch. Since these functions are called a lot, I think they should be as fast as posible. The new color scheme is up to debate (red is back temporarily: just until I play a bit with the palette) and I'm still unsure about the symbol version.

[ChillCore]

Good to see you are taking care of reducing systemload.

If you are not sure about the symbol version you do not have to maintain it just for me.

Maybe some input from some more people will help you decide?

[Terkhen]

It's not very hard to maintain, actually I just update the color version and then I turn it into a symbol version. As you said, I'd like to have more feedback before deciding.

[planetmaker]

Actually I like the idea of a coloured bullet. The leading symbol as it's now is nice, but I guess it's much more work to replace it by something bullet like (but a bit bigger, fitting the font) as in the newgrf dialogue / online content download dialogue where we have red / yellow / green bullets. Such bullet would be more visible than the star as it is now.

My argument for the bullet or symbol version is such that with many companies within a town it may be hard to find the town names itself, if it may vary with colour, too. With only the symbol you can easily find the town name by means of searching for neutral white and then see the colour symbol right next to it.

[HackaLittleBit]

Nice one Terkhen.

I would go even more discrete.
If you don't mind do something like this:

Town Name - (97975) Red: Appalling rating.
Town Name - (97975) Orange: Very poor rating.
Town Name - (97975) Light Orange: Poor rating.
Town Name - (97975) Yellow: Mediocre rating.
Town Name - (97975) Light Yellow: Good rating.
Town Name - (97975) White: Very Good rating (standard).
Town Name - (97975) Light Green: Excellent rating.
Town Name - (97975) Green: Outstanding rating

And ahem no extra options we have enough already incorporate it with
"Show town poulation in the town name label"

EdiT
Change "Show town poulation in the town name label" into "Show extra town info in the town name label"



Regards HackaLittleBit

[Auge]

Hello

The leading symbol as it's now is nice, but I guess it's much more work to replace it by something bullet like (but a bit bigger, fitting the font) as in the newgrf dialogue

You mean something like this?

town-rating-sign.png (8.73 KiB) Viewed 5563 times

I would like this but I don't know how much work this would be. To use a char would be easier, but wich to use to catch it optically in a easy way?

Tschö, Auge

[HackaLittleBit]

Or like this

Town Name(97975) Red: Appalling rating.
Town Name(97975) Orange: Very poor rating.
Town Name(97975) Light Orange: Poor rating.
Town Name(97975) Yellow: Mediocre rating.
Town Name(97975) Light Yellow: Good rating.
Town Name(97975) White: Very Good rating (standard).
Town Name(97975) Light Green: Excellent rating.
Town Name(97975) Green: Outstanding rating

Regards

[Bilbo]

What about this?

Town Name (97975) Red: Appalling rating.
Town Name (97975) Orange: Very poor rating.
Town Name (97975) Light Orange: Poor rating.
Town Name (97975) Yellow: Mediocre rating.
Town Name (97975) Light Yellow: Good rating.
Town Name (97975) White: Very Good rating (standard).
Town Name (97975) Light Green: Excellent rating.
Town Name (97975) Green: Outstanding rating

[Terkhen]

Thanks everybody for the input!

Without thinking about implementation details, the two looks I like the most are colored bullet and town population showing the rating color. I already tried doing something exactly like Bilbo's suggestion some time ago: the problem is that I only can apply non-standard colors at the beginning of the string: that's why the symbol version puts the symbol first. I think I can use two adyacent signs to simulate a single string with different colors at the end, but I don't know how they will show after zooming out. With the colored bullet, I think I'll have a similar problem: after zooming out the bullet and the sign positions will show messed up. I'm going to give both ideas a try, I'll keep you informed

About hackalittlebit's suggestion: I agree that there's already too much options at the advanced options windows, and I was tempted to merge both options because they are basically the same: deciding about having just a town name or more info. In my opinion there's little reason to keep them separate, but I thought that maybe it would be requested if I merged them.

[Terkhen]

New version is out!

v0.6: Symbol version removed: you still can find the v0.5 version here. Now only the population is coloured. Setting option merged with "show population". Fixed a bug where the color won't update after town growth. Made sure that the patch can be applied to 0.7.

The patch and the screenshot can be found at the first post.

I finally decided to merge the settings because it simplify the patch and also removes all rejects against 0.7, so now the patch can be applied to 0.7 without problems. I removed the symbol (or bullet) version because using sprites at the town name sign would overcomplicate the patch source. I don't know any way to make it with characters (as I said, it must be a character compatible with all OpenTTD language fonts), but if someone knows how to do it, it won't be hard to remake symbol version.

[ChillCore]

Nice one Terkhen.
I like how the town is coloured when zoomed out
and the population is coloured and town is white when zoomed in.

I had a quick look through your patch.
you seem to have removed/added a few white lines here and there.

I will be testing some more while playing ...

[Terkhen]

Thank you

I'll take care of tab errors and white lines for next version. For testing: since the patch now uses two signs instead of one, it can have bugs at updating the viewports, building near the signs, etc. Also it's possible that there's some code that updates the rating but doesn't trigger an update in the sign. I'm pretty sure every possible bug was taken care of, which means that there's a huge error somewhere

[Terkhen]

Update without new features or changes.

v0.61: Update to trunk. Added version for stable 0.7.0 (which is essentialy identical to v0.6)

Both patches are in the first post.

[Terkhen]

Update to current trunk.

v0.62: (Minor) update to trunk.

Do you feel there's anything missing in the patch?. Anything that could be improved?

Here's a windows binary for testing purposes:

EDIT: Removed, see newer post for an updated binary.

[CommanderZ]

Post it to bugspray to get some feedback from devs

[Hirundo]

Some things that could be improved, in my opinion, are the following:

• Coding style (e.g. trailing whitespace, improper alignment, '}' should be on the same line as 'else') In my experience, reading the diff closely and examining every line of it helps a lot, although it takes some time.
• Code duplication. All those if statements in TownRatingColour seem somewhat excessive to me. You may want to check how this is done in the gui. The code in viewport.cpp also seems a bit duplicative.
• In the current trend of C++-ification, TownRatingColour could be a method of the Town class.
• Are the colours from the TextColour enum not sufficient for your needs?