[Dev] 0.1 "Babylon" Progress Thread

Yeah, I'm not comfortable with the way things are. I had stalled on coding, anyway, but it feels like there is a logjam now, and I don't really have any feel for what's supposed to happen in a code review. We're all approaching problems differently in the first place, and I thought the chasing-shiny-things process had a pretty good velocity with no toe-stepping so far. I don't really feel like I should be telling anyone to do anything differently.

If I were to code, now I'm concerned about colliding with several PRs' code.

 

I've opened up my PR for the refactoring and full population of the Sav structs from native Civ 3 Sav files. With this in, there shouldn't be any need to have to use/write dummy data anymore. All the useful stuff- units, cities, citizens, tile improvements, etc- should be available for moving into C7GameData.

I realize this is contributing to the backlog problem, but I could switch gears now that this QueryCiv3 is mostly done into reviewing existing PRs. That is, if it's okay with you guys. I know I'm still new to the project, so I don't want to jump the gun and just start deciding what is and isn't valid without permission.

 

I'd be fine with you taking a look at some of the existing PRs. Having reviewed a couple of your PRs, your code quality seemed good to me, so I don't have any concerns about that.

We also definitely have a PR backlog issue. I agree with what Puppeteer said last year in post #141. In a way I'm surprised my enthusiasm lasted for over a month fairly consistently, but the ever-growing PR backlog combined with the holidays effectively sapped it. I've found myself dipping my toes back into previous projects (some Civ related, some not), both because I feel like I "should" be reviewing PRs if I do something C7 related, while also feeling like I'm the only one who has done a lot of that already; and also because I have two PRs of my own that are 18 days old, and that there's a good chance I'd want to built on either one of those or someone else's code, or both.

It could be worse. The test team adjacent to a team I worked on once had a complete tangle of branches that was a huge mess to sort out (I helped with the first part but didn't have the contextual knowledge to help with all of it), and the database team in the same area somehow wound up with 100 PRs to deal with, which was also a big problem. But I feel like just writing more code now would lead down that path.

Overall, I'm thinking the switch from "everyone merges their own things" to "all code goes through a review" was a failure at this stage, creating a logjam and contributing to the end the chasing-shiny-things velocity, and we should try a different balance. A few options I would propose:

- If we still say "there must be a review", you can review your own code, perhaps after a delay of X days so you're a little less biased in terms of "I just wrote it, it must be good", and to give others a chance to review it if they wish.
- Making reviews optional for those who have already contributed. Still have some checkpoints for the code of newcomers, and encourage reviews if you know something is complex/controversial/potentially still has some landmines that haven't been reviewed, but de-logjam-ize things.

I'm also reminded of my 2020 work team, where there was a problem with an imbalance of who was doing PR work. Some people did a lot, some did an average amount, some didn't volunteer much and did little. That team implemented a rotation system. But I'm not sure how you could do that where it's all on a volunteer, unpaid basis.

I have gone ahead and reviewed pcen's latest MR, since pcen is new (new enough that I don't know his CFC username), it was an improvement on code I'd written, and it was of moderate length. It also looked generally good, making the popup code much higher quality and adding the game menu (the one with load game, map options, retire, etc.). So the good news is we seem to be attracting quality code contributions. But I think we still need to decide on an alternate approach for January since the December approach, IMO, isn't working very well.

 

Thanks for letting me. I reviewed and approved your two oldest PRs, so those are ready for merging. I'll try to help out the reviews where I can, but there are some like the ImprovedErrorHandling one where I still don't feel familiar enough with what's going on, so I'll leave that one to you or someone else.

In terms of what to change, I think having every merge go through a pull request and review is still a good idea, because that adds some extra documentation and reflection on what's happening. But you're right that the current process is too slow. Having people be able to review their own pull requests after 5 days seems like a good balance. That should be plenty of time for others to be able to get a chance to look at what's being changed. And then I think there should also be a limit for how long a PR can sit in the approved state before someone comes along and merges it. For instance, the "Modified BuildDevSave" PR has been approved for 13 days now but still hasn't been merged. I think that after 2 days of inactivity, anyone should be allowed to merge an approved PR, even if they didn't work on or review it, just for the sake of getting changes into the main branch faster.

 

Well put.

I've been half-inclined to post the past week or so as the PR logjam appears to be a complete showstopper now, but I don't really have any solutions to offer, so I'm afraid I'd just make things worse by saying anything more about it.

I guess one of my paranoid thoughts is that newer-to-the-project coders–or even original coders–may misinterpret me. I'm afraid I may be seen as one of the "original 3 or 4" coders and somehow have special status because of that. I don't, though, certainly not in any authoritative manner. I am a noisy, thinking-out-loud short-attention-span jack-of-all-trades, master-of-none who has coded almost entirely in my own self-interest. C7 would not have even started if it were me alone, and it would never be finished if I were an authority figure in it. It would be like c3sat, otda3, or cia3, an idea that sporadically radically changes forms and purpose and never does any one useful thing well for any period of time. It would be just another playground for me.

In fact, I still have some selfish ideas for the QueryCiv3 code and maybe ConvertCiv3Media libraries outside of C7. Not that my selfish ideas should impact C7; the original code came from one of my repos, and the modified code is MIT-licensed, so I can always fork it back if my selfish wants diverge from C7 utility. (I mean fork as in the libraries; I'm not proposing to fork C7 as a whole. There are selfish reasons I made those as libraries in the first place.)

I am thrilled for any of my past, present, or future work to be or have been a part of C7, but C7 in no way should be about me or my ideas in particular. So if anyone is ever concerned that my feelings, opinions, or ideas are blocking C7 progress, please know that they are not, or at the very least should not.

So this PR logjam is definitely not about new coders entering the project or their code quality. It's about the challenge of volunteer labor versus the need for a project to have process and quality controls. And again I have no solution unless we can find a volunteer who has a code review fetish.

Editing to add: I am also not saying that I am not coding for C7 right now because of the situation. I'm not coding for C7 right now because I'm chasing other shiny things, some of which may eventually find their way back into C7's future. But I do suspect the PR logjam is a blocker to new contributions for interested parties.

 

Let's try this for January unless and see how it goes.

I've thought about the same basic concern - being misinterpreted. In this project, mainly by being too active, perceived as too authoritative. In general, I prefer in person interaction because I can see people's reaction, and thus quickly get a sense of whether I'm going too far, or failing to make a point clearly, or something else entirely. And then I can adjust if need be. The more I know someone already, the less of a concern this is, and it does help that some of the people here have been around CFC for years... but it's still a different sort of project than I've been on before.

The other key contributing factor is that the "Project Ringleader" as I termed him in the Credits scene, WildWeazel, has had limited availability/forum presence since the start of November. Is WildWeazel our benevolent dictator for life? Do we make decisions by vote? Are we a democracy, monarchy, feudalism, or anarchy? I'm not really sure. But I've occasionally tried to set a direction (see above, but also things like the Babylon/Carthage threads to keep the overall goal visible), because it seems like a better idea than not to do so. And also because I kind of dread the vote-tallying when we don't necessarily having a voting system, a defined electorate, a time limit on voting, any agreement on what's legitimate, etc. The analogy that comes to mind is that if WildWeazel is the king, then in his absence there should be a deputy, but as there isn't a specific one I've tried to fill the void. A kind of Ned Stark role from Game of Thrones, if you will. But if you've watched the show or read the books, you'll know that with that sort of role should come a healthy dose of paranoia about the reaction to one's decisions. Both on a web forum and in a palace intrigue, you only see the reactions people choose to share.

*shrug* It's probably natural to be concerned about possible misinterpretation; at least it shows some self-awareness. I've wondered in the past if some of the more... abrasive... well-known open source contributors in the world consider how they come across at all. In "real life", by which I mean in-person, I tend to be more cautious until I gain confidence, and gradually become more outspoken as both I have confidence and see that others have confidence in me. I suppose to some extent that has happened here (I definitely wasn't confident or making suggestions in the spring, or until I started to become familiar with Godot), but I do wonder if I'm tapping into previous work confidence too much, and acting too much on what I perceive as beneficial with insufficient feedback. Mainly the insufficient feedback being a difference compared to what I'm used to; that was also a significant problem when I was working remotely in 2020-2021. Though it feels like less of a problem here; others are also posting here, whereas as the new person on a large team in mid-2020, everyone already had their cliques and it was tough to break in to any of them without in-person interaction.

 

Quoting from the other thread, the parts that are most relevant here.

I still think it's important to figure out Babylon, even if we may be re-evaluating future plans a bit. If my understanding is right, we've made a lot of progress towards this milestone. Items in it as defined in "The Plan" (yes, I know it's not supposed to be gospel... but it's the only book we have so far).

• World map
• Map visibility
• Prototype mods
• Movable units
• Place cities
• Prototype screen navigation

I'll try to break these down more to help define what's left:

World Map
This does need some more definition in terms of what we do now/soon, versus later. I'll list things that I think do or could fall into this here, with the ones that I believe are done being crossed out:

• Show base terrain
• Show forest/jungles/marsh
• Show hills/mountains/volcanos
• Show barbarian camps
• Show cities
• Show rivers
• Show goody huts
• Show resources
• Show all types of cities, not just one type of graphic where they are on the map
• Show forts/airfields/radar towers/etc.

I've tried to list these in rough order of what I think is most likely to count for Babylon. If I had to define, "where's the line?" I'd put it below "Show rivers".

Map Visibility

This could include:

• The map is not visible until you explore it
• Fog of war (the map is only fully visible with line of sight)

This is one where I personally wonder if it's not prioritized a bit too early; I'd be in favor of having more combat and unit mechanics in place before prioritizing visibility. But its scope will need to be defined regardless.

Prototype Mods
I'm not really sure what was envisioned for this. I believe I'm largely in agreement with Ozymandias that it's a bit too early to be worried about mods, although I suspect the intention here was making sure we didn't code ourselves out of the ability to add mods without realizing it. Maybe this is just trying out some ideas? I'm not going to try to break this one down myself.

Movable Units
We technically have this at the basic level, but nonetheless it can be broken down:

• Moving units via Num Pad
• Terrain cost factored into movement
• Moving units via the mouse
• Moving units via the mouse a longer distance and having them make it to their destination on a subsequent turn.
• Land/sea difference is factored into movement
• Road cost is factored into movement
• River cost is factored into movement

I could see good arguments for anything down through Land/Sea being included here, but would argue road/river should be part of a worker action/terrain improvement feature set.

Place Cities
We might actually be good to go on this one. I can't think of anything missing in terms of being able to place cities with settlers.

Prototype Screen Navigation
What does this include?

• Moving from the main menu to the game and back
• Pop-ups being integrated and dismissable
• Advisor screens (not really implemented as a whole feature, but you can navigate)

I believe this one is probably checked-off too, but maybe I'm missing something that was intended here.

--------------

My proposal is that we figure out where we want the lines for the "final" Babylon release, but I'd also encourage what @WildWeazel has been calling the "pre" release, corresponding largely to what we have now, which is a significant jump from "Aztec". I'll suggest "Assyria" as a possible name, or perhaps "Nineveh", the Assyrian city that the Babylonians helped destroy in 612 BC, and which is second in the Babylonian city list in Civ3.

Once we've figured out where the lines are, we can also put things in the appropriate place in GitHub.

 

I didn't design the milestone, but in my mind it would be nice along the way to toy with exposing some functionality to Lua. I briefly tried to do that with the Civ3-to-C7 save conversion but quickly realized there is no advantage in doing so. (I initially thought it would allow rapid changes without rebuilding the project while also making JSON manipulation easier, but it turned out C# does JSON really well, and I just wound up serializing/deserializing MapData as an interim "save format".)

If there is anything else that might be exposed to Lua for fun and profit proof right now, perhaps unit actions might be optionally scriptable. e.g. Build city might have a scriptable handle whose script might adjust the initial pop count, and in the future bin counts and civ-specific free buildings. But it's not particularly important to try that out now. Just a concept of how and where that kind of thing might be done.

 

If you'd like to do a proof of concept, go ahead! I'll admit that this isn't an area that's especially appealing to me, given my current knowledge of the area, so I'm not likely to be the one jumping on it.

Though as you say, "it's not particularly important right now", which goes along with why I'm curious what WildWeazel's thoughts were on including it in such an early milestone.

By the way, we're down to 3 PRs now, one of which @Flintlock will likely be merging tomorrow as it's now Windows-battle-hardened. I'm finally feeling like I can probably add some code without just causing more conflict headaches.

 

(@WildWeazel see post #147 for a breakdown of Babylon features, as best I understand them)

Good news, we're down to two PRs. @Caro-Kann , I left one maybe important comment on PR #70, and two general-commentary ones that aren't concerning. I'd like to get it merged sooner rather than later; I found I do have code for drawing rivers locally, but it made some changes to improve tiles, and PR 70 has more substantial changes for the same purpose, so they conflict and I'd rather have 70 merged before re-applying my now-outdated river changes.

I'm also hoping to implement issue #58, to separate out terrain identifiers from their display names. This will avoid a lot of pain going forward; for example I noticed yesterday that Mountains weren't displaying because with our improvements over the past 1.5 months, their name was now properly "Mountains" rather than "Mountain", and the isHilly method checked for the display name of "Mountain". But that one probably also is best done after 70 is merged. So I'll probably play some Civ VI instead or something like that.

------------

More generally, I think this illustrates the problem of having a formal review processes without having anyone who is passionate about, or financially compensated for, performing code reviews. It took 1.5 months to resolve this logjam, it appears all of us wandered off to work on other projects of interest in the meantime, and if it happens again, I'm not sure I'll be able to be persuaded to return to active development on the project.

 

So, instead of playing Civ VI, I baked some cookies, and after baking cookies I wound up writing some more code. Focusing on starting to differentiate between water and land, and making a few improvements to barbarians.

This screenshot tells the story well:



Now if that isn't a scary sight! Barbarians have been trained to keep their warriors and camps on land, and have also learned how to build Galley, if they are on the coast. For testing purposes I turned the chance of galley creation each turn up to 68% for a coastal barbarian camp, but I plan to set the default to 3% for non-testing purposes.

I also gave the barbarians some hitpoints, so they aren't perpetually in the red zone for their health bar. Yes, they should be scarier!

Next I'll probably add restrictions to player units walking on water. I might also play around with the barbarians a bit more. I've added a separate barbarian camp list to the Map, so we don't have to check every single tile to see if there's a barbarian camp when spawning barbarians between turns, but I'm quite tempted to add another barbarian AI class or two, that's a bit more interesting than the random one.

My general goal with these changes is to continue to make things a little more interesting, while having parts of the code that might live long-term (e.g. the sea/land differentiation). Firing up the Aztec release, the world already feels considerably more alive with unit animation, forests/marshes/jungles, hills/mountains, and barbarians that move around. But we aren't quite at the point of having actual gameplay yet.

-----

Edit: These changes are now in PR #76 (including player units only being able to walk on land). This isn't everything I want to do in this area, but by opening the PR now it stays a reasonable size that might not deter all reviewers, and I can still branch off of the branch.

-----

Edit 2: I've also started revamping UnitPrototype so that the prototypes are actually used instead of being re-created as duplicates every time something is produced. (In a branch, UnitPrototypeRevamp) UnitPrototype now implements an IProducable interface that is meant to represents things that a city can build... units, buildings, maybe other things eventually. The refactor isn't quite done yet, but barbarian and city production is now using prototypes.

As part of that, I've also given coastal human cities a 1/3 chance to build a Galley instead of a Chariot. These are land-based galleys for now, but soon will sail on coasts instead of plains.

This is starting to point towards some future doors to open. One of them is an AI component for choosing what is built next. Another is a native C7 specification for unit prototypes. Currently they're all just specified in code, but that's obviously placeholder.

 

I made a PR. I finally returned to the civ3 location picker, and having a static field did the trick. The "button" needs some styling, and somewhere along the lines the no-graphics text seems to have gone off-center, but I figured we can pretty it up later. The select home button does hide if it can successfully find the graphics and draw the main menu screen.

I looked at the hated GlobalSingleton. It's all killable using the same tactic, but I decided not to do that in this PR.

 

I left a couple comments on the PR! Hopefully at least somewhat helpful ones; I'm also rebuilding my Linux VM to be less slow than it was before so I can hopefully fire that up to test with, without being annoyed by how laggy its UI is in a VM. Switching from GNOME to MATE as the display manager; I noticed XFCE was a lot snappier than GNOME when working on another project the other day and am hoping maybe MATE is a happy medium in eye candy. But if not, I'm installing Mint, so I can swap it to XFCE as well.

I see that my PRs have also garnered a few comments! Maybe the smaller-PRs option is going to work... which also means I'll have another PR not too long after addressing comments on the current ones.

That following PR will be from what I've done in starting to add some basic other players. I realized that additional players weren't specified anywhere else in the proposal document, but we probably don't want to wait until everything is done to add them. For one because we're more likely to garner play-testers if there are some non-barbarian opponents to make things interesting, but also because we can start experimenting with teaching the other players how to utilize the existing mechanics when there aren't that many existing mechanics. Starting to add players has also resulted in addressing some tech debt, such as the hack where the only reason the player couldn't move barbarian units is that they were fortified. With the pending Player changes, all units are properly assigned to a player, and the unit auto-selection picker will only return your units.

More importantly, it feels like some progress is being made again, and it's also enjoyable again.

 

Quintillus... Glad you have some new found enjoyment

By the way... what does "PR" mean? Guess I am getting old but Abbreviations start messing with my mind as I am Not up on what others think is known by everyone

 

Ah yes, that is not only an acronym, but a programmer one at that! PR = Pull Request; synonym Merge Request which has the acronym MR. A merge request is used to request that one's updates to a code base, in this case C7, be added to the main copy, or in other words be merged in. The site we host the code on calls them pull requests instead of merge requests, although having used both on various projects I think the merge terminology is the one that makes more sense. I suppose the best analogy for pull would be that a bunch of us are thinking up lyrics for a song, and one of us has the pen for the master copy, and someone says, "I thought of this cool new lyrics! *recites lyric* Any way we can pull that into the song?"

Good to see you active on the forums; hope you are doing well! It's been a while since I saw you post here, although that may have something to do with having been absent myself for part of January.

 

I've been having some fun teaching the AI how to use Settlers. The Greeks, it seem, are fast learners:



Athens was their default starting location, but they decided to build Sparta there by themselves. Compared to the mountains, plains, and deserts to the north, I'll take their choice, even if they're likely over-valuing the forests. Most of all, I'm happy that they were able to choose a site based on some criteria, and successfully send their Settler there and build the city.

This is the first non-random AI behavior, instead being driven by a reading of the map near their existing territory.

I'll probably play around with this a bit more tomorrow; notably I need to teach the Greeks not to send their second settler to the same location as the first one. But with a bit of luck, before long we'll have the AI building cities across their home continent.

 

Quintillus... I've been around daily and posted a few times but basically just recovering from eye surgeries.

I agree with the Greeks choice for Sparta... access to the Coast was a Good decision

 

The AI has been trained to prefer coasts, assuming things are otherwise similar. It will also prefer hills, for the defensive bonus.

The results of the latest session... the AI has discovered infinite city sprawl (ICS)!



What's interesting is that I didn't teach it ICS, it figured out it could do that on its own. I taught it to use CxxC or CxxxC spacing when deciding where to place a new city. But it realized that if it used CxxxC spacing, it could put another city in the middle later on while still complying with the letter of what I had taught it, and without breaking the "too close to another city" rule.

We probably don't want it using CxC spacing by default, but it's an example of something that could be enabled if you wanted. Maybe it's even an option that the AI will on rare occasion choose to keep you on your toes. 20K temples-everywhere, Feudalism Pikeman swarm strategy for a Religious civilization that gets cheap temples. Or maybe if the human player starts using CxC, some of the AI will follow suit so as not to be left at a competitive disadvantage. Not saying we should go that route, but it's a possibility.

But before that point, I think I'll teach the AI to defend its cities.

 

I've been going in a few different directions as I run into things that could use a fresh coat of paint... or maybe the wooden boards that can have paint applied in the first place. Several of these sprung from going back and dusting off the River code I'd started in December, which is itself more or less ready for yet another PR. But a few other branches spring from it.

https://github.com/C7-Game/Prototype/pull/82 <-- Fixes non-square maps again. They got broken when SAV-slurping was added, but the root cause had been there before; we apparently had been working around it before. Two-line change.
https://github.com/C7-Game/Prototype/pull/84/files <-- Allows opening BIQ files, part 1 of ???, 71 lines. This was helpful in testing river code as I have a lot of readily-accessible BIQs; I also think it's important to support BIQs as scenarios are more likely to be starting points than existing saves, IMO. Still follow-up to do on main menu work ("Load scenario" should default to this right folder), but this gets the basics in-place.

I may well open one for rivers, too... I'd rather review a bunch of small ones than one big one, so I try to favor that when opening them as well.

 

Recently I've been working on a basic implementation of unit combat. I'll submit a PR for it soon since I'd like to see it applied quickly after that PR by @Quintillus adding AI players to the game. It should be fun to watch the AIs fight among themselves and against the barbs. I branched unit combat off of my EngineThreadedArch branch, b/c I wanted to do combat animations, so it's going to make for another thousand line PR. Hopefully it won't take a month to merge, if no one feels like looking deeply into it then a quick look-over will suffice. (Presumably you've all already seen it but I wrote a high level description of the threaded arch here: https://forums.civfanatics.com/threads/architecture-thread.674254/#post-16206357.) It's more important to keep the project moving than to go over every addition with a fine-tooth comb.

On that note, I skimmed over Quintillus' basic opponents PR. It looks fine to me, I think it's okay to just merge it in. The only thing that stood out to me is stylistic: the name of the "distanceToOtherTile" method is too long for my taste. I would have named that method "distanceTo". I prefer not to repeat things in method names that are already specified by the names & types of the arguments.

So what do you guys want to do for Babylon? IMO adding animations, AI players, and combat is enough to justify putting out another release. According to the projects page we're still missing map visibility and prototype mods. I could quickly add map visibility, at least at a basic level, but I don't think it makes sense to bother with that yet. That's the sort of feature we'd leave disabled while testing, which is all the time right now. I also think we should push prototype mods to a future release. Mods will raise a lot of architectural questions and I fear we'll delay Babylon for weeks while we talk about those, and still not have a definitive answer at the end of it.