Nightinggale
commented
6 years ago Implementation details
Adding a new xml file (not CivEffect specific. This is how any file is done)
Make an info class, which inherits the base info class (providing a certain set of variables etc, mainly Type) Add this to CvGlobals. It needs a get(index), get() and getNumElements. The get without arguments is getting the entire vector for xml reading. I want to change this eventually #53 Add the file to CvXMLLoadUtility::readXMLfiles() (in CvXMLLoadUtilitySet.cpp)
The CivEffectInfo class then needs a ::read(pXml) function, which can be a copy paste from another info::read() and then delete the individual tag read lines.
That's it for adding a new xml file in the DLL. The actual xml file needs to be added and likely needs the xml schema file to be updated as well.
CivEffect base code
It consist of 3 functions in CvPlayer
Reset: Restore default value to each cache variable.
Init: Reset and then loop all conditions, which can provide a CivEffect. Say we only have traits, which can, then the function would be looping all traits and for each trait that the player has, get the CivEffect from CvTraitInfo and if any, call apply with that CivEffect and iChange = 1. Init is called on game start and game load, not during the game.
The autogenerated CivEffect should also be applied here.
Apply(iChange, *pCivEffect): Apply the CivEffect to the cache or remove it if iChange is -1.
Adding a tag to CivEffects
Say we add AllowsUnitClasses. We want it to be of type InfoArrayMod (a class from M:C), though we can use vanilla code too. We init it to
m_info_AllowsUnitClasses(JIT_ARRAY_UNIT_CLASS, JIT_ARRAY_ALLOW)
This tells it that it is an array of types from UnitClassInfos and each index has a variable of type allow.
In read:
m_info_AllowsUnitClasses.read(pXML, getType(), "AllowsUnitClasses");It's the instance, pXML and getType() is the same for all (info arrays expect this) and the last is the tag name in xml. The info array does the rest of the work and will assert if set up incorrectly. Note: info arrays are fully python exposed and both DLL and python can gain read only access to the data, though only xml reading can write to it.
In apply CivEffect:
m_ja_iCacheAllowsUnits.addCache(iChange, pCivEffect->getAllowsUnitClasses());It's a JIT array and it can figure out what to do by just being given the info array. However this is the standard form used by most tags, but here we need to convert from UnitClass to Unit. In other words we add some extra variables.
m_ja_iCacheAllowsUnits.addCache(iChange, pCivEffect->getAllowsUnitClasses(), this, &CvPlayer::getUnitType, NULL, JIT_ARRAY_UNIT_CLASS);This will give the reading code the ability to convert from unit class to unit for the player in question. There is a similar function for buildings already written in M:C.
Last, but not least, we want it to remove all the units the player can't use because of CivilizationInfo. In reset (or init, doesn't really matter which one), loop all units and for each the player will never be able to build, add -50 to that index in m_ja_iCacheAllowsUnits. Since addCache can't affect units the civ can't build, the number will stay negative forever and forever be banned for the player.
That's it. Now it's possible to use bool CvPlayer::canUseUnit(UnitTypes eUnit) const anywhere in the code and get the result virtually instantly, particularly compared to the current approach of looking up xml data.
I should make a new ticket about JIT_ARRAY_UNIT_CLASS, or rather the system behind it. In short it assigns an enum value to each xml file with a Type. The enum value can then be used to get length of the file and getType of each index in that file. This is very useful for detecting invalid xml configurations and when preserving savegames #46 . Since the system also has overloaded functions, which takes an xml enum type as argument and returns the matching JIT_ARRAY enum, the system allows functions with template arguments, which really benefits code readability and code reusability.
abmpicoli
ShadesOT
orlanth
LibSpit
I developed a system in Medieval Conquest called CivEffects. While intended to give more xml freedom, it's also a cache system, which should improve overall performance. Note that what I write here is not precisely the same as what is implemented in M:C. The difference is based on experience and should provide the same result, but with significantly reduced coding time.
xml configuration abilities
The idea is that we add a new info class called CivEffects. Other info classes like traits, founding fathers, events and so on can then get a tag, which makes it provide a CivEffect (though None should be ok). The idea is that tags can be moved from any of those into CivEffects, which makes them reachable by all classes using CivEffects. In other words it removes the issue of needing a tag for traits, but you stall because it's implemented in FFs. Adding ways for a player to get CivEffects seems easy and will not affect the CivEffect code itself or the places, which relies on data from CivEffects, meaning it's a system, which is easy to expand.
Performance (cache)
The entire structure of CivEffect is based around a cache system. Vanilla frequently loop info classes to figure out xml data while CivEffect looks up an int or int array to do the same task. This means it's not only fast, it will not be slowed down by increasing the number of elements in xml. Even better, the cache is incremental, meaning CivEffects are only looped at game startup and game load. Otherwise it's just one at a time and performance will be completely unaffected by the total number of CivEffects.
The idea is as follows. Say we take the function
It's a lookup in an int array (with 8 or 32 bit depending on what we agree on. It can always be changed later). It returns true if the cache is > 0. There is an autogenerated CivEffect, which provides +1 if no CivEffect is higher than 0, meaning anything not mentioned explicitly as unlockable by xml will be allowed. The value is the sum of all CivEffect owned by the player in question. Coding note: maybe make the default -1 if there is something higher than 0 and then make the cache default 1. That way if everything is allowed, the array can be an unallocated JIT array, which would save both memory and lookup time.
Boosting performance of vanilla code
There is more to this though. Units can be disabled by vanilla code in CivilizationInfos. This mean to figure out if a player can use a unit, the code has to look up the UnitInfo to get the unit class and then use the player to look up CivilizationInfo to see which unit that unitclass has for the civ in question. On CivEffect cache reset, we can do this check for each unit and if CivilizationInfo disables the unit, we can grant it an allow value of -50. That way regardless of which CivEffects are active, the combined allow for the unit in question will always be negative. This mean the code can just look up eUnit in the array and compare with 0 rather than looking up two different info classes. This should boost runtime performance even if no CivEffect will enable or disable any unit. The same approach can be used with buildings.
CivEffects aren't saved
Say we have a CivEffect, which grants +1 food production if the plot produces at least 5 food (that's a CivEffect tag in M:C right now). Somebody change the xml file to make it trigger at 4 instead of 5. On game load, the CivEffects are applied and it reads 4. There is no 5 in the cache anywhere and the effect is then applied to all the existing plots. In other words the degree of freedom to modify xml without breaking savegames is really high. Vanilla (and hence current codebase) has issues with cache being saved, meaning certain xml changes can corrupt savegames (which is another issue we should deal with eventually).
Implementation
M:C has classes for most of this. This means the CivEffect class reads from xml is a one line operation and it's very picky about what it reads. You can't write a unit if it expects a unit class and so on. On failure, it can actually tell Type it failed in, tag, what it read and what kind it expected. In other words the assert message should be enough to fix the problem without debugging (unlike most of such issues in vanilla).
Applying the CivEffect is similarly simple from a coding perspective. It's a function, which takes a pointer to the CivEffect in question and an int, which is 1 or -1 depending on adding or removing. Each cache class in CvPlayer is then called, being given the 1/-1 variable and a const pointer to the storage object from the CivEffect. One line operations for 1D, 2D and 3D caches. (avoid 3D caches if possible since they are big, but the code supports them). If it's simple int values (not arrays), it's just a matter of cache += int from CivEffect * (1/-1)
This allows easy to read and code implementation while it still gets the job done regardless of complexity.
At the end of the apply code, it has to have some code to conditionally mark stuff dirty for recalculating caches elsewhere or screen redrawing. It's fairly simple and obvious if and how this should be done when you work on a specific tag.