boformer / BuildingThemes

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

[WIP] Theme editor gui #49

Closed SamsamTS closed 9 years ago

SamsamTS commented 9 years ago

Just the beginning. It ain't much.

On the left is the list of themes. Buttons to add/delete/rename themes are planed.

On the right side will be 2 lists

With the ability to select and move (with arrow buttons or drag&drop possibly) buildings from one to another. Filters will be available to only display certain types of buildings (buttons with icons. ie. residential, commercial, etc...) and size (combobox). And of course a search box.

I'm also planing to add a way to import collections form the workshop. And maybe add a share button, it would generate a theme mod and send it to the workshop, all automatically.

Let me know what you think.

boformer commented 9 years ago

I think a search field for themes (on the left) is not required.

boformer commented 9 years ago

Also, it might be better to have one list for the buildings with a checkbox to include /exclude a building.

We will also need space for a building preview (+ stats) on the right side.

boformer commented 9 years ago

Any news?

SamsamTS commented 9 years ago

Sorry it's going slow. I have been unexpectedly busy lately (injured coworker I had to replace, plus the heat...). I'm hoping for a first version of the UI by the end of the WE if all goes well.

boformer commented 9 years ago

Another feature that will be needed: 2 dropdowns to set the allowed min and max level of a building in a theme.

I'm adding that feature right now.

I'm also working on themes that only limit the growth of certain zone types (e.g. Only residential low = commercial and industry will still grow without a limit). No idea how we can incorporate that. Maybe check boxes?

boformer commented 9 years ago

Just tested it, the filtering works great!

boformer commented 9 years ago

The steam icon is a nice idea! I also like how you solved the "not loaded" problem.

A few ideas and notes:

We can place the button to open the Theme Management GUI in the Policy Panel, just like the checkboxes I added with the last mod update. This is the code section.

I really like that the preview displays the building of the last hovered item, but we also need a way to select a building to set a few advanced properties (which can be displayed under the preview area): The properties I implemented in my branch are: Spawn Rate (Integer) Upgrade Name (Name of the asset that this one should upgrade to)

A few properties should only be editable for duplicate assets:

Base Name (For asset duplication. The name of the base asset) Name (For asset duplication. The name of the duplicate) Level (For asset duplication. The level of the duplicate)

All of these properties can be edited per theme.

I don't know how we can implement asset duplication in a user-friendly way. Maybe with a "Create Duplicate" button under the preview area. When you click on it, a new building item is created and selected. The user has to enter a unique name for the duplicate and select a level. Duplicates should be displayed in a different color

I propose the following behavior: Building items are selectable, like the theme items on the left. When no Building is selected, no preview is displayed. When the mouse enters a Building item, the preview + properties of the selected Building is overridden by the hovered Building item. When the mouse leaves the building item, the preview + properties of the selected Building is displayed again.

Right now the theme selection in the Theme Manager does not work. No matter what theme I select, the building checkboxes always resemble the last theme on the list. Screenshot. What works is that "not loaded" buildings are displayed depending on the selected theme (the checkboxes of those are also selected).

SamsamTS commented 9 years ago

This is pretty much what I had in mind. Glad we have the same ideas, this comforts me in my design choices.

I didn't knew much about spawn rate, upgrade and duplication. Not in details anyway. Thanks for the clarification. I planned to use the space below the preview to display information about the building (category, size and level) but it's also a good place to put editable properties, duplicate button and what-not.

Also the layout is not final yet. I planned to add some space under the theme list for "Add" and "Remove" buttons and some space under the building list for "Include all" and "Exclude all" buttons.

While I'm here, I have a couple of questions about the European and International themes. Should they be editable? Should they be removable?

boformer commented 9 years ago

Editable? yes. Removable? no.

European and International Theme and all other "Mod Themes" are loaded from separate xml configurations. These separate configurations are merged with the user configuration in the game directory when a city is loaded.

The Configuration.Theme#isBuiltIn field indicates if the theme is built in or created by the user.

There is a similar field Configuration.Building#isBuiltIn which indicates if a building config setting is part of the built in configuration, or if it was added by the user.

The field Configuration.Building#include is used to exclude a building from a built in theme.

Here is how you would modify a user theme:

var theme = BuildingThemesManager.instance.Configuration.getTheme("User Theme");

// add building
if(!theme.containsBuilding("tenement idkwhat") 
{
    theme.buildings.Add(new Configuration.Building("tenement idkwhat"));
}

// remove building
var building = theme.getBuilding("tenement idkwhat");
if(building != null) 
{
    theme.buildings.Remove(building);
}

BuildingThemesManager.instance.SaveConfig();

Here is how you would modify a built in theme:

var theme = BuildingThemesManager.instance.Configuration.getTheme("European");

// add building
if(!theme.containsBuilding("tenement idkwhat") 
{
    // this constructor sets the Building.isBuiltIn field to false
    theme.buildings.Add(new Configuration.Building("tenement idkwhat"));
}

// remove building
var building = theme.getBuilding("tenement idkwhat");
if(building != null) 
{
    if(building.isBuiltIn)
    {
        building.include = false;
    }
    else
    {
        theme.buildings.Remove(building);
    }
}

BuildingThemesManager.instance.SaveConfig();
boformer commented 9 years ago

What is missing is a method in BuildingThemesManager to refresh the compiled building lists for all districts and to check if the referenced themes are still listed in the configuration.

I will write one and push it to master.

SamsamTS commented 9 years ago

Perfect explanation. Thanks.

boformer commented 9 years ago

I forgot one thing. When you add a building to a built in theme, you have to do this:

var theme = BuildingThemesManager.instance.Configuration.getTheme("European");

// add building
var building = theme.getBuilding("tenement idkwhat");
if(building != null) 
{
    if(!building.include) 
    {
        building.include = true;
        building.isBuiltIn = true;
    }
}
else
}
    // this constructor sets the Building.isBuiltIn field to false
    theme.buildings.Add(new Configuration.Building("tenement idkwhat"));
}

A building might be listed in the config with "include=false". That means the user manually excluded the building. In that case, to enable the building you have to set "include=true" and "isBuiltIn=true".

SamsamTS commented 9 years ago

I think I got it. If it's builtIn then use include, else use add/remove.

boformer commented 9 years ago

Your level display does not work correcly for farming/ore/forest/oil industry. All of these buildings are L1. Some of them are displayed as L2 in the theme manager.

SamsamTS commented 9 years ago

According to mod tools the ones that display L2 effectively are level 2 which is... odd. I not really sure how to deal with that. It's not really my fault if CO made those buildings Level 2 even-though it's probably not used afterward. Should I check if the building is one of these subservices then display L1?

boformer commented 9 years ago

I guess.

boformer commented 9 years ago

Just merged my PR. Upgrade control and spawn rate...

boformer commented 9 years ago

I think we can live without the better upgrade part for now.

boformer commented 9 years ago

Testing it right now. You did an excellent job!

I got one suggestion: Keep the scroll position when switching themes. That makes it easier to compare 2 themes.

boformer commented 9 years ago

When creating a new theme, the buildings of the previous selected theme are still selected.

There is also a related bug that causes duplicate entries in the xml config (though no building is selected in the theme manager):

    <Theme name="UK Terraced Housing">
      <Buildings>
        <Building name="agricultural_building_05" />
        <Building name="agricultural_building_05" />
      </Buildings>
    </Theme>
boformer commented 9 years ago

No bugs so far.

boformer commented 9 years ago

Ok, I think this is ready for merge and to be published to the public. I will do it this evening. You can announce it on reddit when the update is out.

SamsamTS commented 9 years ago

I have one final push to make (minor things) then yeah I think it's ready to be released.

SamsamTS commented 9 years ago

It's over, it's done. You can merge now I think.

boformer commented 9 years ago

I noticed one exotic bug: When you uncheck a building that is not loaded, then check it again, this is added to the config:

    <Theme name="Mini Homes">
      <Buildings>
        <Building name="L1 1x1 Detached" />
        <Building name="L2 1x1 detached01" />
        <Building name="L3 1x1 Detached" />
        <Building name="" />
      </Buildings>
    </Theme>
boformer commented 9 years ago

I fixed it by adding a m_name field to BuildingItem.