Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move "town name" selection into map generator GUI #8566

Merged
merged 6 commits into from Feb 14, 2021

Conversation

@frosch123
Copy link
Member

@frosch123 frosch123 commented Jan 13, 2021

Motivation / Problem

There have been plenty of attempts to redesign the map generation window. Most get derailed into wanting to change everything.
This PR deals with two "world generation" settings:

  • "Town names": currently only in "game options".
  • "Road drive side": currently in "game options" and in the "settings tree".

Both settings do not belong into "game options". The most reported issue related to NewGRF town names is, that players do not "find" the "town name" dropdown to enable them.

Description

Main change:

  • "Town name" selection is removed from "game options", and put into the map generation window.
    • This setting is "interesting" enough to put it into a prominent GUI location.

Collateral changes:

  • "Road drive side" is removed from "game options", and demoted to "settings tree only".
    • This setting does not fit in with the other "basic" settings in "game options". I don't think anyone would look for that setting in "game options", unless you are used to it being there.
    • It gets the same "basic" visibility as "train signal side".
  • "Tree plant algorithm" is removed from the map generation window, and demoted to "settings tree only".
    • The difference between "original" and "improved" is hardly a popular setting.
    • "none" is probably popular, but only makes sense when "in-game growth" is also disable, which is in the settings tree already.
    • It gets the same "basic" visibility as the "in-game growth" setting.

Limitations

None.

Checklist for review

N/A

@andythenorth
Copy link
Contributor

@andythenorth andythenorth commented Jan 14, 2021

Easy +1 from me.

I did use the 'thumbs up' on the PR, but I don't really trust that as a process yet.

STR_GAME_OPTIONS_ROAD_VEHICLES_DROPDOWN_LEFT :Drive on left
STR_GAME_OPTIONS_ROAD_VEHICLES_DROPDOWN_RIGHT :Drive on right
Comment on lines 950 to 951

This comment has been minimized.

@LordAro

LordAro Jan 16, 2021
Member

I wonder if the remaining string ids should be renamed, given they're no longer in the game options

This comment has been minimized.

@TrueBrain

TrueBrain Jan 22, 2021
Member

I agree that might be a good thing for future-us :D

uint gen = _settings_newgame.game_creation.town_name;
StringID name = gen < BUILTIN_TOWNNAME_GENERATOR_COUNT ?
STR_GAME_OPTIONS_TOWN_NAME_ORIGINAL_ENGLISH + gen :
GetGRFTownNameName(gen - BUILTIN_TOWNNAME_GENERATOR_COUNT);

This comment has been minimized.

@LordAro

LordAro Jan 16, 2021
Member

pedantry: not sure i agree with the line splitting here. All on one line, or split with the ?/: as the first character

@LC-Zorg
Copy link

@LC-Zorg LC-Zorg commented Jan 17, 2021

Town names
It's a good change, but what about creating scenarios? Here, choosing a setting in the create new game window doesn't seem like a logical place.

  • Currently: important that changing the setting also should works for the editor
  • In the future: it would be nice to add options for selecting city names in the scenario editor
    City creation window v1 0

Traffic direction on the roads
It also seems like a good change. My only doubts are that this setting maybe should be in the Vehicles / Routing section - I haven't thought too much about it, but maybe it's a more appropriate place?
Detail: In the scenario editor, when there are moving vehicles, this option could be disabled.

Trees algorithm
The choice of an algorithm really seems unnecessary here. In my opinion, this is one of those rarely or never used settings that would fit into the real "Weird" category. ;)
However, the very selection of whether the map is to be generated with or without trees is one of the very basic settings that should be available in the map creation window.
If possible deletion of option is dictated by lack of space for "Town names", "Land generator" seems to be a much less used setting.

  • Currently: I would leave "Trees: Yes / No"
  • In the future: I would have a proposal that it would be possible to define the limit of the number of trees on the map. I wrote a little more about it on the forum.

World generation window layout
Since you are changing the layout a bit, it would be a good idea to group the settings a bit better. :)

World Generation window v1 0
-without Land generator
-Smoothness setting in terrain section
-River and Sea level close together (water section)
-Town names and No. of towns also close together (towns section)

World Generation window v2 0
-with Land generator

World Generation window v3 0
-another layout, rather not the best but just for inspection

Copy link
Member

@LordAro LordAro left a comment

#shipit

@frosch123 frosch123 merged commit 5a1fa18 into OpenTTD:master Feb 14, 2021
8 checks passed
8 checks passed
Emscripten
Details
Commit checker
Details
Check for preview label Check for preview label
Details
Linux (clang, clang++)
Details
Linux (gcc, g++)
Details
Mac OS (x64, x86_64)
Details
Windows (x86)
Details
Windows (x64)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants