We-the-People-civ4col-mod / Mod

This is the repository where the mod resides.
90 stars 37 forks source link

Restructuring XML schema structure regarding buildings and building slots #161

Open ShadesOT opened 6 years ago

ShadesOT commented 6 years ago

Currently the mod uses a xml schema structure regarding building slots, that is the legacy of civ4 or civ4:BTS (I don't know). After lengthly looking at the XML and the code, my finding is, that this structure is overly complicated. Some of the reasons why this structure existed in civ4 do not exist in the mod and on the other hand the mods data with its game feature of building slots and building tiers can only be mapped inadequately on this legacy structure. Also there is no code in the mod, that handles the XML structure following the original purpose in civ4 (like code that could handle tech unlocking or obsolescence by tech). Summarized, the general legacy structure was kept, it was heavily distorted by removing from and adding to the XML schemas and code was written, to handle this specific, distorted XML schema structure.

All this makes the XML as well as the code unreadable. Equally important in this case, it has led to inefficient code, when it comes to traversing the raw game data.

So I would like to propose changes, that would make the schema structure fit better the needs of the mod and would enable to write code that efficiently handles this structure. But before proposing anything, I would like to hear your opinions and gather knowledge.

For you to know a bit about the context: The XML schemas that are of particular interest to this topic are the ones for BuildingInfo, BuildingClassInfo and SpecialBuildingInfo.

  1. I would like to know, if there are any hard requirements for a certain xml schema structure, set by the game engine, which can not be changed.
  2. I would like to know if this topic is touched by Nightinggales work and ideas on CivEffects and if so, what to take care of or what needs to be discussed before changing the structure.
  3. I would like to know about any pitfalls you know of, regarding this topic.
  4. And I would like to know if you have any special wishes, regarding a new xml schema structure, regarding building slots and buildings.
ShadesOT commented 6 years ago

One thing that comes to my mind is, that the avalability of a building slot, can depend on CivEffects.

This way we already could map onto CivEffects, what building slots are available to natives and europeans.

Nightinggale commented 6 years ago

The vanilla schema files are a mess and modders certainly haven't made it any better. It seems that everything comes in a semi random order and reading a schema file has become difficult. I started a new schema file for CivEffects where it starts with all the string tags, followed by the int tags (no bool tags yet). Those lists are sorted alphabetically. This is followed by tags used for lists, again alphabetically. The actual file is at the bottom.

One thing I hate about the vanilla layout is the long list of tags in random order. We can add a file/folder like structure and group tags in a logical way, which is more readable than a long list of unrelated tags. We could say make a group of graphics, and while it's closed, we only have game mechanic tags on the screen, keeping those two very different kind of tags apart.

To answer the questions: A: no requirements, which aren't obvious. Any errors (mentioned a tag, which isn't defined etc) will throw errors right away.

B: Right now CivEffects provides bool CvPlayer::canUseBuilding(BuildingTypes). It's intended to be used in canContruct or whatever those functions are named, meaning it can remove the ability to add buildings to the build queue. canUseUnit() is intended to work based on the same principle. Right now that's all the building interaction that's in CivEffects.

JIT array, BoolArray and InfoArray in the CivEffect branch have all gained the function read(pXML, ...), meaning they can read full arrays in xml by simple code like:

m_info_AllowBuildings.read(pXML, getType(), "AllowBuildingClasses");

I should write a wiki page on how this is used. getType() serves no purpose other than providing useful assert messages because it asserts if you add a Building when it expects a Building Class or vice versa (well assert on wrong type in general). The expected class is set in the array constructor.

BoolArray and JIT arrays have the length of some xml file and can be accessed by indexes. Both have a default value. InfoArrays stores "tokens" of data. Each consisting of 1-4 variables and they are looped token by token, meaning if xml has a list of BuildingClasses and only one is mentioned, InfoArray will have one token. This makes InfoArray worse for getting a specific index, but better for looping everything in the list. The fact that it has up to 4 variables gives it the ability to store say a production change of (IMP_FARM, GRASSLAND, YIELD_FOOD, 1) while the other twos are just int/bool arrays.

One good thing about those array types is that you can say make a list of promotions and it will be a list of promotions.

<ePromotion>a</ePromotion>
<ePromotion>b</ePromotion>

Vanilla has to have an int value for each, making it way more verbose in xml.

Nightinggale commented 6 years ago

One thing that comes to my mind is, that the avalability of a building slot, can depend on CivEffects. I agree and the thought has crossed my mind. An implementation would be to add a BuildingArray to CvPlayer, on init CivEffects, loop it and set it to the number of slots of each building read from CvBuildingInfo. Whenever CvCity needs to know the number of slots, it will use that array instead of xml data. Next add an InfoArray to CivEffectInfo, load an xml array into it. The xml array would be (building, iChange) pairs. When a player gains a CivEffect, call BuildingArray::addCache(InfoArray*).

I think that's all it takes to have a working implementation. That's 8 lines of code + the init code where the BuildingArray gains the data from building xml.