News:

Simutrans Chat Room
Where cool people of Simutrans can meet up.

Multi Tiles Inner City Building for Standard

Started by THLeaderH, January 26, 2018, 11:32:03 AM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

THLeaderH

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
max_level_leap_per_a_tile = 6
in 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

prissi

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.

THLeaderH

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.

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

Leartin

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?

prissi

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.

THLeaderH

Quote from: prissi on January 27, 2018, 03:29:07 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?

Ters

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.

Isaac Eiland-Hall

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!)

Dwachs

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 ;)
Parsley, sage, rosemary, and maggikraut.

THLeaderH

QuoteThis already contains the change in the algorithm you mentioned?
Yes. The algorithm was already modified.

QuotePlease 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?

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

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

Dwachs

Quote from: THLeaderH on January 29, 2018, 08:45:05 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

if (a / b < 0.1) { .. }

by

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.
Parsley, sage, rosemary, and maggikraut.

prissi

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.

Dwachs

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.
Parsley, sage, rosemary, and maggikraut.

THLeaderH

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

prissi

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.

Dwachs

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)

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.
Parsley, sage, rosemary, and maggikraut.

Dwachs

@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.
Parsley, sage, rosemary, and maggikraut.

THLeaderH

Quote from: Dwachs 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.
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).)

Dwachs

@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?
Parsley, sage, rosemary, and maggikraut.

prissi

Sorry, now I am confused. In what way are you referring to handle them similarly? Tile wise levels?

THLeaderH

Quote from: Dwachs 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?
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()?

Dwachs

Quote from: THLeaderH on February 02, 2018, 01:43:26 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.
Parsley, sage, rosemary, and maggikraut.

Dwachs

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.
Parsley, sage, rosemary, and maggikraut.

THLeaderH

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!

Dwachs

Take your time. I have not time at the moment to work on this either.
Parsley, sage, rosemary, and maggikraut.

Dwachs

Quote from: THLeaderH on February 07, 2018, 02:57:03 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).
Parsley, sage, rosemary, and maggikraut.

prissi

I finally had a look at this, but in the end it was a complete rewrite. This is probably more "simutransish". It carries the same limitation (i.e. building only grow in size during renovation), but allows also for the initial construction of larger than 1x1 buildings. I.e. there could be a pack of entirely 1x2 and 2x1 buildings as the smallest size. (Somewhere deep in the forum there were small residental 2x1 buildings in the screenshot contest.)

The maximum size is 3x3, but the cityrules of many paks never allow for such buildings to be constructed (like pak128.japan for instance), because they only construct a single row near the roads.

Leartin

Quote from: prissi on March 25, 2019, 01:54:37 PM
[...]allows also for the initial construction of larger than 1x1 buildings. [...] The maximum size is 3x3, but the cityrules of many paks never allow for such buildings to be constructed (like pak128.japan for instance), because they only construct a single row near the roads.

Makes me wonder how it works. If buildings larger than 1x1 can spawn initially, it would make sense that larger buildings should be able to spawn on tiles which are only partly occupied by 1x1s, right?
Do all empty tiles need to be eligable according to cityrules, and if so, do their chances matter? Or is it enough that there is a chance at all?

Consider this:
### center of 3x3 house
##
# . . . . .
# . h h h .
# . h n h .
# . h h h .
# . . . . .
#
house_x = ..... .hhh. .hnh. .hhh. .....
house_x.chance = -99


The center would technically be eligable, but unlikely to spawn a 1x1 building. Would a 3x3 be equally unlikely, using the same chance?
(Personally, I would like to have a 3x3 or smaller buildings that cover the middle of a 3x3, but I don't like the idea of a nice 3x3 terraced houses block with some foreign element in the middle. Perhaps a rule to enforce whether citybuildings need a road on one of the 8 surrounding tiles - second-row-buildings could be prevented by that, while multi-tile-buildings would be allowed to use them.)

Is there a difference between 1x2 and 2x1? Do they rotate properly (one has the short side towards the road, one has the long side towards the road)?

prissi

First the current cityrules are used as starting point. From this point all surrounding points are tested that those are either nature as similar slope height, or 1x1 city buildings. Upon constructions, the 1x1 buildings will be replaced by the new building. For that reason  2x2 or 3x3 building rarely spawn at streets, because the center tile is at a street. 2x1 resp. 1x2 more often, though. For renovation, the logic is slighly different: Allowed are only 1x1 buildings, all tiles of the current building.

2x1 and 1x2 have in principle equal chances, but if both fit, the first is taken. (Same for 2x3 resp. 3x2.)

And so far building can only grow in area. (Allowing the reverse is almost trivial, but as long as smaller buildings are the majoity, it would make large area buildings even more rare.)

Leartin

Okay, so you start out with one tile (which would otherwise be the place a 1x1 would spawn). You only test neighbouring tiles (causing the limitation to 3x3), but you don't test them for accordance to cityrules, only whether they are empty/buildings and on the correct height-layer.
That would mean while spawning, the center of a 3x3 'block' can be filled by any 1x2, 2x2 or 2x3 initially. However, if it is not filled at spawning, it will never fill through renovations, because renovations ignore empty tiles. But if it's filled, that center tile can be the one due for renovation, in which case it may combine all the surrounding tiles to a 3x3. Is that correct?
Are there technical reasons why empty spaces are not considered on renovation, or was it just never thought of?

Quote from: prissi on March 26, 2019, 01:32:58 PM2x1 and 1x2 have in principle equal chances, but if both fit, the first is taken. (Same for 2x3 resp. 3x2.)
So they do follow rotations and are not the same - if we draw driveways, they will lead to the road even on multitile buildings.
A 2x1 building would be one with the short side towards the road.

Are citybuildings themself restricted to 3x3, or is it only the spawning code that makes larger ones impossible to spawn, and therefore useless?
Say someone creates an L-shaped citybuilding (or anything non-rectangular) - could this system cope with them?

prissi

Your summary is correct. And the only reason was code reusing. Renovations could easily add new buildings. And the reason to stop at 3x3 is time. City growth is one of the most time consuming tasks when generation maps, and I would like to avoid slowing down much more. (Hence the code tests for the largest building in the pak and then only test the needed areas)

L shaped building will crash with the current code, since all tiles will be needed to be replaced by other tiles.

One could certainly reuse the old code, just with larger building and space constraits, to use all 8 rotations. But what if roads are on both sides? My take would be that this rotation wins. Also if a 2x1 building only fits in wrong rotation, should it be discarded?

Leartin

Thank you for taking the time to explain things.

Quote from: prissi on March 27, 2019, 12:11:49 PMOne could certainly reuse the old code, just with larger building and space constraits, to use all 8 rotations.
Yes please :)
Quote from: prissi on March 27, 2019, 12:11:49 PMBut what if roads are on both sides?
Well, I'm still not quite happy with how 1x1 are handled in some cases (see: https://forum.simutrans.com/index.php/topic,16927) but I don't see a particular need for multi-tile buildings. Perhaps one could ask on which side there is more road - if a 2x2 has two pieces of road to the south, but only one each to the north and east. But that's stretching it.

Quote from: prissi on March 27, 2019, 12:11:49 PMAlso if a 2x1 building only fits in wrong rotation, should it be discarded?
I would think so, yes. I'll use existing graphics to illustrate my point: Say you create a 1x2 building like this:

Then you don't want it to be placed like this:

If a 2x1 or 1x2 was created that really wants to be in a specific relation to a road, that should be possible.


But there is more to this: I think especially in the beginning, many buildings won't be new creations, but rather combinations. For example, this building:

can easily be reused in combination with a new tile as the front yard - I use a park as example to create a 2x1:

But it could also be used in combination with a different front yard that bends around to form a 1x2:

So really, making sure a 1x2 is a 1x2 and a 2x1 is a 2x1 feels like the right thing to do, and whoever does not want to destinguish can simply create both with the same graphics.

Now, to be fair: None of the shown buildings actually have rotations. This is mostly due to rotations being very work intensive in a pixelled pakset, compared to those that render from 3D objects. But it also goes to show that not having perfect road alignment wouldn't be the worst that could happen, as there are enough examples where it already happens, despite the technical capability to avoid it.


prissi

Incorporated in r8749 (although in the end a totally different patch from scratch, to better obey rotations etc.) I my testing I go occasional 2x3 as largest size but 3x3 almost never appears. 2x1/1x2 are very frequent, and 2x2 less than 10%.

Import for this patch is that larger houses will not be broken into smaller ones. Hence one should have at least every 6 levels (e.g. level 6, 11, 15, 20, 25) for a house of that type in that size or the city will grow quite large, since a lot of places are blocked for further renovation.

Furthermore, the rotation are determined at build time, but the city grow on and often the rotation does not match at later stage. So I think 8 rotations for 2x2 buildings do not make sense.

Leartin

Quote from: prissi on April 23, 2019, 03:31:15 AM
Furthermore, the rotation are determined at build time, but the city grow on and often the rotation does not match at later stage. So I think 8 rotations for 2x2 buildings do not make sense.

I'd assume that rotation is also determined anew if upon renovation, a larger building than before is created (since otherwise it hardly makes sense anyway)
If so, one could circumvent this issue simply by not spawning larger buildings initially, but only at slightly higher levels, at which point all roads are already buildt. The only thing speaking against this would be that on renovation, no empty tiles can be included.
However, there is a different problem: 8 rotations wouldn't work for non-square buildings. For each of the four main rotations, the corner can either be to the right or to the left, that would already be 12 rotations. If you had only the left or only the right corners, as 8 rotations imply, people like me will complain anyway ;) so perhaps 4 rotations are better than 8.


I'd like to point out, though, that there is a rather different option: Seperate tile-rotation and building rotation.
Say you have a 1x2 facing the road with the long side (to the south, rotation 0). The left/west tile could have it's own rotations: Rotation 0 if there is only a road to the south, rotation 7 if there is also a road to the west, making it a corner. The right/east tile could have rotation 0 if there is only a road to the south, or rotation 4 if there is also a road to the east. So not only could there be a corner on either side, but also a corner at both sides. (and, using other rotations, there can be different graphics if there is a road in the back etc. - but it's not feasable to create seperate graphics just for that)
This would allow for seamless combination of multitile-buildings and 8-rotational rowhouses, as all tiles of the multitile-building would obey the very same rotation rules.

You could have a 2x2 building with no rotation of it's own, but it's tiles use it's own rotations to "connect" to the sidewalk of the road or place fences on all sides without roads, since there would be neighbouring buildings. You can have a door on each tile that faces a road, but not on tiles that don't face roads - something like that. All the things where it's not a matter of actual building rotation (which is front yard with the nice facade, which is the back yard with the smoker's corner...) but a matter of which tile specifically connects to a road.

prissi

8 rotations work out for 2x1/2x1 buildings, which are the only one that will appear frequently. 2x2 I would only use at very high levels, like occasional big parks, large skyscraper etc.

And even with renovation realign the rotation, it tend to not match due to later roads on the backside etc. I have added a debug routine. If you use a debug build, then the routine tells you the current rotation, and the "best" rotation on the current road loayout. Those mismatch in later stages in about 60% for 2x2 buildings ... For smaller building this is less of an issue, since their "roadfront" is shorter.

And no, I will not rewrite the building routines from scratch to include individual tile rotations. It was hard enough to get the layout looking right to a human.