The International Simutrans Forum

 

Author Topic: Multi Tiles Inner City Building for Standard  (Read 4162 times)

0 Members and 1 Guest are viewing this topic.

Offline THLeaderH jp

  • Coder/patcher
  • Devotee
  • *
  • Posts: 271
  • Languages: JP,EN
Multi Tiles Inner City Building for Standard
« on: January 26, 2018, 11:32:03 AM »
This is a transplantation of multi-tiles inner city building for extended. The patch, that is attached to this post, is r8374 based.

What does this patch do?
Pak authors now can designate the dimension of city buildings other than 1x1.
This patch enables players to enjoy a more realistic city view in simutrans.

Side Effects
No one can recognize any behavioral changes when there are only single-tile city buildings in the pak set.
The amount of calculation slightly increases compared to the current simutrans. The difference might be perceived when there is an extremely large city building in the pak set. (e.g. 32x32 tiles)

Parameters
In the dat file of city buildings, the level of a building is defined as the level of the whole building. So, a single-tile building whose level is 10 and a 2x3 tile building whose level is 60 should have approximately same height, because "level per a tile" of the both buildings are 10. This rule is same as that of city monuments.
In simuconf.tab, a parameter max_level_leap_per_a_tile was added. To understand what this parameter is, you have to know what happens in the city building renovation.
In the renovation, first, we choose the building to renovate. Then the size of the new building is determined. After that, we search city buildings of the size from the pak set. In the end, we replace the old building with the chosen building.
Let's consider renovating a small single-tile house. If 3x3 is selected as the size of the new building, and all 3x3 tiles buildings in the pak set are skyscrapers, a skyscraper suddenly appears from a peaceful residential area. To prevent this, maximum leap of level per a tile have to be defined and this is max_level_leap_per_a_tile parameter. The default value is 10, since in general level of city buildings varies from 0 to 50 in standard. To modify this parameter, please write
Code: [Select]
max_level_leap_per_a_tile = 6in simuconf.tab. I recommend that you write this parameter in the simuconf.tab of a pak set, since the adequate value depends on the variety of city buildings in the pak set. This parameter is not stored in the save data because the variety of city buildings can be changed when players add a new city building to the pak set.

Limitations
Multi-tile buildings can be constructed only in the renovation. Buildings that are newly constructed must be 1x1, and of course the construction obeys cityrules.tab.
Only layout=0 and layout=1 are available for multi-tile city buildings. layout=2 or more are not chosen when the renovation happens. For single-tile buildings, the layout determination algorithm is still available.


I'm really excited to see a whole new city scene in simutrans with multi-tile city buildings ;D

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9313
  • Languages: De,EN,JP
Re: Multi Tiles Inner City Building for Standard
« Reply #1 on: January 26, 2018, 02:13:34 PM »
One question, why choosing size first? That seems to promote multitile buildings over single tile ones. Would it not be more fair to sort multitile buildings within the normal buildings at that level (single tile) and then start the replacement once we picked a multitile building and try to match it with the current building. That would avoid also the sudden appearance of inadequate buildings.

Also there is a maximum leap for normal buildings, which is at 3 (or so, was needed for the OpenTTD pak, since this had exactly 11 houses only.)

And any parameter influencing town growth must be saved with the game to avoid desyncs (no discussion of course) but imho should be part of the city rules, not simuconf.tab.

And how to grow a multitile building? Can it be broken up in multitile buildings again? Otherwise any city will end up with multitile buildings mostly.

Offline THLeaderH jp

  • Coder/patcher
  • Devotee
  • *
  • Posts: 271
  • Languages: JP,EN
Re: Multi Tiles Inner City Building for Standard
« Reply #2 on: January 27, 2018, 11:24:14 AM »
Thank you for the reply and indication. As you put it, choosing buildings by the level per a tile is appropriate. So I changed the algorithm and it works. Thus, parameter max_level_leap_per_a_tile is no longer used.

Quote
And how to grow a multitile building? Can it be broken up in multitile buildings again? Otherwise any city will end up with multitile buildings mostly.
The current algorithm allows to gather buildings into one multi-tile building, but does not allow to split one multi-tile building into multiple smaller buildings. Because of this, most cities will end up with multi-tile buildings mostly. So I'm thinking about splitting a multi-tile building in the renovation.

Offline Leartin

  • Devotee
  • *
  • Posts: 1088
  • PAK-DEV P192C
  • Languages: DE, EN
Re: Multi Tiles Inner City Building for Standard
« Reply #3 on: January 27, 2018, 02:42:37 PM »
I don't think that was mentioned yet: Do you need to cover the full multitile-building with a station or is it enough to have one tile covered, or is that a setting you can choose yourself?

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9313
  • Languages: De,EN,JP
Re: Multi Tiles Inner City Building for Standard
« Reply #4 on: January 27, 2018, 03:29:07 PM »
The algorithm will work regardless of how many tiles, as it is the same as for a multitile attarction. Only tiles covered will contribute. Imagine multiple entries-exit of a building.

And decomposition into multiple buildings would be easy, one would just need a "no multitile flag" for the renovation routine, and then call it for each tile. If it fails completely for a tile, then just take previous level.

Offline THLeaderH jp

  • Coder/patcher
  • Devotee
  • *
  • Posts: 271
  • Languages: JP,EN
Re: Multi Tiles Inner City Building for Standard
« Reply #5 on: January 27, 2018, 03:53:00 PM »
And decomposition into multiple buildings would be easy, one would just need a "no multitile flag" for the renovation routine, and then call it for each tile. If it fails completely for a tile, then just take previous level.
I'm afraid of not understanding correctly what this means. Could you explain it in more detail?

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 5374
  • Languages: EN, NO
Re: Multi Tiles Inner City Building for Standard
« Reply #6 on: January 27, 2018, 07:53:50 PM »
I'm not worried about cities getting completely covered in multi-tile buildings. The street construction in Simutrans is so chaotic that there will always be tiles that don't fit a multi-tile layout. The only real-life "multi-tile" building I know of having been split up have been what would most likely have been factories in Simutrans. I know of cases where individual parts have been upgraded and where further "tiles" have been added over time, individually and many at the same time.

Offline Isaac.Eiland-Hall us

  • Benevolent Dictator
  • Administrator
  • *
  • Posts: 3594
  • PanamaCityPC.com/support/
    • Facebook Profile
  • Languages: EN
Re: Multi Tiles Inner City Building for Standard
« Reply #7 on: January 28, 2018, 05:06:09 AM »
The mention of a 32x32 tile building makes me hope one is made, JUST because I'd manually place it and then giggle when it got upgraded and broken apart into a 32x32 tile mess. LOL.

(but this idea otherwise sounds pretty awesome, so I hope it works out!)

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4494
  • Languages: EN, DE, AT
Re: Multi Tiles Inner City Building for Standard
« Reply #8 on: January 28, 2018, 07:27:41 PM »
Many thanks for porting this to standard. I am reading through the diff at

https://github.com/aburch/simutrans/compare/master...teamhimeh:std-MTICB

This already contains the change in the algorithm you mentioned? I.e. first check sizes, then choose building.

I could not run tests.

Some random small thoughts:
- At some places you can use house_decs_t::get_x(rotation) and get_y(rotation) to simplify code.
- Why do you need to call check_tiles_height? The house builder sometimes fails to build on uneven terrain? Heights are checked in get_available_building_size, so some corner case is not caught there...
- The value -100 is used for 'unknown height', which can appear in maps. You could use sint16 and initialize with an unreachable value (like -200).
- Please do not use 'double' type for any computations. It is not safe to use for network games, as the computations are not guaranteed to be consistent.
- To calculate replaced_level_per_a_tile in hausbauer_t::get_city_building_from_list tiles are visited more than once. Calculations can be restructured for performance.
- The dimensions of the new building are not forced to larger than the dimension of the old?

Please don't get me wrong: These 'issues' are very minor. They can be all addressed. Even after the patch is committed ;)

Offline THLeaderH jp

  • Coder/patcher
  • Devotee
  • *
  • Posts: 271
  • Languages: JP,EN
Re: Multi Tiles Inner City Building for Standard
« Reply #9 on: January 29, 2018, 08:45:05 AM »
Quote
This already contains the change in the algorithm you mentioned?
Yes. The algorithm was already modified.

Quote
Please do not use 'double' type for any computations. It is not safe to use for network games, as the computations are not guaranteed to be consistent.
Looking simtypes.h, I cannot find the type definition of a decimal number, like sint16. What should I use instead of type double?

Quote
Why do you need to call check_tiles_height? The house builder sometimes fails to build on uneven terrain? Heights are checked in get_available_building_size, so some corner case is not caught there...
Heights are checked in get_available_building_size. However, I often observed a phenomenon that the height became different in a one building without calling check_tiles_height. I don't know exactly why this happens, but I assume this is because in get_available_building_size, the height of a tile is estimated on the assumption that other tiles are still building tiles. Anyway, I check height of tiles after the construction to prevent that the height becoming different in one multi-tile building.

Quote
The dimensions of the new building are not forced to larger than the dimension of the old?
In the current code, the dimensions of the new building are forced to be equal or larger than the dimension of the old. I know this causes the problem that cities end up with multi-tile buildings mostly. So I'm thinking about splitting one multi-tile building into multiple single-tile buildings.

Thank you for your review. It's so helpful for me :) I'm also working on the other issues that Dwachs listed up.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4494
  • Languages: EN, DE, AT
Re: Multi Tiles Inner City Building for Standard
« Reply #10 on: January 29, 2018, 10:59:36 AM »
Looking simtypes.h, I cannot find the type definition of a decimal number, like sint16. What should I use instead of type double?
We have to rely on integer arithmetic. It is not so difficult as it sounds. One has to replace stuff like
Code: [Select]
if (a / b < 0.1) { .. }
by
Code: [Select]
if( 10*a < b) { .. }
Quote
In the current code, the dimensions of the new building are forced to be equal or larger than the dimension of the old. I know this causes the problem that cities end up with multi-tile buildings mostly.
Could you point me to the places in your patch, where this is forced? I did not find it. I will definitly test your patch later this week.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9313
  • Languages: De,EN,JP
Re: Multi Tiles Inner City Building for Standard
« Reply #11 on: January 29, 2018, 01:20:22 PM »
If you remove a foundation, it will try to restore the previous terrain. However, you are just replacing the tile with a new tile"gebaeudet::set_tile", hence one should not need the full removal. One just need to take care that the bookkeeping is ok afterwards.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4494
  • Languages: EN, DE, AT
Re: Multi Tiles Inner City Building for Standard
« Reply #12 on: January 29, 2018, 08:25:40 PM »
here is a small fix to get_available_building_size: the comparison was not working as intended, and also test coordinate was wrong.

The function seems to loop a lot over already visited tiles.

More comments:
-- the calculations of orthogonal_neighbors does not take rotation of the building into account
-- removal of multi-tile buildings is very expensive. Seems removal cost per tile has to be divided by area.

Offline THLeaderH jp

  • Coder/patcher
  • Devotee
  • *
  • Posts: 271
  • Languages: JP,EN
Re: Multi Tiles Inner City Building for Standard
« Reply #13 on: January 31, 2018, 01:25:18 AM »
Thank you for your help. Dwachs' patch was already applied and the commits are all pushed to the github repository. I'll post a patch file again on this forum when various tasks are done, but I'd be glad if you review my code that is in progress. It boosts the development speed. Diff can be seen from the URL below with my commit messages.
https://github.com/aburch/simutrans/compare/master...teamhimeh:std-MTICB

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9313
  • Languages: De,EN,JP
Re: Multi Tiles Inner City Building for Standard
« Reply #14 on: January 31, 2018, 06:11:07 AM »
What I am not too happy about is that the replacement level calculation is done by the haubauer_t. That should be3 rather part of the city, I think. I would just match the level of the tile that is replaced and then replace all buildings accordingly, not matter of the inabitants (and job- and homeless) increase or decrease. I.e. just the exiisting logic (return a random weighted building based on its single tile value) with an additional size parameter, which gives the maximum possible area around this to be replaced house.

1) find house to replace.
2) find largest rectangle (probably limit at the largest size in the set) around it without cutting into another multitile buildings and not a road and at same height.
3) call as now hausbauer_t::get_industrial etc. with that size.
4) put the building there (if there is more than one fit, one could try to change buildings, so that the all the other citicens levels are least disturbed; but I think it is not really needed).

I think that could avoid a lot of extra checks.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4494
  • Languages: EN, DE, AT
Re: Multi Tiles Inner City Building for Standard
« Reply #15 on: January 31, 2018, 07:33:44 AM »
Part of this is the confusion about the level of a multi-tile city building: Is a building of size W x H with level L equivalent to a one-tile building of level W*H*L or a one-tile building of level L? The routine stadt_t::add_gebaeude_to_stadt suggests W*H*L, but I think I have seen L used somewhere else as well.

The level of the needed replacement also depends on the level of all buildings to be replaced. If in the following situation (number corresponds to one-tile buildings with level)
Code: [Select]
18
88
the '1' is replaced by a 2x2 building of level 2, then at the end less passengers are generated then before. Maybe one can limit the search such that only buildings of level <= level-of-building-to-be-upgraded+1 are affected.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4494
  • Languages: EN, DE, AT
Re: Multi Tiles Inner City Building for Standard
« Reply #16 on: January 31, 2018, 07:36:43 AM »
@THLeaderH: can you please point me to the source code lines that prevents upgrading a multi-tile building by a building of smaller dimension? It never happened with the test pak's you provided. But I suspect this is only a lucky coincidence due to the distribution of levels.

Offline THLeaderH jp

  • Coder/patcher
  • Devotee
  • *
  • Posts: 271
  • Languages: JP,EN
Re: Multi Tiles Inner City Building for Standard
« Reply #17 on: January 31, 2018, 07:38:58 AM »
@THLeaderH: can you please point me to the source code lines that prevents upgrading a multi-tile building by a building of smaller dimension? It never happened with the test pak's you provided. But I suspect this is only a lucky coincidence due to the distribution of levels.
https://github.com/teamhimeh/simutrans/blob/ex-MTICB/simcity.cc#L4177-L4178
Sorry for being late.
(I know these lines can be simplified with get_x(layout) and get_y(layout).)

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4494
  • Languages: EN, DE, AT
Re: Multi Tiles Inner City Building for Standard
« Reply #18 on: February 02, 2018, 07:09:34 AM »
@THLeaderH: thanks for all your updates :)

I still would like to discuss, how level of multi-tile buildings is taken into accout. Why should multi-tile inner-city buildings be handled differently than attraction buildings? I think it could simplify the code, if both classes of buildings would be handled identically. The calculations with area could be omitted. Also the code would be more robust against broken buildings with missing tiles. What do you think?

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9313
  • Languages: De,EN,JP
Re: Multi Tiles Inner City Building for Standard
« Reply #19 on: February 02, 2018, 12:49:26 PM »
Sorry, now I am confused. In what way are you referring to handle them similarly? Tile wise levels?

Offline THLeaderH jp

  • Coder/patcher
  • Devotee
  • *
  • Posts: 271
  • Languages: JP,EN
Re: Multi Tiles Inner City Building for Standard
« Reply #20 on: February 02, 2018, 01:43:26 PM »
@THLeaderH: thanks for all your updates :)

I still would like to discuss, how level of multi-tile buildings is taken into accout. Why should multi-tile inner-city buildings be handled differently than attraction buildings? I think it could simplify the code, if both classes of buildings would be handled identically. The calculations with area could be omitted. Also the code would be more robust against broken buildings with missing tiles. What do you think?
I defined the level of a multi-tile city building as the level of whole building because city monuments take this approach. As far as I know, city monuments are not restricted to 1x1, and the level is defined as that of whole monument.

By the way, stadt_t::cityrules_rdwr() seems not to be called during the load and save in the game. I introduced a parameter split_building_percentage and this has to be stored in the save data. Should I write this in stadt_t::cityrules_rdwr()?

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4494
  • Languages: EN, DE, AT
Re: Multi Tiles Inner City Building for Standard
« Reply #21 on: February 02, 2018, 04:51:09 PM »
By the way, stadt_t::cityrules_rdwr() seems not to be called during the load and save in the game. I introduced a parameter split_building_percentage and this has to be stored in the save data. Should I write this in stadt_t::cityrules_rdwr()?
It is in simworld.cc, around line 4656, and it is only called for network games.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4494
  • Languages: EN, DE, AT
Re: Multi Tiles Inner City Building for Standard
« Reply #22 on: February 02, 2018, 05:11:34 PM »
Sorry, let me explain: My interpretation of the code in current simutrans standard trunk, is that for multi-tile attraction buildings *each* tile counts as building with level given by desc->get_level(). To follow the code arround:
  •   hausbauer_t::build calls welt->add_attraction( gb )  for each tile
  •   the attraction list in karte_t is used in simcity.cc to send passengers to attractions
  • see the routines in simcity.cc,  each tile counts: stadt_t::add_target_attraction, stadt_t::find_destination
There is also the advice to 'cover each tile of an attraction', which reflects that each tile counts. It took me a while to find these connections ;)

Now, in the current state of the patch, multi-tile buildings are treated differently: each tile counts as building with level equal to desc->get_level() / area . Hence why I am asking to change the patch, such that for multi-tile buildings each tile is considered as of level desc->get_level().

Sorry for the confusion, I was confused myself. The city's bookkeeping of bev, arb, won, etc is not something I have deep knowledge about.

Offline THLeaderH jp

  • Coder/patcher
  • Devotee
  • *
  • Posts: 271
  • Languages: JP,EN
Re: Multi Tiles Inner City Building for Standard
« Reply #23 on: February 07, 2018, 02:57:03 PM »
Sorry for not updating for 5 days. I'm currently busy outside the simutrans universe and this seems to continue for a week at least. I'd be glad if you continue the discussion and the improvement of the code.
The major concerns are as follows:
  • The discussion about the definition of the level of a building. I agree with the opinion that the level should be defined as the level of whole building, but this results in a loss of the consistency of code because the level seems to be defined as the level per a tile for city monuments. City monuments are not limited to 1x1 in size.
  • As prissi put it, the replacement level calculation should be a part of a city. I wrote this part in hausbauer_t because it needs the city building list that is held by hausbauer_t. Though this is not behavioral but an aesthetic problem, the code structure should be improved.
  • I haven't brought a lot of improvements to the extended that were done in this thread. The improvements have to be brought to the extended as soon as possible since this patch is already a part of the master branch of simutrans extended!

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4494
  • Languages: EN, DE, AT
Re: Multi Tiles Inner City Building for Standard
« Reply #24 on: February 08, 2018, 07:15:49 AM »
Take your time. I have not time at the moment to work on this either.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4494
  • Languages: EN, DE, AT
Re: Multi Tiles Inner City Building for Standard
« Reply #25 on: February 23, 2018, 03:21:28 PM »
  • The discussion about the definition of the level of a building. I agree with the opinion that the level should be defined as the level of whole building, but this results in a loss of the consistency of code because the level seems to be defined as the level per a tile for city monuments. City monuments are not limited to 1x1 in size.
This is not true: in stadt_t::add_gebauede_to_stadt *all* tiles of a multi-tile building are added (not just one, as the function signature suggests).