OpenTTD

Tasklist

FS#104 - Enhanced financed window

Attached to Project: OpenTTD
Opened by Zr40 (Zr40) - Friday, 07 April 2006, 09:01 GMT
Last edited by Peter Nelson (peter1138) - Saturday, 31 January 2009, 21:27 GMT
Type Patch
Category Interface
Status Closed
Assigned To No-one
Operating System All
Severity Low
Priority Normal
Reported Version trunk
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 1
Private No

Details

I've made a patch which reorders the finances window. It groups together recurring expenses, income and others, and shows a total for each group.
Comments are welcome!
This task depends upon

Closed by  Peter Nelson (peter1138)
Saturday, 31 January 2009, 21:27 GMT
Reason for closing:  Implemented
Additional comments about closing:  In r15301
Comment by Zr40 (Zr40) - Friday, 07 April 2006, 09:26 GMT
New patch. Changes:
- Patch made with patch, not with tortoisesvn.
- Made finance window wider.
Comment by Zr40 (Zr40) - Friday, 07 April 2006, 09:46 GMT
New patch. Changes:
- Moved income group to the top, switching places with the expenses group.
Comment by Zr40 (Zr40) - Friday, 07 April 2006, 09:54 GMT
Seems the patch wasn't attached.
Comment by Zr40 (Zr40) - Friday, 07 April 2006, 16:07 GMT
New patch. Changes:
- Moved 'other' to third group, as it includes local authority costs.
- Added comments describing which group is being drawn.
Comment by Zr40 (Zr40) - Tuesday, 25 April 2006, 18:15 GMT
Attached a screenshot of the enhanced finances window.
Comment by precision (precision) - Thursday, 04 May 2006, 08:44 GMT
# There is something wrong with the running cost "Total" (and the final "Total") on the screenshot.

# Adding a 4th column which contains a sum for each line would be a good extention.
Comment by Meush (Meush) - Sunday, 20 August 2006, 07:41 GMT
I second precision's comment, and I'd like to see some colour difference between adjactent rows/groups to distinguish the values easily. On the other hand, it would be much easier to commit a smaller patch, and add eye-candies later.
Good job on this patch. Small and useful :)
Comment by Zr40 (Zr40) - Sunday, 06 May 2007, 10:45 GMT
I've updated the patch to apply again. It applies against the current trunk: r9793. Changes:
- Fixed the problem precision mentioned by precision.
- Added comments explaining the various indexes.
Comment by Zr40 (Zr40) - Sunday, 06 May 2007, 10:56 GMT
...that should be 'Fixed the problem mentioned by precision.', of course.
Comment by Patric Stout (TrueBrain) - Sunday, 06 May 2007, 11:15 GMT
I did a quick code-wise check, and some comments:

- if (tally < 0) { tally = -tally; str++; } <- wrong, put it on 2 lines
- 'tally', wtf is that? Use a better name?
- DrawString(2, 27 + i*10 + 140, STR_7011_CONSTRUCTION + i, 0); <- please use spacebar... 'i*10' -> 'i * 10'
- don't use tabs in GUI defines; use spaces, like everywhere else

That were the 4 things I notice on the first glance. Sorry, but code-style is important (and a bitch :))
Comment by Zr40 (Zr40) - Sunday, 06 May 2007, 11:42 GMT
- player_gui.c currently contains those one-liners, which I've copied and pasted. I've changed them to two lines, and altered the already present lines as well.
- A tally is a counter. According to my dictionary: a current score or amount. I've renamed it to 'total'.
- Those missing spaces originate from the original patch, where back then, player_gui.c contained the same code style. I've noticed that has changed, but apparently I missed some instances.
- As for the tabs, I don't know how they ended up in there. They've been removed.
Comment by Zr40 (Zr40) - Sunday, 06 May 2007, 11:58 GMT
New patch. Changes:
- Fixed wrong displayed total.
Comment by Zr40 (Zr40) - Tuesday, 15 May 2007, 20:22 GMT
New patch. Changes:
- Removed trailing tabs.
Comment by Zr40 (Zr40) - Thursday, 31 May 2007, 20:41 GMT
I was told on #openttd the patch needs a rewrite. However, it wasn't stated what exactly needs rewriting.

So... What needs to be done? :)
Comment by Zr40 (Zr40) - Tuesday, 19 June 2007, 22:13 GMT
I've updated the patch to apply against r10230.
Comment by Zr40 (Zr40) - Tuesday, 19 June 2007, 22:29 GMT
It seems patch (the program, not my patch) ate part of r6924 when I was working on -v5. Fixed.
Comment by Zr40 (Zr40) - Thursday, 28 June 2007, 19:56 GMT
I've updated the patch to incorporate changes in r10208-r10209. It now applies against r10380.
Comment by Zr40 (Zr40) - Sunday, 15 July 2007, 14:42 GMT
I've updated the patch with the following changes:
- More comments. Everybody wants comments :)
- Reorder header name drawing calls to follow screen order instead of index order.
- Reduce coordinate math where possible.

Patch applies against r10581.
Comment by Zr40 (Zr40) - Wednesday, 18 July 2007, 11:07 GMT
Patch updated for changes in r10587.
Comment by Zr40 (Zr40) - Friday, 27 July 2007, 19:03 GMT
Patch updated for changes in r10704.

Can this be committed, please?
Comment by Patric Stout (TrueBrain) - Saturday, 18 August 2007, 11:09 GMT
Some questions:

1) maybe you can make the numbers enums, so you don't have to add in comments what each number represent? Makes modifying things much easier
2) the result I personally do not like too much, as it becomes rather hard to read... I dunno what exactly makes it so, as in fact it is useful to see it per category. Maybe make tabs for each category where you can review things in more detail (like: expected income at end of year)?
Comment by Zr40 (Zr40) - Tuesday, 04 September 2007, 20:51 GMT
1: Agreed, but that doesn't completely solve the problem. You still have the loops, which are dependent on the order (which is probably not documented anywhere). Maybe unrolling them is the solution?
2a: Perhaps it's not really clear which section is what? If so, all that's needed is section names.
2b: I don't like the tab idea, and it's also not trivial to implement - all I did was some simple reshuffling :)
2c: Trains show profit for this year and last year. Do they show expected profit?

I've updated the patch to follow changes in recent commits. It applies against r11043.
Comment by Zr40 (Zr40) - Wednesday, 06 February 2008, 23:25 GMT
I've updated the patch for changes since r11043. It applies against r12073.
Comment by Zr40 (Zr40) - Thursday, 07 February 2008, 20:11 GMT
I've removed most loops; loop bodies have been extracted into functions. I've also replaced magic numbers with enums where possible.

The patch applies against r12083.
Comment by Zr40 (Zr40) - Thursday, 07 February 2008, 20:59 GMT
Removed sums from draw functions; placed in main DrawPlayerEconomyStats function. As a result, the DrawPlayerEconomy{Sum,Total} functions are redundant and have been removed.
Comment by Zr40 (Zr40) - Thursday, 07 February 2008, 21:56 GMT
New patch. Changes:
- Widgetified labels.
- Total is now shown when zero.
Comment by Zr40 (Zr40) - Saturday, 31 January 2009, 15:02 GMT
Updated patch to apply against r15297.

It's been almost three years. What does it take to get this committed?
Comment by Zr40 (Zr40) - Saturday, 31 January 2009, 15:47 GMT
Changes:
- Fixed copy/paste error due to applying the 2008 patch manually. (It didn't apply because of a year worth of changes in svn.)
- There's some magic somewhere. Moved the Borrow/Repay buttons to the correct position to accommodate the magic.

Loading...