Nightinggale
commented
12 months ago Looks like I have to add an xml file for CivCategories as the current implementation is causing issues due to not following the standard. That got me thinking about this and #926.
I came up with the idea of creating CxGlobals now and let it inherit CvGlobals. The singleton instance is then a CxGlobals instead and GC will return a CxGlobals. This way we will have GC, which can access both, hence the code will not care which one the function is located in. We can then add all new functions and variables to CxGlobals as the exe will obviously not need access to those. Over time we can move functions from CvGlobals to CxGlobals and eventually end up with a CvGlobals, which contains the DllExports and nothing else.
As for xml data, add CxInfoBase to use instead. Perhaps CxInfoType is better as it should only contain CvString m_Type as well as the functions getType, readFirst, read and postLoadSetup, none of those virtual. This will be enough to get the xml reading code working and then individual info classes can inherit this and only add read and whatever get functions will be needed.
Storing those info class instances should be in an EnumMap of class instances. This eliminates the need for a virtual deconstructor and if we rely on our own data types (mainly InfoArray and EnumMap), then the default deconstructor should be good enough. Also we will not allocate a new instance for each read loop meaning whatever we read in readFirst() will still be there in read(). Last, but not least: EnumMap stores the instances sequentially unlike the current system where each instance is allocated using new, hence randomly spread across memory. Sequential looping through all entries will obviously be faster.
CvGlobals and CxGlobals can be split in the future if we like, but that's not something, which will happen anytime soon, if ever.
Right now the code is chaotic in the sense that everything calls everything. This is partly design and partly just a consequence of the precompiled header, which grants access to more or less everything everywhere. This is bad because it can easily cause more bugs, but there are other reasons why it's bad.
We should split the code into the following regions:
Note: naming is to be decided later. Maybe the exe needs access to Cv to avoid redirections.
Example: The exe will call CxPlayer instead of CvPlayer. CxPlayer will then call CvPlayer as needed. Likewise if CvPlayer wants to tell the exe something, it will have to travel through a Cx file/class.
Communication between those 3 main groups should be done using interface classes, like CxPlayer calls CvPlayerInterface.h without even including CvPlayer.h.
Why this split?
There are many reasons why this solution would be attractive.
Isolate exe restrictions
The exe has many expectations of the dll. It has a number of DllExport functions where arguments or return types can't be changed. However it has more restrictions, which are more invisible, like calling virtual functions apparently through expected offset of virtual function pointers. Remove or add a virtual function and the exe messes up.
Cx headers can obey exe expectations easily as we aren't really working with it anyway. Cv headers will then be free to be changed as much as we like as the exe can't reach them. If a restriction is disobeyed, linking Cv and Cx will fail, hence fail at compile time and the Cx wrapper function can then typecast or whatever it takes to make it work without changing what the exe will see/get.
This saves us from crashing bugs we can't predict from our code and won't catch at compile time.
Easier access to being type strict
Many functions return int instead of enum types because both python and exe expects an int. This split makes it easy to change an int to say UnitTypes while Cx and Cy will convert it to an int for the outside world to see.
More design freedom
If exe/python can't call Cv files, then we can organize the code just like we want. No need to use the same class/function layout as vanilla if we figure out something else will be better.
Network sync
We can mark files to en entirely synced or entirely run locally. Local files can call synced files, but only through const.
Speaking of const, this too is easier to set up in Cv when it's out of reach of exe and python.
Memory address space
If interfaces between Cv and (Cx+Cy) is done without pointers and only through values, like enum indexes, then the restriction that the Cv files will have to be in the same memory address space will be gone. This allows us to use two address spaces, each with a 4 GB memory limit.
Not having the same address space will remove other restrictions like demanding the same compiler. This is particularly true if we can manage to compile Cv into a different dll file. This will require us to fix lazy linking to make the game look for more than one dll in our mod, but since we want to fix that for tbb.dll, tbbmallc.dll and possibly future dlls too, then that shouldn't be a major new restriction.
Performance considerations
Switching between indexes and pointers could have a minor performance impact. To combat this, we can store exe used data in Cx, such as the variable for CxPlot::getTerrainType. We can even use this as a cache because only the exe will call it, hence only the graphics engine. Rather than looking up current team to see what they see, CxPlot can be a cache of what current team can see.