Move terrain requirements from improvements to build infos

[Nightinggale]

Modding or adding an improvement is not intuitive as it is right now. Setting up xml might seem easy, but then you can go "I just built it on peaks and i thought I couldn't do that". We need a much simpler and direct xml interface.

We should remove requirements from improvement infos and add them to build infos. This way we can have two builds, one for flatlands and one for hills. Pioneers will then use a one hot system meaning the player won't notice which one is used. We can then turn each build on/off using CivEffects meaning we can do something like unlocking an improvement on peaks during the game. Since requirements are in build info, non-improvement tasks like build route can get requirements too meaning we can make level 1 roads allowed on peaks, but not faster roads or faster can be unlocked during the game.

The following should be possible to set in build info

bools:

• allow flatland
• allow hills
• allow peaks
• allow large river
• allow coast
• allow ocean
• requires river

lists: (empty means all)

• required terrain
• required feature
• required route
• required improvement

If it turns out that we need improvements to know allowed data, we can generate it on xml load by looping through builds.

Technically the lists should be read using InfoArrays and then unless empty they should be copied into EnumMaps of type bool. The EnumMaps defaults to 1, hence allow all when empty. Empty EnumMaps also allows none, like empty required features will allow building with no feature.

---

Maybe remove the code for clearing features and instead let the game pick the clearing build too. This way the amount of yields from clearing will stay consistent because there is only one clearing forest build where it's defined.

Looping lots of pointless builds during gameplay isn't a good idea. If something like goodie huts needs a bunch of build infos to work, then maybe some other solution would be good, like adding a "setup" build xml file, which only has the info for setting requirements. This way units won't loop build infos, which no unit can use.

Alternatively we can use some EnumTypeCacheArray. This will generate a list of build info IDs, which can be used by at least one unit/profession and loop that one. Come to think of it, maybe we should make one for each unit/profession and generating the list of possible builds a unit can make on a specific plot will skip checking builds the unit can't do anyway. Perhaps this would make sense to add to profession info. The check would then be get EnumTypeCacheArray from profession info, loop it and start by checking each against CvPlayer::canUseBuild(), which is an array lookup, almost instant.

[raystuttgart]

Guys, this again sounds like a coding change will little to no gameplay purpose. :( Where is the use case for that?

Recently we tend to want to implement everything we can technically but without any actual need.

It is again unnecessary effort to rebuild a proven in use system that every XML modder knows how it currently works. Thus it is also again technical risk and trouble to change all the affected XMLs.

Yes it is possible. Yes it might have been smarter to originally do it like that.

But that does not mean it is a good idea to redesign it now if there is no game play concept that justifies it ! Please, stop coding if there is no need from gameplay perspective, AI, performance, ....

Show me a concept (gameplay) or give me a real reason (performance, AI, ...) for this and I will support it. Otherwise "Never touch a running system!".

We do not have to rebuild just because it is "better design". This alone does not suffice. Again, such rebuilds come with effort, risks and potential side effects - even if it just teaching old modders the new system.

All I always hear is: We could maybe"" then in the future maybe if somebody - who never learned the current system - might maybe** want it ...

But what really matters: What are the use cases we have now and what do the current modders want for creating their features and improvements. What does it help you and me in our current and planned work to rebuild this system?

Summary:

First "Gameplay Concept" or "Technical Reason", then "changes" where needed. Not completely turning the logical order around and changing just for fun without purpose.

Proper "Product Design" (which a game is as well) is working this way:

• Functional Need --> Technical Solution

This here is just wasting wasting effort:

• Technical Possibilities --> Changing Functionality for fun or future eventualities that may never come

Please also think about the side effects of your changes for other modders actually using those XMLs. Especially if the current system is working fine and nobody ever complained.

It is not the technological possibilities that dictate the changes made. It is the actual need (gameplay, performance, AI, ...).

We are extremely fast in operating the patient sometimes ... But again, if you give me an actual real existing reason we have right now, I will shut up.

---

By the way, the claim "Modding or adding Improvements is not intuitive" may be right for an absolute beginner in modding improvements in Civ4Col. But the way it is currently working is 100% intuitive for every modder like Schmiddie and myself which have been doing it like this for the last 13 years ....

And it is really not that difficult and has always delivered all funtionality we needed ...

Again, I am not against change in general. But I am against change that does not have any real reason other than thinking the design could have been made better.

[Nightinggale]

The main issue is that I get questions about this in discord as people mod xml and then ask why it isn't working and it's because of some random hardcoding in the dll, which nobody around knows about. We even had that with large rivers where the question was why river crossings could only be built inside your owned land (well fixed now).

And you are right. It's not the most urgent task imaginable. Still I do believe it would be wise to make a decision on this as it answers the question of what to do the next time an issue is encountered: more hardcoding or adding a tag and if adding a tag, where to add it.

Please also think about the side effects of your changes for other modders actually using those XMLs. Especially if the current system is working fine and nobody ever complained.

Well I did receive a complain in discord. Or rather "I can't figure out why this isn't working" and then I had to use a debugger to figure out the answer because I didn't know it either.

[raystuttgart]

Sure, sometimes things are hard to know if you never did it. And yes, sometimes you have completely new Use Cases and they may not work at first.

But again:

Once you really have that Use Case (in WTP) we may think about how to make it work. Just not knowing how something currently works is not enough to completely recode so you may know. Also think about the effort to "migrate" all existing configuration to the new system.

This here myself and probably all XML modders intensively working with Improvements in Civ4BTS will most likely reject: "Modding or adding an improvement is not intuitive as it is right now."

• Right now Civ4BTS modders and ourselves can easily exchange and answer questions of each other.
• Right now you can simply look into XML guides or XML references of Civ4BTS.
• Right now you can simply ask modders like myself that did Improvements for more than 10 years.

Summary:

Seriously, there is currently absolutely no reason to rebuild a system that does everything we want. Once that is no longer the case and we can not easily make it do what we want (with much less effort than rebuilding) we can discuss this again.

Expanding / modifying the current system is most likely much easier as well, than completely rebuilding it. (And it will be much less risk and also much less effort to migrate current config.)

[Nightinggale]

I still think we should keep this open for 2 reasons:

• Design goal used whenever somebody has to change something. It should then be changed towards this goal.
• This design is faster, hence faster AI.

[inaiwae]

I guess I am the person who originally talked to Nightinggale

the use case to me would be to move away from hard-coded parts unless absolutely necessary, as this is not about not knowing how things work in XML, it is about things not working in XML at all because regardless of XML TAG being flipped to "1" DLL hard-code prevents the behavior or is simply broken by changing basic attribute defined in XML

of course, changing the structure of XML by itself doesn't fix the code for hard-coding instances, but as a design goal, I think Nightinggale's idea is valid ... although I agree that to change XML structure extensively may not be the way, adding XML support files for special cases might be the way

I was under impression that with such an extensive XML infrastructure there is very little hard-coded stuff, but unfortunately I am finding a lot of it, I already mentioned to Nightinggale very non-intuitive hard-coding about Promotion system ... I think the viable way would be to throw away all hard-coding and create new PromotionSpecialRules.XML that handles what is in the code

another example, I recently came across part of the code where Carriage limit to Plastered Roads is in the code (also the way how it is identified, if (getUnitInfo().getCargoSpace() == 6), is very prone to be broken by simple XML change of cargo space) ... things like these should be all moved to XML files and I think that's what Nightinggale is trying to accomplish

[Nightinggale]

although I agree that to change XML structure extensively may not be the way, adding XML support files for special cases might be the way

Yeah all or nothing changes tend to be massive and could take a while to implement. Instead let's look at the original issue: peaks. Add must be peak/hills/flat and if all 3 are false, set all 3 to true. This way if nothing is changed in xml, nothing changes ingame. Remove the "this specific improvement is hardcoded to avoid peaks" and then set that specific one to hills and flatland.

With this kind of thinking a system like this can be implemented step by step as issues are encountered. The bad part about such an approach is that it will be impossible to do a performance measure of before and after, hence we would lack knowledge of how much faster the speed boost would be.

another example, I recently came across part of the code where Carriage limit to Plastered Roads is in the code (also the way how it is identified, if (getUnitInfo().getCargoSpace() == 6), is very prone to be broken by simple XML change of cargo space) ... things like these should be all moved to XML files and I think that's what Nightinggale is trying to accomplish

Off topic for BuildInfo/improvements, but good example. The unit can go offroad if somebody decides it should have a cargo space of 5, though there is no indication of this when editing xml or in the game. I will however say that this is one of the most important functions in the entire game when it comes to how much CPU time the AI uses and as such every single change should really consider performance. It's one of the few places in the code where there could be strong arguments for hardcoding.

[raystuttgart]

@Nightinggale

I am perfectly fine with removing old hardcoding and transforming it into an XML attribute. :) That is a totally different category of change than what this discussion was started.

If you get confused by things not working just ask me. I am the one who originally coded most - almost all - of that stuff. :)

I simply coded very specifically for "my mod" and would of course do some things differently now. In these days, RaR however was simply not meant to become the mother of all Colonization mods.

Even if I still doubt we will ever have more than tiny private taste modmods, I do support to become more XML friendly. What I do not want is radical change of working systems without necessity that causes more negative side effects than having benefits.

And please do not forget, I often hardcoded for performance reasons as well. (Reading from XML is slow.) And also coding too many XML attributes in the big XML core systems that are only used by only one Unit / one feature will at some point make the XML extremely confusing. (I only added XML tags for really generic stuff not things I would never need again in XML.)

But again, the world in RaR was different than in WTP. I accept that.

---

Again, please do not get too radical with this old modder. He still needs to feel comfortable in his old mod to be able to code new features efficiently.

[inaiwae]

Off topic for BuildInfo/improvements, but good example. The unit can go offroad if somebody decides it should have a cargo space of 5, though there is no indication of this when editing xml or in the game. I will however say that this is one of the most important functions in the entire game when it comes to how much CPU time the AI uses and as such every single change should really consider performance. It's one of the few places in the code where there could be strong arguments for hardcoding.

if I can continue to be off topic a bit more here :)

also risking that I am stating something obvious, or worse, not doable ... is there a way for certain important values to be kept in XML or some other type of plain text file, that would be read at the start of the game, fed to memory and/or internal variables and read from there during code execution, instead of from XML file every time ? that way things can be still fairly simple to read and mod and wouldn't cause too much strain on CPU execution and/or disk access time

[Nightinggale]

all xml data is read at startup and stored in memory. There is an issue with that though and that is all Type tags are stored in an unordered map. Some code looks it up from that map directly (like GC.getDefinedINT) and reading from such a large unordered map is dead slow, at least with the map implementation used in our compiler. I added CvGlobals::postXMLLoad() to have a single place to store data in memory in a fast to access approach.

[Nightinggale]

I have been thinking about this. This can be implemented without major xml updates.

CvBuildInfo should have 3 InfoArrays, telling allowed PlotTypes, FeatureTypes and TerrainTypes. On xml load, store the arrays in enum EnumMaps as this is the fastest option for random access (read: checking if it works with the data for a specific plot) If the InfoArray is empty, set all bools to true. Check the 3 EnumMaps at the start of CvPlot::canBuild

---

That's it. CvBuildInfo will then have the ability to restrict builds in xml and it's added without messing with the existing xml data. In fact this can be added without touching the xml files entirely.

It's better than that though. After loading the xml files, loop CvBuildInfos again. This time check resulting improvement. Check the terrain requirements. Any terrain, which the improvement can't be placed on should be set to false in the terrain EnumMap. Now we can remove the check for terrain for CvImprovementInfo because we know it's true if it passes the EnumMap check.

We can continue this approach with resulting feature, required feature, isWater and so on. This way the EnumMaps will start to work like caches, which in turn boosts performance.

---

Allowing CvBuildInfo to be more restrictive than the resulting improvement/route/feature/whatever allows us to make two buildinfos, which does the same, except one is for flatland and the other is for hills. The game will then view those two as "one hot", hence only one will show up. CivEffects can enable/disable individual buildinfos, meaning we can do something like an upgraded road being available on flat land, but it requires technological advances to allow placing it in hills too.

---

Using EnumMaps like that will result in a lot of buildinfos being rejected by the EnumMaps, meaning the code will loop all builds and read the same memory in each. This is the approach, which will benefit the most from #53 as the CPU will notice the read pattern and read buildinfos prior to them being needed, hence optimizing the CPU cache usage.