News:

Congratulations!
 You've won the News Item Lottery! Your prize? Reading this news item! :)

Improvement in terraced building construction -- clusters (pretty-cities)

Started by neroden, May 02, 2013, 12:43:15 AM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

neroden

It was irritating me that terraces were not constructing the way we wanted them to look (long rows of identical houses).

So I programmed what I think is an elegant solution.  Tested, compiled, runs, works.

It's on my pretty-cities branch in git.  A test run for modifications needed to the pakfiles is on the pretty-cities branch of the pak128.britain git repository.

There is a new line available in the .dat files for city buildings (and only city buildings, and only residential/commercial/industrial).
This is "clusters".  There are 32 possible clusters (numbered 1 through 32) available for a pakset.  Buildings do not have to be part of any cluster, and are not part of any cluster by default.  A building may be part of two or more different clusters.  (It's a bitfield.  I had to use a bitfield for internal reasons, so I figured I might as well allow pakset makers to use it.)

I would suggest putting rowhouses and other buildings which fill from side-property-line to side-property-line into clusters.  I would put everything with the same setback from the front-property-line in a single cluster.  I would leave detached houses without clusters.

For example, I put the grey stone terraces (both houses and shops) into cluster #4.
clusters=4

I put the wall-to-wall 1750s warehouses into cluster #6.
clusters=6

If I wanted a building to cluster with both of those, I would write
clusters=4,6

If a building is a part of a cluster, then when the city growth routine tries to build a building next to that building, it will preferentially (but not always) build one of the buildings of the correct cluster type.  This means more grey terraces next to existing grey terraces.  This does not override the level checks, so if there are no grey terraces in the next level of buildings, down come the terraces.

(The way this works, specifically, is that the four orthogonally adjacent tiles are checked.  A building which matches any one of the clusters for any of the buildings on any of those 4 adjacent tiles will get a bonus to its chance of being built -- currently a factor of 10.  A building which matches none of the clusters will get a penalty -- currently also a factor of 10.  If something is being built with a grey terrace on one side and a brownstone on the other side, brownstones and grey terraces will both get the bonus.)

(Further technical note: I used extra_data, which isn't used yet for res/ind/com buildings, so I didn't have to alter the data structures.  Save game compatibility is preserved, and you can even use old compiled paksets with the new executable.)

A building which is not part of any cluster will continue to follow the existing rule -- it will not be placed next to an identical building (to "avoid boring cities" as the code says).  However, a building which is part of any cluster will ignore this rule (obviously, as the clustering rules wouldn't work otherwise).  I am open to the question of whether this rule should be removed entirely in favor of a truly random building distribution.

On the whole I like the results.  I get blocks of matching rowhouses, while the blocks of detached houses remain a mix of different buildings.  See the following screenshot for something which you could not have generated randomly with the prior code.

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

(This code could probably go into standard, since I think experimental has barely touched any of this code.  It's proven to be extremely painful and difficult to get patches into standard, however.  If someone else is willing to extract the diff between my branch and the existing experimental branch as a patch for standard, it would be appreciated.  Be warned: while I was fixing and deciphering the code, I translated great hunks of it from German to English, which is what is supposed to be done.

If the developers for standard want this kind of nice little uninvasive improvement to go into standard... they should switch to using git, and review requested patches promptly.)

I have a couple of other "pretty city" projects in mind, to fix the last couple of annoyances:
(1) Buildings built next to the beginning of a bridge frequently face the wrong way.  I haven't figured out why the start of road bridges isn't triggering the street detection algorithm correctly, or how to fix that.
(2) For buildings in small blocks (2x2) it is somewhat random which direction they face, so that even though I can get four rowhouses, they may all be facing in four different directions.  I have some ideas about how to fix this but I think I want to fix the bridge issue first.
(3) Factories aren't facing the right direction (they're frequently facing away from the road)  This is mostly an issue with "final destination" factories like the Bakery and so forth.
(4) When a building has a choice of facing two roads, one of which is flat, and the other of which is on a slope, it should prefer to face the flat one.  It often doesn't.
(5) I'd like to improve my clustering algorithm, so that it only looks "along the road" for buildings to cluster with, not "across the back lot".  I think this needs to be done after fixing (1), (2), and (4), as it's going to require the same sort of information.

But anyway, I think this is an improvement even as it is.  And if you don't like it, well, you don't have to put "clusters" in your dat files.  :-)

jamespetts

Excellent, thank you very much for this. This does look very interesting, and I know that the developers of Pak128.Britain (especially The Hood) would find this interesting, too. I don't have time to test it now, but shall look to do so with interest when I do.

Incidentally, from what you have written, I do not think that this will interfere with the plans that I have (after the next proper release of code and pakset) to make growth and upgrading depend on local transport using the passenger success rates already incorporated into buildings but not yet used in the game; am I correct?
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

neroden

Quote from: jamespetts on May 02, 2013, 09:45:48 AM
Incidentally, from what you have written, I do not think that this will interfere with the plans that I have (after the next proper release of code and pakset) to make growth and upgrading depend on local transport using the passenger success rates already incorporated into buildings but not yet used in the game; am I correct?

You are correct!  The way this works is, *after* there is a call for a building of a certain level to build or upgrade on a certain location, this provides a selection bias in the choice of building for the new upgrade.

What you plan to do will affect where the building will be built or what level will be requested, which all happens outside this code.  Modularity for the win!

jamespetts

Thank you - that is most helpful. Hooray for modularity indeed!

Apologies that I have not yet had time to get around to replying to the more in-depth discussions: have been busy with work and am currenly on a train; will reply when I get a chance. Thank you very much for all  your work and careful considration, however.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

kierongreen

The reason things take a while to get into standard is because patches are checked to make sure that the code and features are consistent with existing game. This patch does look like something that could be incorporated into standard and is useful in my opinion, there might need to be a bit of discussion about the exact format of dats though for example. Just because git is easier for you doesn't make it easier for everyone. I find it incredibly awkward and time consuming to merge changes using git and easier with svn.

prissi

This would be a nice additions for paks which have clusterable houses (pak96 and pak64.geman too). Personally I would like to incoporate this in standard too. Even more since it is only eyecandy and does not influence gameplay much (ok. slightly different levels may result).

Since those rowhoses have an orientation (or rather when they have rotations) then from those the directions to ceck for neightbours could be derived immediately. That could partly solve the "looking in the backyard) problem.

jamespetts

Would you be able to upload your changes to the .dat files as well for this?

Edit: I have incorporated this into the code, but I should still like to see how you have modified your .dat files so that I can implement this into the pakset, too.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

neroden

Quote from: jamespetts on May 03, 2013, 11:41:47 PM
Would you be able to upload your changes to the .dat files as well for this?

Edit: I have incorporated this into the code, but I should still like to see how you have modified your .dat files so that I can implement this into the pakset, too.

Real life interfered with my being able to respond to this promptly.  The pakset changes are on the "pretty-cities" branch.  You can diff that against my "new-experimental-master" branch to see what I did.  I didn't finish going through all the city buildings in the pakset by any means.  I started with the earliest year of city buildings, and then I went forward through the timeline for a while before I stopped.

Standard maintainers should feel free to adapt this branch for standard.  I'm just not going to do the work to rebase it, too much trouble.

kierongreen

Note that standard incorporated this a couple of weeks ago:

r6501 | prissi | 2013-05-09 22:35:17 +0100 (Thu, 09 May 2013) | 1 line

ADD: (neroden) clustering of houses, set "cluster_factor" in cityrules.tab to about 10 times the typical distribution weight

jamespetts

Yes, real life can interfere with Simutrans some times: I have also been busy in work of late. Glad to see this implemented in Standard, too! In the meantime, I have in any event set up cluster sets in Pak128.Britain-Ex.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

neroden

Quote from: kierongreen on May 25, 2013, 07:01:13 AM
Note that standard incorporated this a couple of weeks ago:

r6501 | prissi | 2013-05-09 22:35:17 +0100 (Thu, 09 May 2013) | 1 line

ADD: (neroden) clustering of houses, set "cluster_factor" in cityrules.tab to about 10 times the typical distribution weight


The adaptation to standard is missing a crucial part of the patch. 

See, this is why I like working with experimental; I get my entire patch committed...

This needs to be fixed.  Buildings in clusters must not have the "don't build next to identical neighbor" check.

Here's the bad code:

        } else if (gr->get_typ() == grund_t::fundament) {
          // do not renovate, if the building is already in a neighbour tile
          gebaeude_t const* const gb = ding_cast<gebaeude_t>(gr->first_obj());
          if (gb != NULL && gb->get_tile()->get_besch() == h) {
            return;
          }


Here is the correct replacement code:

        } else if (gr->get_typ() == grund_t::fundament) {
          uint32 my_clusters = h->get_clusters();
          if (my_clusters == 0) {
            // This is a non-clustering building.
            // If an identical building is in a neighbor tile, do not renovate.
            gebaeude_t const* const gb = ding_cast<gebaeude_t>(gr->first_obj());
            if (gb != NULL && gb->get_tile()->get_besch() == h) {
              return return_value; //it will return false
            }
          }
        }


This change is actually more important than the whole rest of the patch, in terms of getting the  city to look right.

kierongreen

I'm not sure whether this change was intentional or not - but there are benefits to having this code even for clusters. You can define terraced houses as having several different variants, then assign each one to the same cluster for example. Potentially you could even have it as an option for each cluster as to whether it prohibited or allowed the identical neighbour check. Alternatively prissi might have just overlooked this particular part.

As for getting the entire patch committed or not - just because someone has a certain vision which they implement with a patch doesn't mean this is the only way of doing things. Sometimes other developers might feel that tweaks would improve it - that's the nature of an open source project after all.

neroden

Quote from: kierongreen on May 25, 2013, 04:09:32 PM
I'm not sure whether this change was intentional or not - but there are benefits to having this code even for clusters. You can define terraced houses as having several different variants, then assign each one to the same cluster for example. Potentially you could even have it as an option for each cluster as to whether it prohibited or allowed the identical neighbour check. Alternatively prissi might have just overlooked this particular part.
You could.  In the paksets I've looked at, we have rowhouses and it's desirable to have a run of identical ones.  You just don't get the right results without this bit.  It was probably just overlooked.

QuoteAs for getting the entire patch committed or not - just because someone has a certain vision which they implement with a patch doesn't mean this is the only way of doing things. Sometimes other developers might feel that tweaks would improve it - that's the nature of an open source project after all.
And I very much appreciate the tweak so that the weighting is configurable. :-)  Which I am now attempting to backport to experimental.

prissi

The problem was that mostly everything was marked as conflicting. Thus (with the not so clear comment ;) it was simply overlooked.

However, I am somehow with kierongreen: 100% identical buildings are not so much desired and most rowhouses are not 100% identical. If there are two just variants in the cluster (which is not much), then it would still generate nice rowhouses.

However, the standard code should not only check for same besch but also check for same rotation before giving up.

neroden

Quote from: prissi on May 25, 2013, 08:54:29 PM
The problem was that mostly everything was marked as conflicting. Thus (with the not so clear comment ;) it was simply overlooked.
:-)

QuoteHowever, I am somehow with kierongreen: 100% identical buildings are not so much desired and most rowhouses are not 100% identical.  If there are two just variants in the cluster (which is not much), then it would still generate nice rowhouses.

Pak128.britain has several rowhouses with only one variant.  Also, if there are two variants, then prohibiting "same" gives strict alternation (A-B-A-B), which looks odd.  If you look at the demo game for pak128.britain, it has rows of identical rowhouses....

Quote
However, the standard code should not only check for same besch but also check for same rotation before giving up.
I do not know how to check what the rotation of a building is.  I do not know how to specify the rotation, either.  If I could do those I could improve the look of the rowhouses quite a lot, as well as improving the look of detached houses. The building rotation code is VERY complicated.

jamespetts

The Pak128.Britain demo map had its buildings placed by hand, so far as I am aware. As to rotation - it would certainly improve things if we had more consistent rotation.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

prissi

The rotation code is there, and works. Test is easy: tile is the besch with rotation; if the buildings tile are identical, then it has the same besch and the same rotation. (This I added to standard.)

You can even get identical houses to the pakset. If you have rowhouseA and rowhouseB with identical images, the images of those will be merged after loading. There will be just memory needed for one additional besch structure. But it would generate much more variation when there are more than one graphic.

neroden

Quote from: prissi on May 26, 2013, 08:34:22 PM
The rotation code is there, and works. Test is easy: tile is the besch with rotation; if the buildings tile are identical, then it has the same besch and the same rotation. (This I added to standard.)
I'll check out what you did.  As soon as I manage to merge the accounting code from standard to experimental.  :-)

What I want to do with rotations is this: if there is a rowhouse next door, I want to build a rowhouse in the SAME ORIENTATION.  IF you have a block

+---+
|xyx|
|xyx|
+---+

What happens right now is that some of the buildings in the "x" locations face east-west, while others face north-south.  So even if you have nice rowhouse graphics, it looks bad.
If I could find a way to check the orientation of neighboring rowhouse and COPY that orientation, I could successfully build rows of rowhouses.

Fabio

you can define corner images with 8-rotation buildings. check out pak 128 blocks.

neroden

Quote from: Fabio on May 27, 2013, 04:21:32 AM
you can define corner images with 8-rotation buildings. check out pak 128 blocks.

Well, that's all very well, BUT since I don't do artwork myself, I can't very well ask the artists to make corner images.  So I want this to work if those images aren't present!   Also, it's realistic to have buildings which don't "corner" properly, and I still want the rowhouses to line up like rowhouses.

So, this is the logic I'm trying to implement
(1) I have already decided to build a rowhouse (cluster 2, for example)
(2) I have already found a neighbor which is a rowhouse from the same cluster
(3) I have two or more different options for orientation. (For instance, I'm at a corner and can either face north or east.  I am not building a corner building, presumably because there isn't one.)
(4) I wish to copy the alignment of the neighbor if it is along a road from me
(5) or copy the "backwards" alignment of the neighbor if it is "behind the back garden" from me

This seems rather tricky.  I will have to dig into the orientation code quite deeply in order to figure out what is going on.

kierongreen

Defining a corner rotation doesn't mean you have to add new graphics though.

neroden

Quote from: kierongreen on May 28, 2013, 07:22:54 AM
Defining a corner rotation doesn't mean you have to add new graphics though.

That doesn't do the trick, though.  Think about this: you have a corner

+--
|xy
|z

If you define a corner rotation image for x, you have to define it facing EITHER north OR west.   What I want is, if "z" is a matching rowhouse, it faces west, but if "y" is a matching rowhouse, it instead faces north.  See what I'm trying to do?  I want to make sure that rowhouses are allowed to run both north-south and east-west (for diversity, to avoid boring cities, you know :wink: ).

VS

Well, you have a set of rules, and one wins, either the first, or the last, or some weighted combination. If-else-if-else... or maybe switch ;) If there are rows on both sides, you can still choose only from 2 orientations, and it matters little which you choose, as the row fits on both sides.

Perhaps you want the rotation to be to the side which matches with fewest cluster "constraints" needed?

Or maybe I misunderstood completely.

My projects... Tools for messing with Simutrans graphics. Graphic archive - templates and some other stuff for painters. Development logs for most recent information on what is going on. And of course pak128!

jamespetts

Since this has been incorporated into Standard, and the further discussion is (presumably...?) aimed at further improving the feature as implemented both in Standard and Experimental, and since the Experimental development forum is generally intended for features exclusive to Experimental, do people think that this thread ought to be moved to the Standard development board (perhaps the board for included patches)?
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

kierongreen

Yes - any further development should be coordinated to prevent unnecessary divergence between standard and experimental.

Fabio

bookmarking this great extension for (not so) future pak 128 use...