Public roads type selector

bigyihsuan commented 1 month ago

Motivation / Problem

Kale suggested in #jgr-patch-pack on the Discord to be able to select the road type of public roads. This was in the context of preventing towns from building on public roads; not doing so leads to these weird "tentacles" of housing along the public road:

Selecting a road type that prevents houses from being built next to it solves this problem; as the town is serviced and grows it will preferentially select its town roads over the public roads to build houses next to.

Plus it looks nicer to not have the town road stretch for miles between towns sometimes 😛

Description

This PR adds the ability to select a road type for public roads in the scenario editor.

When you are in the scenario editor, and click on the landscape generation button, the public roads window will open with it. This public roads window can be closed separately, and reopens when you click the landscape generation button again. When you close the main landscape generation window, the public roads window will also close.

The dropdown allows you to select a public road type. When you click the "Build public roads" button, it will use the selected road type instead of the default.

Images

Vanilla

With U&RaTT, and custom font

Limitations

The default road type is initialized on first opening the window, and will not change until you pick a new road type. When you build towns a different road type will be used. It is recommended to build towns before public roads.

Weird things happen when the available road types changes when you change GRFs. (Intended? since changing GRFs is always weird anyway)

Allowing public road type selection in a new game would require adding a new setting that depends on the available road types before they are fully loaded (i.e. when I tried migrating my code from the scenario editor to the new game menu, it was extremely glitchy and didn't work). This would come in a new PR.

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, game_changelog.hpp need updating.
- The compatibility wrappers (compat_*.nut) need updating.
This PR affects the NewGRF API? (label 'needs review: NewGRF')
- newgrf_debug_data.h may need updating.
- PR must be added to API tracker

JGRennison commented 1 month ago

This looks pretty good. A few minor things. There should probably be a bit of padding over the public road types label to separate it from the buttons above. GetTownRoadType() is not necessarily in the list of user-buildable road types as returned in GetScenRoadTypeDropDownList(). This is only the case for some old/awkward GRFs (e.g. U&RATT 0.2), but it'd probably be worth adding it to the list if not present.

JGRennison commented 1 month ago

I've pushed a few changes to tidy up some minor things. In particular, the default selected road type should now be initialised to the right value for the current road type configuration. It'll be reset when the road type configuration is changed. This should fix the issue when changing GRF configuration or changing between scenarios.

I'm not really convinced about the button opening two windows which are partially linked. This seems somewhat strange and unexpected from a user interface point of view. It would seem more straightforward to use the dropdown pattern like the other toolbar buttons with multiple functions.

Generally the public roads function only needs to be used once when setting up a scenario, whereas the landscape tools may be used frequently. Having the public roads window pop up every time the landscape window is requested is not likely to be appreciated by all scenario editor users.

bigyihsuan commented 1 month ago

This seems somewhat strange and unexpected from a user interface point of view.

There is some precedent with the terraforming toolbar being linked with the rail/road/etc toolbars, but I see what you mean.

JGRennison commented 1 month ago

I've made the changes so that the scenario editor toolbar is used. If you're all right with things as they are, I'm happy to merge this.

bigyihsuan commented 1 month ago

I've made the changes so that the scenario editor toolbar is used. If you're all right with things as they are, I'm happy to merge this.

I'm fine with your changes, go ahead.

JGRennison commented 1 month ago

Thanks for this

JGRennison / OpenTTD-patches