It wouldn't be possible to move the "fight" method into the game data module since it depends on "animate", which depends on EngineStorage. That's not a problem since we wouldn't want to move it there anyway, but a couple of months ago I stumbled over an annoying case where I wanted to put a method in the data module but couldn't because of EngineStorage. I forget the details but as I recall it needed to call tileAt but couldn't b/c it didn't have access to the actual game map. It struck me as weird and awkward that game data has complete access to tile objects but not the map instance that contains them, so it can work with tiles but only if it gets a reference to them from the outside.
These issues also tie in with how we're going to allow for mods. To re-use my previous example, we can allow for limited railroad movement, among other things, by enabling modders to override some method that determines the cost of moving a unit between tiles. If that happens to be an extension method in the engine, then that may be a problem since as far as I know there's no convenient way to override extension methods on C#. Alternatively, if it's a class method, the standard approach would be to make the method virtual and then allow modders to create a subclass that overrides it. The problem with that is it's very coarse-grained, e.g. if that method is in MapUnit then the entire unit class would have to be replaced to implement that one change and that would conflict with any other mods that modify the unit methods somehow.
I don't what the best approach is. These are just things I've been thinking about for the past few months. I read what you said about separating out the game data module so utilities can work with it without pulling in the entire engine as a dependency, and that makes sense, but I'm skeptical that it's worth the trouble. Do you have some specific utilities in mind? The only one I can think of is CivAssist, but I think we should try to make our mod framework robust enough that that could be implemented as a mod instead of an external program.
CivAssist, scenario editors (which could probably be done as a module), other ones that were popular over the years include a map seed generator (e.g. for Moonsinger's milkruns) and Puppeteer's Civ3 Show-and-Tell. I'm not really sure how much of MapStat and CrpSuite aren't duplicated by later programs, as I haven't used those extensively. Although it does occur to me that simply by having out data be relatively easily-consumable JSON instead of binary like Civ's, it will be way easier for whoever wants to, to vacuum it up into whatever language they prefer to work with.
We might yet have some gaps in how the data can reach other parts of the data - I don't think it should be necessary for it to ever call EngineStorage. It probably isn't tileAt as that's on GameMap.cs, which is in data... although perhaps the case you saw was where some other part of the data wanted to call it, but couldn't reach GameMap, so it went via EngineStorage? Although as I think about it more, since C7GameData is a dependency of C7Engine, it shouldn't be possible for that to happen... and indeed a search for EngineStorage turns up no results within C7GameData.
Still though, I wouldn't be surprised if we had some awkward logic that's at a higher level than feels appropriate because it goes across a couple different parts of the C7GameData. I've run into that sort of thing in my editor, too.
Thinking about the railroad example... it's fundamentally going to have to be a method that takes two tiles, as the cost depends on the improvements (and possibly base terrain, and possible technology [bridges]) of the two tiles. You're right that if it's done as a virtual method, then if two mods both try to modify it, it's a "last one wins" situation. It's probably impossible to fully avoid that type of situation, which is probably why some Factorio mods (for example) are not compatible with other Factorio mods. But it also has me thinking about what's the appropriate level of modification? My gut feeling is that it would make more sense to make the components of that modification modifiable, than to make it possible to re-write the whole thing. Let the user set how far rails should allow the unit to move (e.g. it costs 1/10th of a movement point), rather than let them re-write the whole method (which will be relatively complicated). Maybe they still can modify a whole TransportCosts class if they want to... but that would be akin to C++ DLL modding in Civ4, whereas just modifying the rail cost would be akin to the (much simpler) XML modding in Civ4.
tl;dr, I think it's wiser to make things easy to change in mods than to allow maximum power in making changes in mods. Factorio has demonstrated this to a fair degree; they've gradually made more things modifiable in response to demand for more aspects to be modifiable, rather than trying to predict too much up-front. Although this probably isn't a surprising point of view considering I'm someone who wrote an editor to make it easier to mode a game that's
already probably the best in its series in terms of ease of modding.
Good to know that extension methods aren't overridable. I hadn't been aware of that. I wonder if it would make more sense to favor service classes that are overridable in some cases, for that reason? Though again, I'd favor a judicious approach based on expected demand.
I expect we'll try a few things, and some will work better than others, maybe in some cases approach C works best and in others approach D works best. I don't really know what the best approach is either.
In my personal vision of how things should be, the AI, local player, and remote players all use the same interface to make things–such as fight–happen.
For a proper game UI, then–depending on settings–the fight has to wait for the animation, but perhaps this could be accomplished by a messaging bus or callback function, the latter optionally passed to the fight call. I suppose a simple event listener could be worked in there somewhere along with a timeout so the game does something if the animation never signals it's finished.
I agree (in theory, not necessarily in practice) that all of those types of players should use the same interface. The asterisk on "in practice" since I'm not sure how feasible that will be.
I should probably read more of Flintlock's PR to more accurately say how closely it matches what you describe so far - I've only really read one side of it - but I'm torn in part because from a readability standpoint, what he has looks good now. Whether it will survive future additions intact - not just things like possible eventual remote players, but closer things like tile visibility - I don't know.