[Development] Code Rewrite

[Leoreth]

I have now officially started the code rewrite. In case you need context for this, it's best if you just review the plans for 1.17 thread. This basically covers all but the first point. I have branched off develop into the "rewrite" branch. It's not published yet and it makes little sense trying it out, because it will be either broken or largely indistinguishable in features from develop.

I won't give frequent updates here probably, because everything is very code focused and not that interesting to most people. As some small context for what I've started on first for those who have worked with the code before, here is what I've started with:

Handling lists of plots is one of the most common operations that a lot of functionalities require. Previously, this usually worked using pairs of tuples tTL and tBR and then using utils.getPlotList(). There are many more specialised functions for more specific use cases. Now you can do:

# get rectangle between (0,0) and (2,2)
print plots.start(0, 0).end(2, 2)
> [(0,0), (0,1), (0,2), (1,0), (1,1), (1,2), (2,0), (2,1), (2,2)]

# position of rectangle corners does not matter anymore
print plots.start(2, 2).end(0, 0)
> [(0,0), (0,1), (0,2), (1,0), (1,1), (1,2), (2,0), (2,1), (2,2)]

# get all plots on the map
plots.all()

# get plots in a region
plots.region(rMesopotamia)
plots.regions(rMesopotamia, rEgypt)

Also the plot list returned is a thin wrapper around the actual list, that is a bit easier to use:

# can check if plot, city or unit is in area
plot = gc.getMap().plot(1, 1)
city = plot.getPlotCity()
unit = gc.getPlayer(0).makeUnit(iSettler, 1, 1)

plots = plots.start(0, 0).end(2, 2)

print (1, 1) in plots
> True
print plot in plots
> True
print city in plots
> True
print unit in plots
> True

# iterating over the plot list returns plot objects immediately
for plot in plots: print plot.getX() + plot.getY()
> 0
> 1
> 2
> 1
> 2
> 3
> 2
> 3
> 4

 

[bluepotato]

There was some discussion in that thread about replacing scenario files with map scripts, and since I made something like that for my WIP RFC clone (which I will probably never finish) about a month ago, I thought it'd make sense to post it here. Sorry if this is the wrong thread.
Looks like @Jarlaxe Baenre made pretty good progress with placing all the stuff in dictionaries but didn't make a mapscript. I actually started with an existing one (I think it was some desert generator? not that it matters) and replaced every function with my implementation. I don't know regex so my wb save reader is probably less efficient (and has less features), but Jarlaxe's Mappifier script could be used for reading as well. Mine simply splits every line and puts everything into a single dictionary, not including units, cities, etc. Actually I'm not sure whether you could "generate" those or you'd need events. So the script only "works" with 3000BC.
I actually tested it and it generates the terrain perfectly (even the rivers!), but civs are completely messed up (=autoplay is broken, random civs replace others, probably because of randomized slots; dynamic spawns seem to work anyways, China spawned at 2070 BC after I've pressed end turn enough times with Egypt). If you want to try it, just put the attached Mapscript.py into the PrivateMaps folder of DoC. Also, you'll want to replace iDefaultPlayers with 49 in GameInfo/CIV4WorldInfo.xml (any world size works, I used Huge and removed all the others for convenience) or the whole thing CTDs. Then just erase all the settlers that spawn at 10,10 with WB and observe with ctrl+z.
So I guess this could be used as kind of a starting point for the mapscript, or just ignore it if not. I thought it could be useful, anyways.

 

[Leoreth]

Oh interesting. I thought I would simply plug in the pyWB scenario parser instead, but this is a good alternative example.

 

[Jarlaxe Baenre]

@bluepotato I wasn't able to get it to read the file within the mapscripts folder when I tried a few months ago. I took a look at yours, and modified it to work with mine. I haven't tested it, nor have I added the additional information (cities, culture, units, etc.) to be generated, but it's all available to be taken.

I kept yours in, and it's still used for the information I didn't create regex to grab (the initial constants—description, top and bottom latitude, etc.).

Lists that start with "lPlots" have an entry for every plot. Lists that start with "lCities" have an entry for every city, should someone try to add units, cities, etcetera to the mapscript.

I have not tested this, but I think it should work.

 

[Leoreth]

Just a quick update to let you know I am not inactive, I've actually worked a lot on this in the last two weeks. I'm about halfway through overhauling the RFCUtils module and have already simplified the code across the project, deleted a lot of dead code, and identified a couple of functions with shared functionality that could be combined. So it's going good at the moment.

 

[Temujin8k]

Hi, Leo. How are you doing with the code rewrite? I'm actually curious with your progress, if you'd mind to share with us. I for one am looking forward to testing out the new code

 

[Leoreth]

I am still working on it, you can always see the current state on the rewrite branch, or see the comparison here on github. It's now totaling on +8.5k -7k lines of code, so it's quite extensive. The main idea is still to move commonly repeated patterns to a new Core module to either replace existing RFCUtils code or boilerplate code that had to be coded over and over again, or that required some inside knowledge about how the game is set up.

I also added a lot of tests for the Core module, so we can be reasonably certain it does what it is supposed to, which wasn't the case for the old code. I found a lot of errors that way.

The I applied those changes back to the remaining code in RFCUtils, simplifying it further, and made some low hanging fruit improvements to the rest of the code base. Since a full rewrite of many of those modules is still coming up, I did not spend too much effort on completely perfecting that code, so there is still more to be gained. The line of code changes look like things are more complicated now, but most of that is the new Core module and its tests, which are not supposed to be touched during development. All other modules have significantly reduced in length and significantly improved in legibility.

Still, it took me some time to get the game running on the refactored code again, which I am doing now, to detect errors that are only apparent at runtime. I'm pretty confident that I can fix the last remaining of those in the next couple of days and then move on to other objectives. Right now I think it will be, in order:
- fix reported crashes and game breaking bugs (such as the stopped culture growth bug)
- another pass over the new map with the currently accumulated feedback
- return to this branch and implement periods for civilisations so I can remove the "reborn" attribute

If you can tolerate a few non-critical Python errors the rewrite branch is currently playable though.

 

[Temujin8k]

Nice! While I had been patiently waiting for the branch to be pushed for sometime now, I did not check within the past 5 days, so I didn't know you had already uploaded it. I'll try testing it in-game over the next few days/weeks and then give inputs and observations.

This is exciting! Can't wait to see this to completion. Thanks, Leo!

 

[Leoreth]

I'm glad to see you're excited about this. It's hardly relevant for most people since the gameplay remains unaffected.

That said, this is only the beginning, although certainly the most extensive change in scope. I have lots of additional changes coming as part of this project.

 

[Temujin8k]

I'm glad to see you're excited about this. It's hardly relevant for most people since the gameplay remains unaffected.

That said, this is only the beginning, although certainly the most extensive change in scope. I have lots of additional changes coming as part of this project.

While the gameplay itself remains unaffected, the whole refactoring could significantly improve performance and remove unnecessary code. It would actually be great if interested modders could actually read the whole code without encountering significant difficulties. It also allows easier implementation of new features that may be added in the future, especially with the Core module you made where you simplified the tedious things that appear multiple times like the CyPlayerContext() methods (e.g. getPlayer(), getMap()). Looking forward to the rewrite of RiseAndFall.py the most, since that one is admittedly a bit too messy.

 

[Leoreth]

That's right. It's admittedly probably a net zero on the performance end with the Core functions. For RiseAndFall, I wanna start deep in the DLL code for autoplay etc. and then rebuild on top of that, and also not just rewrite existing functionality but also expand on spawns a bit and get rid of some annoying mechanics like unit flips etc.

 

[Leoreth]

Quick status update: made a lot of progress recently. I introduced the idea of periods to change some civ attributes during the course of the game. Right now this is only used to change the core and historical areas and war maps of civilisations, e.g. to make Japan more aggressive with the Industrial era. Previously those changes had been implemented inappropriately and inflexibly using the reborn property. Now reborn is only used for civilisations that are actually reborn.

Periods could do a lot more for us in the future, such as changing modifiers and making determining dynamic names easier. I've also mentioned making use of this in the context of dynamic city naming. But for the time being I kept it limited to what is currently being done using reborn.

Now I'm roughly in the middle of changing all references from player slots to civilisation types. As you've heard me mention multiple times, this is the most important step to uncoupling civilisations from slots and to allow additional civilisations in the game. Right now, I won't be going as far, only making the preparations. It will allow us to get rid of the reborn property entirely though, because then you can distinguish whether its Persia or Iran in the Persia slot from its current civilisation type. Progress is surprisingly quick, which is great.

 

[Temujin8k]

Hi, Leo! I don't know if you accept bug reports related to the write branch, but I just would like to let you know that loading the 1700 AD scenario on Epic or Marathon yields the following Python exception error, which causes the game to be unplayable (everything is over 1 only). See following stack trace and screenshot for reference:

Traceback (most recent call last):
    File "BugEventManager", line 400, in _handleDefaultEvent
    File "CvRFCEventHandler", line 100, in onGameStart
    File "RiseAndFall", line 229, in setup
    File "RiseAndFall", line 266, in initScenario
    File "RiseAndFall", line 336, in adjust1700ADCulture
    File "RFCUtils", line 290, in convertPlotCulture
    File "Core", line 1007, in without
    File "Core", line 987, in where
    File "Core", line 1007, in lambda
    AttributeError: 'NoneType' object has no attribute 'getID'

Spoiler Screenshot :

Loading the 1700 AD scenario on Normal speed doesn't yield any Python errors, as well as loading the 3000 BC or 600 AD scenarios on any speed.

Just some discovery I'm sharing while tinkering around. I hope this helps in any way especially as you're still in the development process.

 

[Leoreth]

Ah, without, my familiar friend. Thank you for reporting this. I don't mind bug reports but as general expectation setting I expect playing on this branch to be rocky at the moment.

 

[Leoreth]

Update: I'm almost done completely getting rid of slot based constants in the Python code, it was a lot tougher than I thought to do this without making the code terrible, and I think it's in a good place now, even if there is still room for improvement. I also need to do the same for C++ but that should be a lot easier, because it has less custom code. I expect there are a lot of errors in that refactor that I need to catch, but with this out of the way I can finally get to some actual improvements to the code.

 

[Vectors]

Leoreth, would you be interested in starting a thread so people could start to discuss new civilizations, such as Renaissance Nordic Country (Vikings), Maratha (India), Iraq (Babylon), Mamlukes, Peru, Modern Greece, etc.?

 

[Leoreth]

There was one, I can't find it right now.

 

[Vectors]

There was one, I can't find it right now.

Is this the thread you were thinking of?
https://forums.civfanatics.com/threads/what-other-civs-would-you-like-to-see-added-to-doc.614314/

 

[Leoreth]

Yes.

 

[Leoreth]

Pushed another series of large commits. Those commits (almost) complete the transition from slot based code to civ based code, which is a pretty big milestone. You should be able to play on the rewrite branch without major issues, although there are still no benefits over develop gameplay wise.

With this, here's a list of what has been completed so far:
- added a suite of core functionalities that makes writing code for repeated tasks faster, easier and more elegant
- added tests for those functions to make sure the game always behaves correctly and predictably
- cleanup of some duplicate and unused code (not yet complete)
- civilisation periods (e.g. Austrian period for Holy Rome) largely replace the need for isReborn in the codebase, and are managed centrally in Periods.py
- replaced slot based logic (e.g. Egypt is always in the 0th slot) with civilisation based logic (i.e. Egypt is defined as the player having the Egyptian civilisation) in Python and C++

From there, I think I will continue with the following, roughly but not necessarily strictly in that order:
- completely remove the isReborn property from the game. Its purpose has been superceded by periods and civ based logic, but it's not yet entirely gone from the code
- add more utility code to help with turn based code. There are a lot of repeated checks around the current turn, like being a specific year, being in a specific range of turns, there might be (seeded) randomness involved, you have to account for game speed etc. This is easily to get wrong or forget a specific aspect, and you shouldn't have to think about this every time. Unfortunately this is surprisingly hard to do, or maybe I just haven't had a good idea yet.
- better event handling (i.e. code that is triggered when a specific event happens in the game, like city being founded or a tech being research). Currently all of this is centralised in the CvRFCEventHandler.py module, which is honestly a mess that is responsible for everything. Instead it should be possible to decentralise event handling to the module that is actually responsible for implementing what happens after an event.
- History module that contains everything around historically scripted events. Those are currently spread across the entire codebase and you can't really tell what is a generic game mechanic and what is a specific scripted event.
- (Maybe) improvements to the Victory code. There is a lot of repetition between goal checks and goal displays that can easily get out of sync, leading to incorrect displays, and make adding more goals more work than it should. Simplifying this would be nice for when more civs are added, and I have some ideas.
- Reimplementing autoplay. Rhye's autoplay implementation is very confusing and needlessly complicated, and it also prevents you from turning on autoplay for other reasons than autoplay from the beginning of the game until your initial spawn. I'd like to add the ability to just let the game running for a set number of turns (would be great for balancing and debugging too), or to let you choose to turn on autoplay until civ X spawns during an ongoing game. Either because you won with civ A and want to continue this world state, or because you lost. Losing and continuing with another civ (currently alive or upcoming) is another feature I'd like to try.
- New Rise and Fall rules. Better mechanics to protect newly spawned civs from being sniped (including the removal of unit respawns and defections), different ways to spawn depending on the historical situation (independence, settlement, conquest, expansion etc.). This comes with a complete replacement of the existing code, because I don't think everything around this doesn't need to be as complicated as it currently is.
- Directed barbarians: I have some ideas how to make barbarians spawned in a specific area not leave their assigned area, preferably target specific civs or cities in that area, and not spawn at all or disband if their targets are not around anymore. This is mostly to make barbarian spawns more responsive to what else is going on in the game, and especially make "excess" barbarians that roam the map because their designated target doesn't exist less likely. In turn, barbarians could scale with the strength of the civ they target to better account for player / AI discrepancies.
- Minor empires: currently all minor states are represented as independent cities and this would not change, but I could still regard certain cities as part of specific minor states in the code and give them corresponding benefits. This could include things like defensive unit spawns, and free buildings. This should both make certain independents less of a pushover, but also their cities less of a backwater in case you take it. Currently independent cities are mostly free to take but also garbage, and I'd like to address both of these issues.

I think that's all? As you can see, there's still a lot to do. I will very certainly take a break somewhere inbetween and return to the new map project and the open bug reports. It's also very likely that I will merge the rewrite branch back into develop at least once along the way, to get better feedback and also to avoid letting them diverge too much. The most likely place for that is before I start touching actual mechanical changes.

Let's see how it turns out. This is such a big lift and in retrospect it's one of the upsides of self isolation right now, because it's given me lots of free time to work on this that I wouldn't have had normally. I know the done / to do columns look very discouraging in comparison but the hardest part is already done now.