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

Change: Remove land generator setting from World Generation GUI #10093

Merged
merged 1 commit into from Jan 12, 2023

Conversation

2TallTyler
Copy link
Member

@2TallTyler 2TallTyler commented Oct 21, 2022

Motivation / Problem

In a Discord discussion about #10092, @JGRennison and @LordAro suggested removing the original map generator code entirely. I'm not willing to die on that hill, but there's no reason the setting needs to be in the already-cluttered World Generation GUI.

Description

worldgen

Removes the dropdown selector from the World Generation GUI.

Players who wish to select the original land generator can do so in Settings.

Limitations

  • "We should not be simplifying the game to appeal to new players," etc...

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR touches english.txt or translations? Check the guidelines
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@LC-Zorg
Copy link

LC-Zorg commented Oct 21, 2022

If this option will be still available in the settings window, from my point of view it doesn't make much sense to keep that in a new game window as well. It seems to be used very rarely. Personally, I only used it once and only to see what it changed. Well, the old style maps aren't very interesting and playable.
Instead, it would be nice to be able to see the setting for the trees again, preferably one that could tell you how many trees will be on the map. :)

@Arastais
Copy link
Contributor

I personally think this is a great change overall, and don't think it simplifies the game for new players - rather it just simplifies the world generation UI. Plus, all it really does it removes some UI anyway; The setting is already there so this makes the most sense to just remove the redundant UI. I also agree that the old land generation should be kept around just as a feature, but it's used infrequently enough that it doesn't need to be a main part of the UI.

Assumin you're referring to the SF_NEWGAME_ONLY in world_settings.ini, i think it makes the most sense to keep that flag. It's already had that flag and all this PR is doing is removing the GUI, so it makes sense to keep it. Also, an even better argument is that it doesn't really make sense to change the land-generation mid-game (or on an existing save). Even though it probably won't have any effect, it's probably best to keep that flag just for the sake of it's only meant to affect new games. Not to mention it will probably confuse people if hey can change the setting in the settings UI in the middle of a game.

@2TallTyler
Copy link
Member Author

Updated for #10058 (including screenshot in PR description).

@2TallTyler 2TallTyler added the size: trivial This Pull Request is trivial label Nov 11, 2022
@2TallTyler 2TallTyler added this to the 13.0 milestone Nov 24, 2022
@LordAro
Copy link
Member

LordAro commented Nov 24, 2022

Currently this has an issue that LC alluded too - the setting is now only in the config file - if you upgrade with it set to TTD-mapgen, there's no obvious way of changing it to TGP.

Obvious solution would be to add it somewhere in the settings window

@2TallTyler
Copy link
Member Author

Turns out it was already in the Settings window, but the original PR commit broke strings and the dropdown. Try now. 🙂

@2TallTyler 2TallTyler added the preview This PR is receiving preview builds label Nov 27, 2022
@DorpsGek DorpsGek temporarily deployed to preview-pr-10093 November 27, 2022 17:18 Inactive
@2TallTyler 2TallTyler removed this from the 13.0 milestone Jan 1, 2023
Copy link
Contributor

@rubidium42 rubidium42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not caused by this change... but maybe more noticable due to this.
When you change the generator setting in the Settings window, the UI of World Generation does not get updated. So the UI will still show the TGP fields, but when clicking on Generate you get a map made with the original logic.

@2TallTyler
Copy link
Member Author

I'm unable to reproduce that behavior either on current master or in the Preview build of this PR. When I change the land generator setting, it is automatically changed in the world gen UI — in master the generator dropdown is updated and in both the TGP fields are disabled.

Can you go into more detail about how to reproduce this?

@rubidium42
Copy link
Contributor

Can you go into more detail about how to reproduce this?

pr10093

In the preview build I opened the World Generation window, then the Settings window and I changed the Land Generator setting in the Setting window. Then the World Generation window does not get updated, i.e. the disabled buttons are not enabled.

In master the buttons do not change either, although... the drop down slowly changes as vehicles are moving behind it. Similarly, changing the Landscape from the Settings window does not (immediately?) update the World Generation window or Intro window, however changing the landscape in either the World Generation or Intro window does update the other and the setting immediately.

Changing the Land generator setting does not update the disabled status of the TGP-only input fields in master for me.

@2TallTyler
Copy link
Member Author

Oh, I see now. I only had one window at a time open, going back to the main menu each time.

The same behavior is also true for any of the other buttons that are duplicated in both Settings and the World Generation GUI (why are they in Settings? Probably another discussion...).

Can we consider having both Settings and World Generation GUIs open at once to be an edge case? 🙂

@2TallTyler 2TallTyler merged commit 5a2907a into OpenTTD:master Jan 12, 2023
@2TallTyler 2TallTyler deleted the no_land_gen branch January 12, 2023 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview This PR is receiving preview builds size: trivial This Pull Request is trivial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants