boformer / BuildingThemes

Mod for Cities:Skylines
http://steamcommunity.com/sharedfiles/filedetails/?id=466158459
MIT License
13 stars 11 forks source link

Building Footprint selection #13

Closed boformer closed 9 years ago

boformer commented 9 years ago

One problem of the default "ZoneBlock.SimulationStep" method is that it always searches for the largest possible footprint.

If no prefab with the right footprint is available in that category, nothing will grow.

It would be better if it worked like this:


Example: Available Space: 4x4 (width x depth)

No prefab found? Look for 2x4 (place it on left or right side of plot to grow another 2x4)

No prefab found? Look for 1x4


No prefab found? Look for 4x3

No prefab found? Look for 2x3 (place it on left or right side of plot to grow another 2x3)

No prefab found? Look for 1x3


No prefab found? Look for 4x2

....

bloodypenguin commented 9 years ago

Yeah, I'll see into it. This approach might also solve infamous Hostel Tower problem when a building cannot levelup when there are no buildings of Level X+1 of the same footprint. I will also check if it's possible to make a separate mod for that (I prefer modularity).

boformer commented 9 years ago

Difficult because the logic is contained in the ZoneBlock.SimulationStep method.

bloodypenguin commented 9 years ago

This issue looks to me much more complex now than I thought previously. Imagine the following situation: a theme has only 4x4 L5 bulidings. If the theme doesn't have 4x4 L1, 4x4 L2, 4x4 L3, 4x4 L4 buildings those buildings will never grow. So I propose the following solution: In GetRandomBuildingInfo if our filtered list is empty but original (pre-filtered) areaBuildings list is not, then we just fall back to any available building of that footprint. What do you think? We still need to think about the following cases even if that algorithm works:

bloodypenguin commented 9 years ago

I think we need to release current vanilla-only themes version first and only after that think of proper custom themes handling. As you see there are just too many pitfalls. Custom assets will still grow now if no theme selected for district. Maybe we need to add temporary 'Custom growables' theme to include all custom buildings if user, say, only wants European buildings to grow in historical downtown and International and custom in the rest of the city.

bloodypenguin commented 9 years ago

Another idea: a user should select at least one of vanilla themes as the fallback theme for each district in addition to N primary themes

boformer commented 9 years ago

I agree with you that this issue describes an advanced feature (not high priority right now)

A simple example where this feature is useful is the "UK terraced housing" project (only 1x2 buildings): https://steamcommunity.com/sharedfiles/filedetails/?id=452704398

then we just fall back to any available building of that footprint.

Or just don't upgrade the building?

I noticed that many vanilla prefabs are duplicates (just minor prop changes) with different levels. Content creators aware of this mod should also provide multiple versions of their buildings (different footprints, different levels).

If a theme has only L3 buildings

We should warn the user. Such a theme will only work in combination with other themes.

We could also display a ingame warning (chirper?) when upgrading fails because of "bureaucracy".

Another idea: a user should select at least one of vanilla themes as the fallback theme for each district in addition to N primary themes Too difficult.

bloodypenguin commented 9 years ago

Too difficult.

A simpler solution may be to fall back to environment's default vanilla theme's buildings

boformer commented 9 years ago

The ZoneBlock.SimulationStep method is a mess. I am trying to understand what it does.

What I found out so far:

This is where the position of the new building is calculated:

vector6 = this.m_position + VectorUtils.X_Y(((float)num26 * 0.5f - 4f) * vector + ((float)num25 * 0.5f + (float)num2 - 10f) * vector2);

With readable variable names:

vector6 = this.m_position + VectorUtils.X_Y(((float)length * 0.5f - 4f) * xDirection + ((float)width * 0.5f + (float)spawnpointRow - 10f) * zDirection);

The position vector is exactly in the center of the footprint of a building.

Later this vector is used to get a random BuildingInfo:

buildingInfo = Singleton<BuildingManager>.instance.GetRandomBuildingInfo(ref Singleton<SimulationManager>.instance.m_randomizer, service, subService, level, width, length, zoningMode3);

After this statement, we could inject a statement that checks if no buildingInfo was found, and then reduce the length and the depth and try to find a BuildingInfo.

bloodypenguin commented 9 years ago

Thanks for that! I'll investigate that more closely later. Also I've seen multiple times that, say, a 3x4 building grows despite that it's enough space for a 4x4 building.

boformer commented 9 years ago

Fixed by https://github.com/boformer/BuildingThemes/commit/674db5e2574911870904ad0e00629028b6a5718a

fadster commented 9 years ago

very interesting discussion, i'm glad i found it! i've been troubled by the fact i could not get a L4 4x4 building to upgrade to a L5 4x4 unless i specified it in the XML file using the upgrade-name attribute. all i got in game were smaller buildings (most, if not all, 4x3) with added props to fill the lot. reading through the (very legible!) code, i thought i could obtain the behavior i want with this little modification:

public FastList<ushort> GetAreaBuildings(byte districtId, int areaIndex, int width, int length)
{
    var areaBuildings = GetAreaBuildings(districtId, areaIndex);
    var sameSizeBuildings = new FastList<ushort>();

    for (int i = 0; i < areaBuildings.m_size; i++) {
        BuildingInfo prefab = PrefabCollection<BuildingInfo>.GetPrefab((uint) areaBuildings.m_buffer[i]);
        if (prefab.m_cellWidth == width && prefab.m_cellLength == length) {
            sameSizeBuildings.Add((ushort) areaBuildings.m_buffer[i]);
        }
    }

    return (sameSizeBuildings.m_size > 0) ? sameSizeBuildings : areaBuildings;
}   

Much to my dismay, this caused the game to crash with a NullReferenceException while buildings were upgrading. This is my first attempt at fiddling with mods and I was wondering if you would be so kind as to explain why this didn't work. Thanks!

P.S. Sorry if this wasn't the right place to post this.

boformer commented 9 years ago

The problem is that the method is called every tick on building upgrade.

Make sure to remove all log calls.

A NullRefException could be caused by many things. Check if:

areaBuildings != null prefab != null

fadster commented 9 years ago

You nailed it my friend! I had to check for null areaBuildings, which happens when a theme doesn't contain any possible upgrades for a given building. Working like I want it now! :) In the meantime, I also implemented another way of getting this result, by building a dictionary for each areaIndex mapping footprint size to a fastlist of potential upgrades of that size. These dictionaries are built in RefreshAreaBuildings, so the performance is slightly better at the cost of increased memory use. The results in gameplay are otherwise identical. Let me know if you're interested in the code.

By the way, any reason you didn't implement things this way? Just curious.

boformer commented 9 years ago

I recommend you to look at this code segment: https://github.com/boformer/BuildingThemes/blob/master/BuildingThemes/BuildingThemesManager.cs#L431-L464

It is a part of the RefreshAreaBuildings method (copied 1:1 from the game code).

When you remove it, buildings will only spawn/upgrade when the exact footprint requirements are met.

With the code segment: empty 4x4 plot --> 4x1 or 4x2 or 4x3 or 4x4 will spawn, backyard will be extended to be 4x4 existing 4x4 building --> will upgrade to 4x1 or 4x2 or 4x3 or 4x4, backyard will be extended to be 4x4

Without the code segment: empty 4x4 plot --> 4x4 will spawn existing 4x4 building --> will upgrade only to 4x4

I think that the game's behaviour makes sense. But this could be implemented as an option per district. If you want, submit a pull request.

fadster commented 9 years ago

Ah, so this is what this strange piece of code does! Yes, I've noticed this is the stock behavior, so I might just see how things go the way they are. Otherwise, I'll submit a pull request and polish up my code. What motivated me to explore this issue in the first place was the fact that I couldn't get any of my fancy 4x4 L5 buildings to appear, with the game insisting on spawning the same smaller buildings over and over. Perhaps another way of addressing this would be to manipulate the availableBuildings lists according to the buildings already present in a district. If, for example, some building has already been built several times in the district, we could reduce its occurrence in the array (similary to how you use the spawn-rate variable). This would introduce some variety in the district over time. Just an idea.

boformer commented 9 years ago

That would be possible. But usually the problem is that there are just not enough buildings to make use of the spawn rate feature.

Another idea how to solve your upgrade problem: A lowered spawn rate for buildings with a different footprint size.

So a 4x2 prefab id is only added one time to the 4x4 fastlist, a 4x3 id is added 3 times, a 4x4 id 10 times. That raises the chance that a 4x4 building is selected on upgrade.

fadster commented 9 years ago

Hey, sorry for the late reply, got caught up with work on traffic mods. I also had the idea of tweaking spawn rates, that would be one way of addressing the issue. I'm not sure what fastlists you're talking about though. Does your code maintain separate lists by footprint or were you talking about the ones I introduced in my alternate solution?

boformer commented 9 years ago

Yep it does. There is a separate list for every combination of zone type, footprint size, level, corner type. The "areaIndex" is calculated using those values.

The areaBuildings field is an array of fastlists, one for every area index.

boformer commented 9 years ago

Btw, that's how the vanilla game works.

This mod adds additional areaBuildings arrays for every district with theme management enabled.

fadster commented 9 years ago

Ok I'll have to look a little closer at the computation of areaIndex, I hadn't noticed there were already separate lists by footprint. Thanks for the info!