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

[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.