Smoovious wrote:

Ok... I added a setting to set how many real-time seconds chat messages stay on the screen... I kept the min 10 game-day limit also... I adjusted some message settings to keep more on the screen, and to keep lots of long broken-up messages from falling over the top of the chat window... probably do more later, but for now, I got the objective of this request...

Probably some style issues to it...

Here it is..., the diff is based on r9127...

-- Smoovious

Attachments

• SmoovPatch[text_stays_visible].r9127.200703111835.diff (7.43 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1053

Smoovious wrote:

ps: diff was tested in Win32 and OSX... still needs testing on the other platforms...

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1054

Smoovious wrote:

removed some redundant code of mine
probably still has style issues... >sighs< :)

New: added setting to keep messages visible at least # of real-time seconds
Change: adjusted message count limits to allow more text to remain on the screen
Fix: adjusted chat window size so when you have many multi-line messages, ones at the top won't fall over the top of the window anymore
Removed: Unneccesary FOR loop in TextMessageDailyLoop() used to remove old messages. Now only the oldest message will be checked for deletion, once per game-day

-- Smoovious

Attachments

• SmoovPatch[text_stays_visible].trunk.r9127.200703121423.diff (8.51 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1062

Smoovious wrote:

Removed: now-unnecessary 'duration' variable (and related variables)

also removed a couple more bits of redundancy I introduced.
style cleanup

-- Smoovious

Attachments

• SmoovPatch[text_stays_visible].trunk.r9153.200703131926.diff (8.39 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1070

Smoovious wrote:

Ok, got a new patch...

-Feature: message duration is now configurable to real-time seconds through the patch menu
-Feature: shaded area now only shows behind the actual text, not the entire window area
-Fix: many long messages now won't force the text over the top of the chat window area
-Change: can now have more chat messages on the screen

plus some various comment and code cleanup from my earlier patches

Screenshot included.

-- Smoovious

Attachments

• Unnamed, 3rd Jan 2000.png (57.52 KiB)
• SmoovPatch[text_stays_visible].trunk.r9259.200703161755.diff (9.80 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1075

Smoovious wrote:

ugh... forgot screenshots don't save the visible text. :(

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1076

Smoovious wrote:

Lets try this one...

Attachments

• Screenshot of new chat text.jpg (258.56 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1077

Smoovious wrote:

Hopefully, this is the last revision of the patch...

-Feature: message duration now configurable
-Feature: shaded area now configurable to none|lines|window
-Fix: many long messages now won't force the text over the top of the chat window area
-Change: can now have more chat messages on the screen
-Cleanup: made some ugly code that actually draws the messages/shadow, a little less ugly

-- Smoovious

Attachments

• SmoovPatch[text_stays_visible].trunk.r9263.200703170235.diff (12.35 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1079

Smoovious wrote:

Revised to current trunk (as of r10349)

-Feature: chat message duration now configurable
-Feature: shaded area now configurable to none|line|box(default)
-Fix: many long messages now won't force the text over the top of the chat window area
-Change: can now have more chat messages on the screen
-Cleanup: made some ugly code that draws the messages/shadow, slightly less ugly

-- Smoovious

Attachments

• SmoovPatch[chat_text_visible].trunk.r10349.200706261918.diff (12.91 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1470

Smoovious wrote:

Revised to current trunk (as of r10369)

made some minor changes also which didn't change the logic.

-- Smoovious

Attachments

• SmoovPatch[chat_text_lifespan].trunk.r10369.200706280119.diff (13.69 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1479

Smoovious wrote:

Revised to current trunk (as of r10450)

messages will now dissapear regardless of if you're paused or not. (thanks to peter1138 pointing out another option available to me)

also did some renaming, to make it clearer which parts were for the chat window, apart from those for the text effects.

-- Smoovious

Attachments

• SmoovPatch[chat_text_lifespan].trunk.r10450.200707060242.diff (22.86 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1534

Rubidium wrote:

The chat messages will stay forever (well, until other chat messages overwrite them) when _realtime_ticks overflows.

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1719

Smoovious wrote:

hmm... how often would it overflow tho... thought I figured it out to something like 45 days, but I may be remembering wrong...

-- Smoovious

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1723

Rubidium wrote:

Every 49 days IIRC, but that is NOT equal to after 49 days!

So we are going to get bug reports about this issue.

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1724

Smoovious wrote:

Well, not a big deal... I've dealt with handling byte-rollover issues before... will update this when I finish something else I'm working on.

Thanks for the critique.

-- Smoovious

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1725

Smoovious wrote:

-Fix: handle _realtime_tick byte rollover gracefully

-- Smoovious

Attachments

• SmoovPatch[chat_text_lifespan].trunk.r10697.200707261229.diff (23.63 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1728

Rubidium wrote:

Why did you clutter this patch with the text->chat change too? It makes it very hard to see what actually changed and what didn't change (except for the naming).

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1870

Smoovious wrote:

It more clearly differentiates the chat-related stuff, from the rest of the more generic text-related stuff.

As far as I was able to determine, and I did a LOT of looking before I changed it, there weren't any other text-related routines that called those chat routines, that didn't have anything to do with the chat window.

When I first made the patch, it wasn't clear which text routines were associated with each other. The more I worked on it, the more I realized how seperate the chat routines, and the text effect routines were. Soo... since they're all about the chat window, I renamed them for the chat window... instead of text windows/effects.

(and I considered the change, to be the opposite of 'clutter')

-- Smoovious

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1903

Rubidium wrote:

I didn't say you cluttered the code, I said you cluttered the patch itself. The name changing makes it non-obvious what really is changed.

The name change in itself is good, but doing a name change and functionality change in the same patch is not.

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1904

Smoovious wrote:

I feel it is inappropriate for me to make a change like that, in a patch, if I'm not making other changes in there as well...

It is one thing if I'm making the change as part of other changes I'm making in there... quite another if I'm just making a patch to change function names by themselves, with no other interest in that section of code, considering I'm not one of the top-tier coders, that is...

-- Smoovious

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1905

Rubidium wrote:

It is not inappropriate to suggest such naming changes, especially if it makes a difference between several parts of the code more obvious. I myself started with some whitespace patches, which got applied pretty fast.

Doing them together with functional changes just makes it harder to review the patch, which wastes time on both sides and interest in the "top-tier coders" in the review process. There's some "law" somewhere that states that the longer a patch is, the harder it is to get it included and the more difficult/complex a patch is, the harder it is.

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1906

Smoovious wrote:

k... well... I won't have much time for a while to seperate them... in the middle of truck driving school... taking up a huge amount of time...

I should be able to have seperated patches for them by end of next week...

-- Smoovious

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1907

Smoovious wrote:

Ok, here's a patch that only covers the variable/function name changes (plus a few comments)

-- Smoovious

Attachments

• SmoovPatch[chat_text_cleanup].trunk.r10931.200708182306.diff (14.29 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1908

Smoovious wrote:

Updated to reflect changes made by my chat_text_cleanup patch being committed (r10932)

-- Smoovious

Attachments

• SmoovPatch[chat_text_lifespan].trunk.r10932.200708191752.diff (16.95 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment1913

Smoovious wrote:

What more do I need to do on this patch for it to be accepted into trunk at this point?

-- Smoovious

(besides, get it current again)

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment2910

Belugas wrote:

1. wait for after 0.6 release
2. try to remember where was it said/promised/suggested that we did liked the patch and that we want it in trunk

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment2914

Smoovious wrote:

well, it isn't easy to remember at this point exactly who said they liked it and what-not... I haven't had anyone say they didn't like it... I've had constructive criticizm of coding style from time to time, which I had addressed. It had been well received among the people who did try it, and gave me comments.

I just don't want to keep updating it if it has been decided that it isn't going in...

Even more constructive criticism would be nice.

I don't spend a lot of time at home anymore, so gotta cram in my work on it within a few days at a time...

Guess what I want most is some kind of feedback... .. .

-- Smoovious

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment2926

Smoovious wrote:

Updated to r11610...

-- Smoovious

Attachments

• SmoovPatch[chat_text_lifespan].trunk.r11610.200712091702.diff (16.79 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/532#comment2927

Rubidium closed the ticket.

Reason for closing: Implemented

In r21513

This comment was imported from FlySpray: https://bugs.openttd.org/task/532