Finances window: maintenance, profit, running costs... per each type of tranport

[jk271]

Hello, I am working on a patch that enables to see running costs, maintenance, profit, etc. per each type of transport:
monorail, train, tram, truck, ship, air, powerlines, others (warehose, posts, etc.).  I used departmentalisation from line management window.

I am running around 200 lines and it is hard to distinguish profit or loss of types of transport. Main motivation for writing patch was:
a)      find out powerline maintenance. --> Does my powerline make profit or loss?
b)      see profit/loss of tram transport (tram rail maintenance is quite height high).
c)      ...

This or something similar has been discussed here:
http://forum.simutrans.com/index.php?topic=1506.0
http://forum.simutrans.com/index.php?topic=6813.msg65858#msg65858

This patch is not ready for merging yet.

I would like to ask you a question:

* Where (in which category) should I include manglev?
 a) with monorail (in same tab as one type of transport)
 b) separate tab
 c) in "others" tab

This patch changes will change saved game structure, so some new features can be implemented too. (To minimize number of different saved game formats.)

* fixed (per time) maintenance costs of vehicles in money_frame
 (discussed here http://forum.simutrans.com/index.php?topic=1920.0 )

*toll - satelite system (no toll gates), not for citycars, only for player's vehicles
       - pay for distance cost = k*wehicle_weight*distance, where k is a parameter, e.g. 0.01c per tile per one tonne. If k is small, it should not affect pakset's setting. This feature could be easily disabled - set k=0.00c
       - it would make player to consider building his/her own roads instead of using public infrastructure
       (discussed here  http://forum.simutrans.com/index.php?topic=6404.0 and http://forum.simutrans.com/index.php?topic=6888.msg66398#msg66398 )

Feel free to add your comments.
                                         
P.S. This patch was made against d681184b83fb214dc60abc447ef1dfc8940f2bb8 (from 19.2.2011)

[Dwachs]

nice idea!

I am running around 200 lines and it is hard to distinguish profit or loss of types of transport. Main motivation for writing patch was:
a)      find out powerline maintenance. --> Does my powerline make profit or loss?
b)      see profit/loss of tram transport (tram rail maintenance is quite height).

To have some statistics for these questions is indeed very helpful.

* Where (in which category) should I include manglev?
  a) with monorail (in same tab as one type of transport)
  b) separate tab
  c) in "others" tab

(b) - you can look in gui/schedule_list.*, where the tabs are implemented for the line management window (you can even use the truck/train/etc symbols in the tabs).

The tabs in the line management window use the same instance of a list. After changing the active tab, the list gets refilled. You could use the same too: After changing active tab, only change the numbers to be displayed. This would reduce code duplication in you patch.

You use a lot of named constants COST_xx_ROAD etc. I think it would be better to have an array for these statistics indexed by waytype directly (with special treatment of powerline_wt).

[prissi]

The top space in the window is already used for scenarios messages. Some other statistics (like the number of pas, mail, goods) is not shown, although those are recorded for some time. It would be very good if those things can be taken into account too, without exceeding 640x416 pixel size.

Probably the built headquarter tool needs to go away for those changes.

And you can easily check for the availability of certain modes of transport (like no costs, not transport, no number of cars) and only show tabs for the ones in use.

[jk271]

(b) - you can look in gui/schedule_list.*, where the tabs are implemented for the line management window (you can even use the truck/train/etc symbols in the tabs).

The tabs in the line management window use the same instance of a list. After changing the active tab, the list gets refilled. You could use the same too: After changing active tab, only change the numbers to be displayed. This would reduce code duplication in you patch.

It is excellent suggestion! I do not know internal of source code of game well yet. I will have to explore line management more to find out how it works.

The top space in the window is already used for scenarios messages. Some other statistics (like the number of pas, mail, goods) is not shown, although those are recorded for some time. It would be very good if those things can be taken into account too, without exceeding 640x416 pixel size.

Thank you for writing about messages. I have not known it. I also did not known exact dimension of window.

I would like to include statistics but I am not decided where to localize it in the window - between money and chart or in a new subtab. It would depend on place (rows that fit into the window).

I found extension request http://forum.simutrans.com/index.php?topic=6951.0 to consider - proceeds split up by passanger, mail, freight. I think, that freight categories are pack dependent so I would not split them up.

Probably the built headquarter tool needs to go away for those changes.

I hope it would be let as it is.

And you can easily check for the availability of certain modes of transport (like no costs, not transport, no number of cars) and only show tabs for the ones in use.

Good idea !

Thank for comments. It will take me some time to implement it.

[IgorEliezer]

Probably the built headquarter tool needs to go away for those changes.

Small idea: transfer "Build Headquarter" to "Special Construction" toolbar. It's not an issue since there already are "Found a new city" and other more expensive/non-essential tools.

BTW, topic renamed.

[jk271]

I

Small idea: transfer "Build Headquarter" to "Special Construction" toolbar. It's not an issue since there already are "Found a new city" and other more expensive/non-essential tools.

Good idea. It's logical to have it in some other place. On the other hand I don't know how to make menus (and handle it too) from program (source code) yet. So I let HQ button unchanged (in tab 'all'). Patch is growing up (124KiB now) and it only one around half of work. Moving HQ button can be implemented in some another patch in future.

Edit: I omited that there is a picture of HQ on finance window. Space of  the picture of HQ might be needed for statistics of transported passenger, mail, goods and all.
New screenshot of finance window attached.

[The Transporter]

  It's a great Idea!
I hope it find the way in to the Game.

[prissi]

In one moves the scenario completion message to the status line (where it anyway should go, same as the month with minus warning) such a restructuring of the dialog would be good.

[dom700]

About the powerlines.
How are you supposed to identify how much money you make with them? Lets imagine the most basic beginning: An oil producer, a refinery and a petrol station as well as any kind of powerplant. If you connect the power plant to the petrol station you will loose money (most definately) for the power alone, but the needs for fuel go drastically up, adding lots of transports without any additional maintenance costs, except for vehicles.

[Randy007]

@jk271
Are you still working on the patch?

[jk271]

Firstly I have apollogize for not replying for such a long time. The patch is quite huge and I was not sure whether I am able to finish it.


Since some time I have began to rewrite it from scratch and today you can see result.
Fortunately there are patches now. The first one (master1857_ ...) changes logics and can be applied without anything else. This patch is appliable to current master  branch (commit 1857c856... from 16 th December 2011).
The another one changes graphics and needs first one having applied.


I was not sure about comment of functions in cases when I copied a fuction from file money_frame.cc to file money_tab and when I changed the function. Some of these functions contained string "@author ..." and I was not sure what exactly do with this.


Feel free to change to code, rename variables to more suitable names, etc. I hope there are not many unnecessary changes introduced by patches.


The thing I am not happy about is size of a chart in the bottom. On the other hand I did not found better place for it.


I hope that this is final version from my side. I have tested it. And now it needs testing by someone more aware of simutrans source code.

[Fabio]

A few sparse thoughts

~ charts like this are little more than useless. They could be made resizable. If you can make the window bigger than default the chart will get bigger too. See new minimap on townhall details.

~ Chart shows only last 10-20 yrs. It could have more sub tabs with scale 1 month, 1 year, 3 years, 5 years, 10 years (showing up to 200 years).

~ Button to export financial data in csv

~ scenarios should go away and have a window of their own, with goals, messages etc.

~ HQ should go away too, best in other bldg menu

~ in charts different units should have different scales. E.g. Margin in % with 0 in the middle of y axis, regardless if it'shown together with values in cash

[jk271]

Thank you for your coments

A few sparse thoughts

~ charts like this are little more than useless. They could be made resizable. If you can make the window bigger than default the chart will get bigger too. See new minimap on townhall details.

It is excelent idea. I hope that I can find out, how it works. The window is 410px high, because there was wiritten not to exceed 640x416 pixel size.

~ Chart shows only last 10-20 yrs. It could have more sub tabs with scale 1 month, 1 year, 3 years, 5 years, 10 years (showing up to 200 years).

Showing more years can be changed by setting macro variable MAX_PLAYER_HISTORY_YEARS to higher values. But it arises problem with loading old games. So more coding in spieler_t::rdwr is needed.
Another issue is performance issue. When new year or new month comes, the whole content of table is shifted. Bigger table (or more tables) need more memory too. There are 3 table for year's finance history introduced by this patch having 8-byte items: 7*12, 9*34*12 and 7*12. And the month's finance history needs same amount of memory. It is around 60kiB per player.  And there is not only one player.

~ Button to export financial data in csv

~ scenarios should go away and have a window of their own, with goals, messages etc.

~ HQ should go away too, best in other bldg menu

Good idea for some future work.

~ in charts different units should have different scales. E.g. Margin in % with 0 in the middle of y axis, regardless if it'shown together with values in cash

This is useful, but I do not know how to make it yet.

[Dwachs]

The thing I am not happy about is size of a chart in the bottom. On the other hand I did not found better place for it.

Yes this window looks much too crowded now. What about the following idea: Let the 'All' tab show the same content in the finance window as now. Then add tabs for waytypes (like you did), and one or more extra tabs for goods. The number of good categories is not fixed, it may vary from pakset to pakset.

are you on github?

[VS]

Narrow gauge waytype is missing, while already present in pak128.Britain...

[falconne]

Like Dwachs said, I reckon the revenue from different categories should be moved to new tab. The current tabs should only contain one total revenue figure for the relevant way type.

[jk271]

Yes this window looks much too crowded now. What about the following idea: Let the 'All' tab show the same content in the finance window as now. Then add tabs for waytypes (like you did), and one or more extra tabs for goods.

Like Dwachs said, I reckon the revenue from different categories should be moved to new tab. The current tabs should only contain one total revenue figure for the relevant way type.

Excellent suggestion. I will split up each tab into two subtabs as can be seen on the picture. I drew tabs as a red and a blue rectangles in the left top corner.
Red one (=first tab) will contain labels and buttons from a red rectangle in the bottom and  HQ. The blue one will contain buttons and labels from a blue polygon in the middle. Chart will be left on tab "All".

The number of good categories is not fixed, it may vary from pakset to pakset.

The code in the patch distinguishes 8 cathegiries (from 0 to 7). Cathegory 0 is split up into three parts: passenger, mail, other items from cathegory 0.
Each another cathegory and COST_TRANSPORTED_GOODS (from old save game format) are mapped onto cathegory 7.
Are there any paks using some another cathegories than 0, 1, ..., 7 ?

are you on github?

Not yet. I am using git on my computer. I will consider using github

Narrow gauge waytype is missing, while already present in pak128.Britain...

Thank you for reporting this bug. I am using mostly pak64 with food addons.

[prissi]

1) I do not think it would be wise to discreminate all income for each good categories and mode of transport (if this was your aim) as often chains could use more than one mode of transport. Thus, better do not use subtabs in tabs. This is really very bad GUI design. (Normal tabs are only very bad ... )

2) You cannot really merge all goods with catg. 0; Those are cars, paper, and many different kinds of goods in severl pak sets. Each of them should get their own line. There is actually an uinique catg index counter in simware.cc (which goes to 12 or so for a typical pak), which tells how many different non-interchangable good categories you have. If out of space, I would rather use a scrollbar.

[falconne]

Excellent suggestion. I will split up each tab into two subtabs as can be seen on the picture. I drew tabs as a red and a blue rectangles in the left top corner.

Actually I meant the goods don't need to be broken down by way type at all, so you may as well just have a new tab for "Goods Income", that shows total revenue per good.

[jk271]

1) I do not think it would be wise to discreminate all income for each good categories and mode of transport (if this was your aim) ...

Yes, it was my aim to display each good category for each mode of transport.

... as often chains could use more than one mode of transport.

I tried out this fact in the game using the most recent build from code of master branch ( = no my code was included): I have build two lines. First line (ship) transports crude oil from oil rig to harbour. The second line (truck) transports crude oil from harbour to Chemical Plant. Each transported item was counted twice. Firstly when reached harbour and secondly when reached final stop. Unless this behavior is changed, I will be able to get mode of transport of thansported item.


So the way, how my code counts number of transported items, does not change this behavior. It only marks transported amount to the 2-dimensional table with first index of transport mode and second index of transported category.

Thus, better do not use subtabs in tabs. This is really very bad GUI design. (Normal tabs are only very bad ... )

Do you mean how does it look like or source code of it (or both)?


There are some suggestion, how to solve it:
1) tabs by type/mode of transport and the whole window is realizable (but i has tabs)
2) two level tabs (not acceptable)
3) tabs by mode of transport and scrollbars
4) something another, suggestions welcomed

2) You cannot really merge all goods with catg. 0; Those are cars, paper, and many different kinds of goods in severl pak sets. Each of them should get their own line. There is actually an uinique catg index counter in simware.cc (which goes to 12 or so for a typical pak), which tells how many different non-interchangable good categories you have. If out of space, I would rather use a scrollbar.

I am merging all goods from catg0 only in finance history statistics. There are (in current master branch) all transported goods counted up in "COST_TRANSPORTED_GOOD".
In my patch amount of goods is split up into catg0 up to catg7.  I tried to be pak-independent, therefore catg0 is not split up into cars, steel, etc. Imagine playing pak64. After some simutrans years you have decided to install food addon (or some another). It would be difficult to implement saving statistics of transported goods distinguished by each single item (steel, paper, ...), because number of item can change.

[prissi]

About saving: You can easily save what kind of goods "name" and its statistics one by one. Using a predefined set will always be not compatible enough.

Personally I still find statistics for each good and waytype too much.

Tabs are bad UI design by ergonomic, since the hide a lot information without giving any hint what might be missing. Subtab, which are only seen on some tab, are even worse, since before you even did not know there was an UI element on that position.

Thus a global statistic of income by goods would be enough for me. For other purposes you could filter the vehicle list by goods to see which vehicle are doing will with a certain good and which not and need replacement.

[jk271]

I am stopping development of some huger change of finance statistics from my side for some time. I have to read and try out more simutrans code to understand it better, mostly graphical user interface. It takes me some time.

I made a little patch, which only displayed collected statistics of amount of transported passenger mail and good. These data were collected by simutrans for a long time. The patch changes only graphics, savegame format is not affected.

Personally I still find statistics for each good and waytype too much.
....
Thus a global statistic of income by goods would be enough for me. For other purposes you could filter the vehicle list by goods to see which vehicle are doing will with a certain good and which not and need replacement.

If finance window was as it is now in master branch and there was a filter enabling  to display same information, as is displayed now, by type of transport, would it be more acceptable? My motivation for work on financial statistic was to enable distinguish infrastructure maintenance by type of transport.

[Dwachs]

Tabs are bad UI design by ergonomic, since the hide a lot information without giving any hint what might be missing.

If there in finance window there is a tab called 'trains' then one usually expects train related numbers there. (Good design - it meets user expectations). If under the train tab in finance window you find for example a list of trains or numbers related to powerlines this would be bad design.

Personally, I like the idea of statistics per good category and per transport type.

What about the following proposal:
-- have tabs like:
------- 'All' - contains the same numbers as of now,
------- one tab per transport type (train, ships etc), here the icons as in the line management window can be used (schedule_list.cc)
------- 'Powerlines'
------- 'Goods'
------------ with a drop-down menu to filter for a specific good or goods category.
------- 'Misc' here the informaiton on headquarter and scenario could be hidden


Edit: I would like to suggest to proceed first with the collecting part of your patch. Here I have the following comments:

(1) Please remove the transport_type. There is a unique mapping between waytype_t and transport_type. The mapping from waytype to index in statistics array can happen within the member functions of spieler_t. This would reduce the footprint of the patch. This removes also the need to define an extra array for income of powerlines.

(2) Do not use fixed number of goods ATV_REVENUE_0 .. ATV_REVENUE_7 etc. Make another array to collect statistics of goods, where you collect income. Goods are index by ware_t::index, I suggest to use this index. Saving/loading should then happen based on goods name.

[jk271]

Firstly I have to thank you for yours comments to source code.

(1) Please remove the transport_type. There is a unique mapping between waytype_t and transport_type. The mapping from waytype to index in statistics array can happen within the member functions of spieler_t. This would reduce the footprint of the patch. This removes also the need to define an extra array for income of powerlines.

The reason, why a made transport_type I was not sure about trams, overheadlines and buildings. I tried to assign buildings like railway station to transport_type railway. When the mapping between transport type and waytype_t is bijective, it is clear.

(2) Do not use fixed number of goods ATV_REVENUE_0 .. ATV_REVENUE_7 etc. Make another array to collect statistics of goods, where you collect income. Goods are index by ware_t::index, I suggest to use this index. Saving/loading should then happen based on goods name.

It was revenue from good category. I found somewhere on wiki, that there are 8 categories used. This made me to assume fixed number. I agree, that using index is better. On the other it would need to make some indices (of finance_history_year, resp. month) having one dimesion more and some not.

Personally, I like the idea of statistics per good category and per transport type.

What about the following proposal:
-- have tabs like:
------- 'All' - contains the same numbers as of now,
------- one tab per transport type (train, ships etc), here the icons as in the line management window can be used (schedule_list.cc)
------- 'Powerlines'
------- 'Goods'
------------ with a drop-down menu to filter for a specific good or goods category.
------- 'Misc' here the informaiton on headquarter and scenario could be hidden

In my initial intention was not distinguish revenue and amount of transported goods per good. Maintenance by transport type was for me more important.


My proposal is: (I used yours with my changes in blue)
-- have tabs like:
------- 'All' - contains the same numbers as of now,
------- one tab per transport type (train, ships etc), here the icons as in the line management window can be used (schedule_list.cc)
---------- same content as has window today (except revenue from powerlines), figures concern to transport type
----------   optionally rows containing : revenue from:
------------- passenger
------------- mail
------------- goods (all goods together)
------- 'Powerlines'
Edit 28.12.2011 :
------- 'no waytype (or some better name)'  - everything, that can not be assigned to particular transport_type (waytype_t==ignore_wt)
----------- maintenance costs of buildings
----------- construction costs of terrain changes, ...
----------- also usefull for testing - to look up objects having not correctly accounted waytype_t
----------- possibly can be merged with following one - 'Misc'
------- 'Misc' here the information on headquarter and scenario could be hidden
----------- Excellent suggestion - scenarios needs more space for texts.


I would put off implementation of tab "goods". It would take a lot of time. It can be done in future.

[jk271]

I am resuming an effort to implemet more detailed finance window. I had not clue, how to avoid tabs.
Several months later I came up to an idea of combo box. Screenshot is provided below. Tabs has been transformed into items of combo box. Any other graphical changes are not needed.

Is this graphical design acceptable?

P.S. I am rebasing my devel brancg onto current master. I will provide patches some time later.

[greenling]

jk271
That Photo thats you make public looks very good out!

[Fabio]

Great you're back, jk271

The combo box idea seems quite elegant.



I'm moving this thread to larger projects.

[greenling]

Good day
I have with paint as I made a proposal to provide funding under Comenius to encourage the best for me would be arranged.

[prissi]

This design is certainly easier to grab.

[jk271]

greenling, Fabio: Firstly I have to thank you for your support. It very nice to read, that effort of making this set of patches is welcome.

I will describe my plans with this project:

I have split up project into following steps. Each step results in at least one patch.
1) MODEL
    - A lot of changes, mostly in player/simplay.{cc|h}
      Class spieler_t has been partially split up - new inner class finances_t containing new finance statistic and some olderly present variables has been created.
    - No changes of graphical interface. (Except new version of savegame in settings - 111.5).
    - Having applied this patch old money_frame can be used as well as new one (if "view" patch is applied).

    * Mostly done - waiting for testing ( step 2.1)
    * I am testing it now.
   

2) VIEW
    - Needed for testing of previous step
    - changes in /gui/money_frame.cc and .h
    - This step can be slit up into:
    2.1) MINIMAL CHANGES - Minimal changes needed to display newly collected statistic
    It is done.
    I am testing it now.
   
    2.2) GUI REDESIGN - More changes in interface design as suggested.
     I am considering this.

3) CODE CLEANUP
    - Removal of now unused functions etc.
    * optional - I have not searched them.

I am trying to test my patches now. I would like to release it in case it will have passed my tests.

My wish is to have parts 1) and 2.1) included in next release.

Redesigned money_frame looks well. On the other hand I would like to have applied and working parts 1 and 2.1. Than I can possibly work on redesign of money window.

[jk271]

I am happy to announce release of two patches adding more detailed statistic in finances_window!

First one (model.patch.zip) changes save game format (adds new tables for finance history per transport type).
I assumed, that new version (with patch) is 111005.
Most of finance statistics are stored in finance_t class. It is inner class of spieler_t. I consider is better than to have more variables spread out in spieler_t class.
This patch does not change graphical interface except addition of version 111.5 to setting window (gui/setting_stats.cc)
This patch can be applied as is without need of another code from my side as view.patch
Edit: This patch was done with revision 5684.

Second patch (view.patch) adds combobox to finance window (gui/money_frame.cc)
This patch needs to have applied first one (model.patch) as it uses some of new functions.
I had to move combobox above buttons on the right side because there is "company bankrupt message" in the bottom.
There is one small graphic bug - I do not know what to do with it - options of combobox are displayed below buttons. This is solved in patch view2.
See figure: view1

Second patch - alternative
There is an alternative view patch (view2.patch). It solves problem with combobox by small reorganizing of items on the finance window. I have been inspired by suggested changes.
I have moved "transported" line on top to move all content of left side down by one line as I needed space for combobox (if it is opened)
This patch is appliable if model.patch.zip is applied. This patch can not be applied with "view1"
See figure view2

I am providing both "view patches". Feel free to choose any of them.


I have tested it for several hours. So I think it is working well and is ready for testing by simutrans comunity or merge to current master branch to enable testing in nighties.

Notice: .patch file is normal .txt file

Edit 2: Link to compiled linux binary is:
http://simutrans-germany.com/files/upload/sim-money.zip
Unfortunately I am not able to crosscompile it fo windows.

[jk271]

I have discovered two bugs since I have released last set of patches.
1) Monthly stats for transport_type "all" were not summed up
2) similar problem for revenue and toll when game is loaded from older savegame version
So I releasing a fix.
(file model_fix.patch)
This file has to be applied after model.patch from previous message was applied.

There is also another patch (file model_fix2_savegame_format.patch) with improvement in savegame format of model.patch from previous post. This improvement saves some space by not saving old COST_ array in games with version 111005 or grater. Old COST_ stats are acquired using functions export_to_cost_month() and export_to cost_year()
This patch changes format of savegame version 111005.

File money20120505_model_witch_fix_1_and_2.patch.zip is one patch having content of model.patch from previous post and two bug fixes.

Edit: During process of beta testing I have revealed a bug in current master branch. When game is being loaded, profit is not calculated correctly. Revenue from powerlines and revenue/expenditures from way toll are not taken in account.
File profit_calculation_bug_fix_in_master.patch contains fix. This bug has been in master for a long time.
Patch does not conflict with any of my patches in this or last post and is appliable in current master branch on revision 5690 or on top of them (patches applied).

[prissi]

Thanks for the bugfix.

I personally would prefer statistics per categorie, i.e. pas, mail, papr, coal, ... However, I am well aware that this would be messed up by mixed convois.

[jk271]

Thank you for reviewing of the patches.
In my opinion is, that statistics per pas, mail, coal, steel ... are doable even with mixed trains as amount of transported cargo and revenue for transported cargo can be acquired per vehicle. On the other hand I would like to have done this small step (released patches) and let stats per pas, mail, iron ore, ... to another step.

I am not sure, how to deal with changes in categories yet: User had played and had saved the game. Than (s)he  added some new addons, for example food addon in pak64. Now he playes again with addons enabled. I have to think it over, what to do exactly in such situations. The result solution has to be robust. I have rough idea, how to do it, but it needs to become more mature.

Edit:
Link to new linux binary:
http://simutrans-germany.com/files/upload/simulinux20120507.zip

Edit2:Link to windows binary:
http://simutrans-germany.com/files/upload/simuwin20120508.zip

[jk271]

Patch set complete
I have finished third (and the last) part - code cleanup patch. Set of patches is complete now.

model - adds new functions and enables more detailed stats -
view - changes money window - small change since last release
code_clean_up - removes old COST_-related functions   several bug fixes - new
all - all 4 patches as one patch

There are precompiled binaries for Windows and Linux for curious users.

http://simutrans-germany.com/files/upload/sim.exe.zip

http://simutrans-germany.com/files/upload/simulinux20120511.zip


So what to do to have it included?

Edit: image added

[prissi]

I have too look deeper into the code.

[jk271]

Thank you for your quick response. 
J.K.

[Fabio]

congratulations, jk!

I always wondered why graphs in st go from right to left, I find it a bit counter-intuitive...

[jk271]

I have no clue, why they are from right to left. Probably default option. (I hope,) I have not changed anything with letf-to-right option.

Screenshot is from scenario "millionaire" from pak128, so it can come from settings in screnario.

Notice: Graphs can be switched between left to right and right to left in settings window on tab general by enabling/disabling option left_to_right_graphs.

[Fabio]

Thank you for checking... Honestly I would invert the default to left_to_right, although it's a tad off topic here...

[meme]

Will ever exist OS X version?

[Fabio]

If it's merged into trunk, there'll be...

[jk271]

The reason, why I have not provide OS X version is that I have not environment for compilation for OS X (To be more precise: I am not aware of having such an environment).

GNU/Linux binary has been compiled using g++  in standard Linux environment.
Windows binary has been compiled using g++.exe from Dev-cpp using wine (program, not content of a bottle ).
I am probably able to make binaries for Freebsd and Opensolaris. I wrote probable as I have such an environment but I have not tried to build simutrans on them yet.
If I had known how to cross compile simutrans on OS X would have compiled OS X version too.
If you know about some articles or howtos about cross compilation for OS X from GNU/Linux, post it here and I will try to make OS X version.

I am a little bit skeptic about merging to the trunk. I will be waiting for one or two months since last patch release and than I will start to consider reworking patch for simutrans experimental only.

[prissi]

It might be merged. But currently the work goes on with a release, so no big patches are to be incorporated. And considering the backlog of patches to be tested and integrated, it might take even longer. This needs to be profiled, tested and tweaked. Usually a patch require a week of work to incoporate on my side, and I have a high backlog of patches, I wrote and want to integrate. Since we do not have any team effort on integrating patches, most of them will be integrated with me. Since I have to care for a two week old again, my time is very limited.

[jk271]

I was not sure if the feature provided by this patch is acceptable in standard.  Thank you for your response - it made me sure.
P.S.: I am sorry for being so impatient.

[prissi]

No, nagging is good to go to the front. Even better would be nagging by other people ...

[meme]

The reason, why I have not provide OS X version is that I have not environment for compilation for OS X (To be more precise: I am not aware of having such an environment).

Simutrans is written in C++ , isn't it? If so, I could compile OS X version in Xcode ...

Or, second option is use of Macports - http://stackoverflow.com/questions/693952/how-to-compile-for-os-x-in-linux-or-windows

[jk271]

New version of simutrans with money patch have been released.

No new funkcionality (for money stats) have been added. It contains only a merge of latest changes revision (r5907) and some code refactoring.
Thank for comments and suggestions!

Binaries and source code can be downloaded from github:

https://github.com/jk271/simutrans/tree/money2012_code_cleanup/build/

Warning! Savegame format of this version of simutrans with money stats might not be compatible with previous release of simutrans with enhanced money stats.
I have not done any changes of savegame format, but changes might be done in other parts of source code.
Solution is simple: Save game with old binary to old game version format and than load it with new binary.

[Dwachs]

I still need to test it ...

Here some comments:

-- we will need a method to correctly retrieve tram waytype (independently from your patch...)
-- you have some methods finance_t::get_finance_something, the second 'finance' seems redundant.

Here is a diff for reviewing the patch:

https://github.com/jk271/simutrans/compare/master...money2012_code_cleanup

Edit: more comments.

About transport_type: For now it is enough to move the definition from simtypes.h to finance.h.

Why did you choose a different order in transport_type compared with waytypes ?

Also a label more intuitive than 'tt_other' is needed.

You have also some 'todos' left 

Imho the patch is nearing completion / inclusion. Good work!

[jk271]

Thank you for review of the patch!

-- we will need a method to correctly retrieve tram waytype (independently from your patch...)

What about new method:
waytype_t get_finance_waytype()
I can make a new branch (fork of master) and add such a method into classes, where it is necessary.
If you (or anyone else) know about better name for the function, feel free to suggest it here in this thread.

-- you have some methods finance_t::get_finance_something, the second 'finance' seems redundant.

You are right, I will try to remove redundant 'finance'.
Edit 10th September 2012: Unnecessary string _finance_ from names in finance_t class has been removed.

About transport_type: For now it is enough to move the definition from simtypes.h to finance.h.

Done.

Why did you choose a different order in transport_type compared with waytypes ?

Long time ago, when I began to work on the patch I have decided to make it in the same order as it is in linemanagement window. That is the only reason of order of transport_types.

TT_ALL is at first place as I expect new tranport types (e.g. funicular) might be added in future versions of game and I needed fixed place for stats of TT_ALL.

Order of transport_type can be changed without need of any another change of source code, but TT_ALL has to be first and TT_OTHER and TT_POWERLINE will be at the same place.
I can change the order if it is desired.
Edit 10th September 2012: transport types reordered.

Also a label more intuitive than 'tt_other' is needed.

I thought, that the string "tt_other" needs to be translated in translation files using simutranslator. Suggestions of any better and not yet used text instead of tt_other are welcomed.

You have also some 'todos' left 

Thank you for remind it. I will have to use "grep" more carefully. Merges are sometimes painfull.
Edit 10th September 2012: I have removed (= fixed) 2 todos. There should be no todo in code now.

[TurfIt]

Maybe I missed it, but is there a comment explaining the ATV_ and ATC_ prefixes?
  Accouting Type Veh ?
  Accouting Type Com ?  com??

[jk271]

Maybe I missed it, but is there a comment explaining the ATV_ and ATC_ prefixes?
  Accouting Type Veh ?
  Accouting Type Com ?  com??

Althow there are no comment in source code saying this, there are two enums in file player/finance.h:
accounting_type_common (line 60) and accounting_type_vehicles (line 78).

Thank you to point it out!

I have added comments like
ATC = accounting type common
ATV = accounting type vehicles
directly into source code in file player/finance.h.

[jk271]

I am happy to announce release of simutrans with patch containing more detailed finance stats.

RELEASE NOTES

r5955-rc3

The version r5955-rc3 does not include any new functionality. It incorporates only new code from trunk and several bug fixes. This version should be ready for playing or merge to trunk.

Savegame format of this revision is INCOMPATIBLE with previos versions of the detailed money stats.
Incompatibility is caused by changes merged from trunk branch: Some new settings
parameter has been added. Ability to read older (older than 111.5 excl.) game
version was not affected.
If you have played any older game release with money stats and you would like
to continue with new binary, save game with old binary into older format
(e.g. 111.0) and than load game with new binary (r5955-rc3).

Savegame version of money patch was changed from 111.5 (111005) to 111.6 (111006). Change was made due to code merged from trunk.

Thank for feedback!

Enjoy playing!

linux binary:
https://github.com/jk271/simutrans/blob/money2012_with_get_fin_waytype/build/linux32/simulinux-money-r5955-rc3

windows binary:
https://github.com/jk271/simutrans/blob/money2012_with_get_fin_waytype/build/win32/simuwin-money-r5955-rc3.exe

Edit 30th Sept 2012:
In my opinion is, that r5955-rc3 is ready for merge.

[Dwachs]

Sorry for the delay. I will look into this next week - hopefully.

[asaphxiix]

so, for the programming and stuff ignorant, some advice please:

is the patch incorporated in simutrans standard/experimental newest version, or the next release?

if not - can it be incorporated to standard/experimental safely by using the executable above?

or just need to wait? :+)

[Dwachs]

These changes are not yet incorporated neither in simutrans standard nor experimental.

You can use the executable above, without any warranties about savegame compatibility.

[asaphxiix]

nice
thanks dwachs!

[TurfIt]

Why is weg_besch_t::get_finance_waytype() virtual?

Code Select Expand


===> CXX bauer/wegbauer.cc
bauer/wegbauer.cc: In static member function 'static bool wegbauer_t::register_b
esch(weg_besch_t*)':
bauer/wegbauer.cc:126:10: warning: deleting object of polymorphic class type 'we
g_besch_t' which has non-virtual destructor might cause undefined behaviour [-Wd
elete-non-virtual-dtor]

Function is also sufficiently simple it could just go into the .h.

'trips' are only counting ~1/2 the value with patch as without?
Why move to top in finance window? No biggy, just looks strange after years of its old position...

'powerline' history is not loading correctly. Current year/month start at 0 after loading an old game.

Where'd the "congratulation scenario was complete..." stuff in player/simplay.cc come from? Nothing like it in trunk...

[jk271]

Sorry for the delay. I will look into this next week - hopefully.

Nothing to bother. Fixes should have bigger priority than this patch. I will continue in merging new code from trunk and in fixing of bugs found in money patch code.
I think about way, how to split the patch into several smaller patches to make integration and code review process easier and more transparent in history of repository.

asaphxiix:
Simutrans binaries with money patch are able to save and load game in savegame format of simutrans standard edition version 111.4 and older. So you should be able to load any game you had been playing with simutrans standard.

Incompatibility is only between issues of new savegame format (with detailed money stats) due to change of savegame format in main development line.

Why is weg_besch_t::get_finance_waytype() virtual?

Code Select Expand


===> CXX bauer/wegbauer.cc
bauer/wegbauer.cc: In static member function 'static bool wegbauer_t::register_b
esch(weg_besch_t*)':
bauer/wegbauer.cc:126:10: warning: deleting object of polymorphic class type 'we
g_besch_t' which has non-virtual destructor might cause undefined behaviour [-Wd
elete-non-virtual-dtor]

Function is also sufficiently simple it could just go into the .h.

Usage of "virtual" here is unnecessary. I will remove it.
If move the function to .h file circular depandency between header files will arise. That is the only reason from my side for placing the function in .cc file.

'trips' are only counting ~1/2 the value with patch as without?

This needs to be investigated. Probably some unwanted code due to merge. Unfortunately I am not able to to reproduce it. Could you provide more info? For example saved game.

Why move to top in finance window? No biggy, just looks strange after years of its old position...

I needed some space for selection of transport type. Original position was below "Net wealth". But scrolled list was not drawn correctly over chart, so I had to move it to another place.

'powerline' history is not loading correctly. Current year/month start at 0 after loading an old game.

This needs to be fixed. I will look on it

Where'd the "congratulation scenario was complete..." stuff in player/simplay.cc come from? Nothing like it in trunk...

It is probably some old code removed from trunk but still residing in money_patch. It is candidate for removal.

Thank you for comments and bug reports. They are very usefull! I will look on it and than I will add responses to all in this message.

[Ters]

If move the function to .h file circular depandency between header files will arise. That is the only reason from my side for placing the function in .cc file.

That should be curable with an extra (forward) declaration, shouldn't it? Though I don't think I've ever done that with functions meant to be inlined.

[TurfIt]

This needs to be investigated. Probably some unwanted code due to merge. Unfortunately I am not able to to reproduce it. Could you provide more info? For example saved game.

I was using the 'standard' testgame - http://www.physik.tu-berlin.de/%7Eprissi/simutrans/yoshi87-2-102.sve

than I will add responses to all in this message.

Editing message doesn't mark it as 'unread' - makes it easy to miss the new info...

[jk271]

That should be curable with an extra (forward) declaration, shouldn't it? Though I don't think I've ever done that with functions meant to be inlined.

The problem is that class weg_t contains enum needed by weg_besch_t::get_finance_waytype(). If it was a function, I could fix it using forward declaration.

When I try forward declaration, gcc says:

Code Select Expand


make
===> CXX bauer/brueckenbauer.cc
g++ -O -DNDEBUG -DMULTI_THREAD=2 -Wall -W -Wcast-qual -Wpointer-arith -Wcast-align  -DUSE_16BIT_DIB -I/usr/include/SDL -D_GNU_SOURCE=1 -D_REENTRANT -DCOLOUR_DEPTH=16 -c -MMD -o build/default/bauer/brueckenbauer.o bauer/brueckenbauer.cc
In file included from bauer/../boden/wege/weg.h:14,
                 from bauer/../boden/grund.h:18,
                 from bauer/../simplan.h:12,
                 from bauer/../simworld.h:32,
                 from bauer/../simwerkz.h:11,
                 from bauer/brueckenbauer.cc:11:
bauer/../boden/wege/../../besch/weg_besch.h: In member function 'waytype_t weg_besch_t::get_finance_waytype() const':
bauer/../boden/wege/../../besch/weg_besch.h:154: error: incomplete type 'weg_t' used in nested name specifier
make: *** [build/default/bauer/brueckenbauer.o] Error 1


I was using the 'standard' testgame - http://www.physik.tu-berlin.de/%7Eprissi/simutrans/yoshi87-2-102.sve

Interesting game. I will look on it. Probably following week.

Editing message doesn't mark it as 'unread' - makes it easy to miss the new info...

I have not expect so much interest in this thread

[Dwachs]

I would like to see these old COST_* constants gone (or at least hidden away in some cc-file). Currently there is a mixture used of the new constants and the old one. I updated script/api/api_player.cc to use the new ones. (see branch jk-finance in my github repository).

The routines to load old history can go to finance.cc, too.

[jk271]

I will work on suggested changes. When they will be done, I will make a new release of the patch.

[prissi]

Just to motivate you: I got again some time to work more on simutrans in the next two weeks. THus there is a good chance that as soon as you finished your patch I may go into the program.

[Dwachs]

Here is an updated patch file. Github branch at https://github.com/Dwachs/simutrans/tree/jk-finance

Still some work left: cleanup, documenting, remove separate powerline statistics in finance window as it is now in extra tab. I would like to wait until after next release before commit. But I do not want to wait too long 

[prissi]

Putting powerlines in a seperate tab will not show the total statistics at one glance, as the number would not add up.

But this is rather a theoretical statement. I cannot test it, could you please link to a diff, or please tell how to generate one? I have no idea how to use github to get a diff.

[Dwachs]

I attached a (zipped) patch file apply it with 'patch -p1 < patchfile'.

In the powerlines tab, there is also a profit row, which should tell whether there is any profit with powerlines.

[Dwachs]

update attached. Still some coding style stuff pending (mostly in simplay.*)

I removed the powerline income labels, as these numbers are shown on powerline tab. Also the chart shows now history of 24 months / years.

[Fabio]

What happens with old history?
It would be nice to have tabs with
1 period = 1 month (total 24 months)
1 period = 1 year (total 24 years)
1 period = 5 years (total 60 years)
1 period = 10 years (total 240 years)
It could be done with 4 tabs.
This would give a historical background to very long games.
Also a Burton for exporting financial data records to csv would be useful, especially for sharing results in the forum.

[jk271]

I have been very busy last month and I did not have any time neither to work on the patch nor to visit the forum. And I not not expect to have time for bigger changes this month too.

Now I see, that a lot of coding have been done: In finance.cc in trunk branch and also in jk-finances on Dwachs's github repository. Merging trunk into my branch and further to merge my branch into jk-finances would take time. So I suggest development team, mostly Dwachs, to overtake development of enhanced finance stats. Development goes much more faster with write access to repository than with permanent merging changes from trunk into my reporitory breaking code having work correctly and see nothing merged back to the trunk.

Thank you all, especially Fabio And Dwach, for yours support and suggestions.

[Dwachs]

Thank you for your patch and your continuous work on this.  Imho, we are near trunk inclusion of the patch, only some code beautifying is left

[jk271]

There is an opened question about "transportted" stats not being answered (and fixed) from my side:
What is the cause of difference between amount of transported cargo between trunk and amount of transported cargo with money patch code.

If the condition in function unload_freight() in file vehicle/simvehikel.cc on line 745 around function book_transported in moneypatch code is removed, "transported" stats appearing in "finance window" would be calculated in the same way as it is now in the trunk.

I hope it would help you to finish it successfully.

I do not end with simutrans. I take it as a long break. After several months I might be back with river patch.

[Dwachs]

What is the cause of difference between amount of transported cargo between trunk and amount of transported cargo with money patch code.

If the condition in function unload_freight() in file vehicle/simvehikel.cc on line 745 around function book_transported in moneypatch code is removed, "transported" stats appearing in "finance window" would be calculated in the same way as it is now in the trunk.

I did not have time to look into this up till now. The 'problem' here is, that in current trunk code both (a) the amount of transported goods and (b) the amount of goods delivered to their destinations are counted. But only the numbers in (a) are shown in trunk's finance window, while the patch does only count according to (b). Thus the difference.

Which of these statistics should be kept? What do you (all) think?

[jk271]

My suggestion is to display same numbers as are being displayed in finance findow from trunk now.
It would be a little bit easier to make a regression testing (using savegame with great tranport company from one of the post above) with (a).
So I vote for the option (a).
Which one would prefere others?

[prissi]

Actually it would make sense to keep both. Because (transport evnets a)/(delivered goods) is the mean number of interchanges. It may be very interesting to statistics freaks and gives also a metric on how interconnected everything is. For display thouhg, maybe a is enough. (But showing also interconnections?)

[TurfIt]

Both could work as described by prissi.

However in the context of the display in the finance window, I think it should show (b) - the amount delivered. I could largely care less about how much cargo is picked up, only how much is actually delivered to the final destination.

[Dwachs]

Here is the final version of the patch. Savegame version not increased yet, would be necessary when incorporated into trunk.

[Dwachs]

Countdown for comitting this is running. Here is the absolutely last final version of the patch. 

As to counting deliverd/transported: both numbers are recorded. The number of transported goods is shown in finance window (as it is in current trunk)

I really would like to get this off my desk

[prissi]

EDIT: Ok, forgot to add weg_besch.cc ...

Maybe one could no show unsued waytypes in the drop down? That would avoid the tt_Other in the finances selection.

Why is the maintenace section zero for the track/street/etc. even when building new stuff? I am a little confused, I expected there some meaningful number, at least for the ways and stops (could be gotten easily also by iterating over all ways in laden(), when savegame is without finances.)

[Dwachs]

Maybe one could no show unsued waytypes in the drop down? That would avoid the tt_Other in the finances selection.

tt_other is just the old finance history without the differentiation between the transport types, it will go away after some time.

Why is the maintenace section zero for the track/street/etc. even when building new stuff? I am a little confused, I expected there some meaningful number, at least for the ways and stops (could be gotten easily also by iterating over all ways in laden(), when savegame is without finances.)

Maintenance is booked at the beginning of a new month, you should wait until new month arrives.

[prissi]

I though all was the old window. Now I am confused: tt_other just displays a static window while "all" shows the previous behaviour (with the exception of "trips". Why does this start with zero after loading an old game? The old value is certainly recorded.)

Since anyway both values of trips are recorded, could the "interconnection" (mean transfers) calculated by those number also be displayed? Just as a simple value like margin or so.

[Dwachs]

I though all was the old window. Now I am confused: tt_other just displays a static window while "all" shows the previous behaviour

All: shows all values, everything summed up (including the old history)
tt_other: shows old values that could not be distinguished between transport type, but also maintenance for houses and cost of terraforming shows up there.

(with the exception of "trips". Why does this start with zero after loading an old game? The old value is certainly recorded.)

This was a bug in the patch, corrected.

Since anyway both values of trips are recorded, could the "interconnection" (mean transfers) calculated by those number also be displayed? Just as a simple value like margin or so.

I would like to finish this before adding more features.

[prissi]

Ok, soe we just need a clever translation for tt_other? Maybe "unknown" or "general"

[Dwachs]

Ok, soe we just need a clever translation for tt_other? Maybe "unknown" or "general"

Miscellaneous / Sonstiges

[Dwachs]

This is now incorporated in trunk, r6424 Finally. Many thanks to jk271!

[hreintke]

Dwachs,

Trying to update my version to the latest trunk I get the message :

1>bruecke.obj : error LNK2001: unresolved external symbol "public: enum waytype_t __thiscall weg_besch_t::get_finance_waytype(void)const " (?get_finance_waytype@weg_besch_t@@QBE?AW4waytype_t@@XZ)

I checked weg_besch.h and it includes

Code Select Expand


* @return waytype used in finance stats (needed to distinguish \
* between train track and tram track
*/
waytype_t get_finance_waytype() const;


But there is no weg_besch.cc, did you forget to include the weg_besch.cc into the svn ?

Herman

[Dwachs]

the file was missing, thanks for the hint.