Districts R27

[instafluff0]

Hey @maxpetul,

I hope things are going well. I've been hard at work on the new additions to districts. As seems to be my MO these days, I set out with a very specific set of goals and a relatively narrow scope. That lasted about ... 2 weeks. As I reworked things and added new features, those in turn inspired me to add more ... and more. I also have gotten a lot of feedback from modders and so on on CivFanatics and did my best to add straightforward features that weren't much of a lift, or seemed worth it.

Anyway, the PR has become quite big (again), for which I can only apologize for asking you to do yet another huge code review. Thank you very much, as always.

After this gets merged and any outstanding bugs get worked out, I plan to go quiet for a while and take a break. At this point I have no intended major additions I want to do to districts - this PR is kind of the kitchen sink. After this I think districts will actually be very mature, in terms of features and robustness. So I can't make any promises, but at this point the only remaining dev I intend to do is adding a seasonal cycle, which will mostly be art anyway.

Changelog

Here's a reasonably exhaustive list of changelog items:

• 11 new districts: Port, Canal, Bridge, Municipal, Central Rail Hub, Data Center, Energy Grid, Great Wall, Ski Resort, Offshore Extraction Zone, Water Park
• 7 new natural wonders: Wadi Rum, Eyjafjallajokull, Ha Long Bay, Lofoten Skerries, Geirangerfjord, Delicate Arch, Savanna (mostly a desert/tundra theme this time)
• 3 new wonders: Great Lighthouse, Colossus, Hoover Dam
• Add support for maritime districts
• Move Research Lab from Campus to Data Center dependency
• Add Great Wall auto-build around borders event after Great Wall wonder is completed
• Add support for Named Tiles, which shows custom verbiage on a given tile (can be set in-game or via scenario.districts.txt)
• Fix bug with districts not rendering on right-click tile detail modal
• Fix bug with workers "building" a natural wonder if user clicks "Move all units of type" button
• Fix bug with custom scenario district art not loading from scenario folder
• Add support for alternative directions of Wonders, based on city vs. wonder tile location
• Add negative district bonuses
• Add custom logic for river- & port-related rendering
• Add granular configuration control over where districts can be built by tile and adjacent tile types
• Add support for districts being dependent on wonders & natural wonders
• Add support for districts enabled by alliances with other civs that can build them
• Add support for restrictions of who can build districts by civ name, trait, culture, government
• Allow multiple advance prerequisites
• Touch up art for Industrial Zones, Newton's University, Holy Site, Bach's Cathedral
• Add support for buildings being buildable if one of multiple possible districts is built
• Add support for districts generating resources
• Add support for districts requiring resources, both in cities or on a tile
• Add optional flag for districts to serve as alternative irrigation source
• Add patches to enable buildings dependent on adjacent water sources to be enabled if water in work area (eg, Ports can be built if city has water in work radius), except for Aqueducts & Coastal Fortresses, which still require adjacency
• Add flag for districts with dependent buildings to be custom tile improvements, which encourages the AI to evaluate whether to build them on each tile, like irrigation and mines
• Add much smarter AI for distribution hubs, with ai_distribution_hub_build_strategy = auto (AI assess civ-wide and city level needs to determine how best to use tiles and distro hubs to build)
• Add more balance distribution hub yields, with amount of shields/food each city receives decreasing as the number of recipient cities grows
• Add fairly sophisticated AI build strategy and analysis of where to place canals and bridges
• Add cap on max distribution hubs relative to number of cities
• Add flag for show_ai_city_location_desirability_if_settler, which automatically shows city desirability over tiles if a settler is selected. This is extremely useful if minimum_city_separation is higher than normal, as it's hard to tell where cities can be settled otherwise
• Add flag for auto_zoom_city_screen_for_large_work_areas, which prevents the city screen from auto-zooming out if the city_work_radius is >= 4 (my personal preference, but off by default)
• Add support for extraterritorial colonies, where colonies can be built in another civ's territory or be built beforehand but not disappear as borders grow, at a relation penalty
• Add happiness_bonus to district yields
• Add display_name, in case name shown to user is different
• Add optional custom height/width & x/y offsets for district art
• Add error messaging to main screen if buildings / techs aren't parsed correctly, like other config options (in R26 this silently fails)
• Distro hub yields on city screen are drawn even if outside city work radius (useful as in R26 they aren't drawn if outside the work radius, even if city is receiving distro hub yields)
• Make tile highlights green, not red - don't kill me, and let me know if you don't like this. I just found green to be more inviting and intuitive in a "deeper green is better" sense
• Replace all redundant, hard to read district tile loops with proper macros
• Add custom logic for air unit position and bombing if using aerodromes. This was necessary as I found that the AI never moved air units otherwise, and turned out the vanilla code explicitly looped over cities & airfields (duh, I suppose), so if cities weren't valid places to move to, the AI never did
• Add support for alternative rendering logic (in districts config, render_strategy = "by-building") for handling districts whose buildings are not dependent on each other. The main use case for this was the Municipal District, but I wanted to generalize it so anyone could do this and we wouldn't need to add custom districts in the future
• Add custom logic for naval units to heal in ports and pillage enemy ports. This was necessary as otherwise, if naval_units_use_port_districts_not_cities = true, the AI never healed ships and never attacked others' ports
• Add custom logic for unit movement on bridges, canals, great wall
• Allow flexible additive bonuses for districts in config based on nearby cities' available buildings and district tile & overlay types
• Allow naval units to pillage (maritime districts)

Current issues need advice on

I'm making the PR now as (1) I don't anticipate any other major changes to the codebase unless you request it, and (2) I've basically hit a wall on a few items I don't know how to resolve and need your help. Most of these are related to unit movement, which I think you understand much better than I do.

Those are:

1. Naval units can't actually enter canals. I've reviewed the vanilla code extensively and tried many different patches, but I can't get it to work. The pathfinder works fine, and correctly calculates the number of turns it would take to move and suggests the unit can pass. But when I actually have a ship try to enter, it just doesn't. Kind of pulling my hair out on this one. Relevant functions are patch_Unit_can_move_to_adjacent_tile and patch_Unit_move_to_adjacent_tile.
2. Land units can successfully move on bridges and workers can enter coastal tiles (if workers_can_enter_coast = true), but even if set on a multi-turn path, they wake up every turn. This is annoying. I suspect this is due to water rules with land units in transports, where they wake up if not sleeping, but again haven't been able to pin down exactly where in the codebase this actually happens.
3. Related to (2), as I said, land units can successfully move on bridges, including handling railroads (yay), but when they leave a bridge and move to land - even if both tiles have roads/railroads - it still takes a full turn. I assume this is due to vanilla logic of land units moving from a transport to land, but haven't figured out where this happens or where to patch.
4. Figuring out how to enable the Pillage command/button for naval units was a huge pain. I initially tried to do so via checks in patch_Unit_can_perform_command and patch_Leader_can_do_worker_job, but those seemed to have no effect and I think for naval units they perhaps aren't checked at all? Not completely sure. As a workaround, I patched patch_Main_GUI_set_up_unit_command_buttons and put the check there, BUT the code I got working to actually enable the button is ugly and took a lot of poking around the source to understand the offsets and get the right images. I hate it but couldn't figure out a better way. This works but I would love suggestions for a better way to do it.
5. I was able to figure out how to negatively affect a Leader's attitude toward one another by setting a penalty for each extraterritorial colony, in patch_Leader_get_attitude_toward. The relation penalty in the config is called per_extraterritorial_colony_relation_penalty. I couldn't figure out how to actually loop over colonies directly though, so as a workaround loop over every tile on the map. In the source code I was able to find and add p_colonies to civ_prog_objects.csv, but don't quite understand how to loop over it. This is probably really dumb but I could not quite figure out why this works for p_cities and not p_colonies.
6. The district property buildable_by_civs checks civ names by strings. I really wanted to do this by just figuring out the civ_id and indexing that on start/load, but as configs are loaded before civs are decided, I couldn't do my typical approach. If you're ok with this I am too, but just wanted to point that out.

I think the code is relatively clean, and I worked hard to incorporate your feedback from the first districts PR and keep everything tight. Overall I think it is solid. As you review the code though, you will probably come across align_variant_and_pixel_offsets_with_coastline and a few other functions which handle rendering maritime district stuff. I'll just say I hated putting specific if/else statements there to try to handle pixel offsets for specific terrain tile sheets and sprites, but it really looked bad without it and I couldn't find any cleaner way. In terms of how far beach/coast/land is on the coastal terrain art, it really is wacky and all over the place. Without handling that, ports and so on often appear way out in the water and look stupid. Anyway long-story-short: I know the code there is ugly and am very open to feedback. But the game looks much better because of it.

Remaining stuff to do

Like I said the code is basically done, besides refinement based on feedback and thie issues I need help on above. I'm planning to refactor and add a buildable_on_overlays property for districts that is distinct from buildable_on, and should be done with that in a day or two. (this is done)

There is still some art to finish as well. ZergMazter is doing the art for Central Rail Hub and Municipal District, and probably needs a couple more weeks. I'm working on the light annotations for the new districts as well. So let's wait to actually merge this in until after those are done, but I saw no need to wait for that in order to start code reviews.

Phew! Sorry for the super long explanation. Thank you for your help. I'm very heartened by how many people have reached out and said how happy they are with the new stuff we're adding. A number of people have told me details about custom districts and scenarios they are working on, which makes it all worth it.

[instafluff0]

Brief update: I had to make a few changes to get the day-night cycle to work again. That's all done and good to go. I'm also done with all light annotations for everything but the last 2 districts' art, which I'm waiting for ZergMazter on.

So no further code changes planned, just art from here, and minor.