Caveman 2 Cosmos (ideas/discussions thread)

[Thunderbrd]

OK, we may use std::vector instead. I was poking around in cpp.sh to make a prototype "vector" class, that uses some tricks to make two vectors easily stitched together.
View attachment 526311

How about making the function return a reference to this object, and do everything with this object, such as access its size, its values, and so on (but be careful not to modify it)
(It's easy to add a lock mechanism)

Isn't the usual way to stitch vectors together to create a local temporary empty vector, loop through every item in the first vector and push each to the temporary, loop through every item in the second vector and push to the temporary, clear the first vector and then push everything in the temporary vector to that original and then we have a true final one? Is there a problem doing so in this case that requires a new approach?

I can't claim any familiarity with quite a bit of what you're working with there Anq but don't take my questioning to be any kind of disdain on what you've come up with... I really don't understand how it works because I don't recognize much of the methods in use. So it's really not a commentary on whether it's valid or not - I'll leave that to AIAndy and Alberts2, who's comments aren't making a lot of sense to me either in the deeper specifics they are discussing. I do hope we can come to some kind of consensus. In the meantime, I'm just bringing up what my limited understanding can offer into the discussion.

 

[Anq]

I've finally come to realize how the modular loading works. The journey, in the namespace of CvXMLLoadUtility, starts from LoadPreMenuGlobals(). The game's characters are loaded class by class, starting with their basic traits, or the common language in which we describe them. This loading order is carefully crafted to minimize the need for a second read-pass to figure out any names dangling on the DelayedResolution lists. Each loading instruction is loyally obeyed to bring in all the files and read them. LoadGlobalClassInfo() tells its workers where to find the regular documents, and they'll fetch the Modulars in addition. Reading the documents is a separate step, called SetGlobalClassInfo(). In here, a (CvInfoBase-derived-or-non-derived-class)::read() function is called to translate the plain text into usable data. For a CvInfoBase-derived class, <Type> is an important identifier for an object, and in this stage, when we are applying modular xmls, we need to look up the default object with this identifier, and merge with or overwrite the default object. That's all how it works. Let's review the process in fewer words:

1. Our boss, LoadPreMenuGlobals(), tells each department to do their work.
2. Our department, LoadGlobalClassInfo(GC.getBuildingInfos(),[...]), starts to make BuildingInfo objects out of plain-text data for everyone else.
   
3. We work on BuildingInfo documents in our own database, but we also accept Modular cases from the outside.
4. Our workflow, SetGlobalClassInfo(std::vector<CvBuildingInfo*> &aInfos,[...]), establishes the default BuildingInfo objects by calling ->read() on them to translate data.
5. And for the modular cases, we check if the flag <bForceOverwrite> is set when we ->read() them, look up the default object of the same <Type>, and depending on that flag we either outright replace it or call ->copyNonDefaults() to merge them and put the result back in place.

This is why our ForceOverwrite flag can be placed in the Global Context. It is consumed right after it is set before the next object is processed. I certainly didn't realize this relationship.


However, one can not justify the bugs in some code that tend to overwrite parts of a default object with those from the modular ones. I'm sure if a modder wants to cancel some part of the default object without modifying the base, he'll specify it with <bForceOverWrite>1</”>. Also, I'd like to make overwrite easier without having to copy information you want to keep, by making the code in SetGlobalClassInfo not throw away the default entirely, but copy its info for the modder. This can be done by moving the switch to the copyNonDefaults() stage. How does this sound?

 

[Thunderbrd]

However, one can not justify the bugs in some code that tend to overwrite parts of a default object with those from the modular ones. I'm sure if a modder wants to cancel some part of the default object without modifying the base, he'll specify it with <bForceOverWrite>1</”>. Also, I'd like to make overwrite easier without having to copy information you want to keep, by making the code in SetGlobalClassInfo not throw away the default entirely, but copy its info for the modder. This can be done by moving the switch to the copyNonDefaults() stage. How does this sound?

It sounds appropriate to me. I'm sure DH would appreciate it! However, I'm not saying this with deeper than a procedural understanding of how to add a tag. You're looking deeper than I have.

 

[Anq]

I'm reverting to use std::vector. An empty vector is 16 bytes compared with an empty pointer which is 4 bytes, but it can simplify the code a lot.
I give up inventing another class which still claims 12 bytes when empty. It's nonsense, and too primitive compared with the STL vector.

Example:

1. type signatures

std::vector<int> m_aPrereqOrVicinityBonuses;
const std::vector<int>* CvBuildingInfo::getPrereqOrVicinityBonuses() const
// the same as `std::vector<int> const * CvBuildingInfo::getPrereqOrVicinityBonuses() const`
{
    return const_cast<std::vector<int>*>(&m_aPrereqOrVicinityBonuses);
}

Notice that the function returning the pointer to vector can no longer be constant.
Edit: seems like I can const_cast the returning pointer, and it seems ok to do so.
Edit: added const keywords as @AIAndy suggested.
Edit: should the method and the returning pointer be not constant?
Edit: I think the returning pointer can be treated as const, because the member m_aPrereqOrVicinityBonuses doesn't change address at any moment.

2. usage

const std::vector<int>* iBonus = kBuilding.getPrereqOrVicinityBonuses(); // kBuilding is of type CvBuildingInfo&
// the same as `std::vector<int> const * iBonus`
for (std::vector<int>::const_iterator it = iBonus->begin(); it != iBonus->end(); it++)
{
    if (*it != NO_BONUS) // dereferencing the iterator should return the value
    { [...] }
}

Although returning the vector pointer makes its data mutable, we can ensure this doesn't happen when we handle them.
(Now that we return the pointer to a constant vector, we have ensured this.)

Advantage of this approach: We don't need to keep a separate function that returns the size of the vector, or any information derived from the data. We just keep one which returns the pointer to our vectorized data for them to process.

If we try to ensure immutability by returning a copy of vector data instead of the pointer, we're wasting a little memory on the copies, although the copies will be removed from the stack once a function using this data finishes.
Which approach seems better?
I think, because we do pure calculations from the data most of the time, it's rather safe and quicker to give them the pointer, to reduce the size of the stack a calculating function creates.

Edit: I learned that const_iterator is better than iterator for read-only access.
Edit: It also needs the vector to be const in order to use a const_iterator. This requires our access method to have a signature of "std::vector<int> const * ". Please read AIAndy's excellent explanation below if you're not sure about the const modifier.


Tips on using a vector:
1. Reserve the space first and then add elements. This way it won't allocate an array of size we don't know and automatically expands the space by making another array (and what's worse, copy the data to there) when the space is used up.
2. It's better to use the iterator than .push_back() to write a vector, if you've reserved the capacity correctly. (thinking again, push_back is really ok when you don't add more than reserved.)

@alberts2, I think in your new template reader SetOptionalVector() you can add aInfos.reserve(iNumSibs); in front of the loop, and use the iterator (much like a pointer) to write the vector. .push_back() is ok

 

[AIAndy]

I like your approach.
What about returning a reference or pointer to const vector so it remains immutable?
If you properly expose it to Python with vector_indexing_suite it should also feel more natural to Python users (alternatively you could make a copy of the vector into a python list).

 

[Anq]

1. It seems like const_cast'ing our returning pointer works. (edited) (a little typo, should be const_cast<std::vector<int>*>)
1.1 Oh I didn't read your words correctly. By using the const_iterator the vector has to be immutable. I think vectors are naturally all mutable... (A vector is only pointers, so by immutable it might mean 1. its pointers cannot change, which is impractical and meaningless. or 2. it was writable from inside the object, but readonly from outside.)
2. Oh my god, I haven't considered the python interface. How do I export a vector there...........

 

[AIAndy]

1. It seems like const_cast'ing our returning pointer works. (edited)
2. Oh my god, I haven't considered the python interface. How do I export a vector there...........

const is somewhat difficult. It matters a lot where you place it. What you did there is that you made the method const which means the implicit this pointer is const and not the return value.
Try either of the following:

const std::vector<int>* CvBuildingInfo::getPrereqOrVicinityBonuses() const
std::vector<int> const* CvBuildingInfo::getPrereqOrVicinityBonuses() const

For exposing it to Python you have two options:
Either create a native Python list from the vector and return it from the Cy variant of the function.
Or use vector_indexing_suite to have Boost Python create a kind of wrapper type around your vector which allows Python to iterate the vector as if it was a native Python list.

 

[Anq]

const is somewhat difficult. It matters a lot where you place it. What you did there is that you made the method const which means the implicit this pointer is const and not the return value.
Try either of the following:

const std::vector<int>* CvBuildingInfo::getPrereqOrVicinityBonuses() const
std::vector<int> const* CvBuildingInfo::getPrereqOrVicinityBonuses() const

Now I got it. A vector is nothing more than 4 pointers (begin, end, end of capacity, and the array allocator(?) <- which is Visual C++ specific. GCC vectors are 3 pointers only.). By saying the vector is immutable, it means its internal pointers cannot change. (nonsense)
The first line means the vector our returning pointer points to is a constant, and for it to be so, the object's internal vector also has to be constant. (also not true)
This means we cannot write to the vector, and it will always be zero. (Think about the end and end-of-capacity pointers and you know why.)
(For this to happen, you have to declare std::vector<int> const m_aPrereqOrVicinityBonuses, which is nonsense.)
The second line means the pointer to our vector is constant, which is what I want. I didn't know the const keyword can show up in these three places!
(Explained below)

 

[AIAndy]

Now I got it. A vector is nothing more than 4 pointers (begin, end, end of capacity, and the array). By saying the vector is immutable, it means its internal pointers cannot change.
The first line means the vector our returning pointer points to is a constant, and for it to be so, the object's internal vector also has to be constant.
This means we cannot write to the vector, and it will always be zero. (Think about the end and end-of-capacity pointers and you know why.)
The second line means the pointer to our vector is constant, which is what I want.

Actually both lines mean the same. The const always refers to the object on the left, but if there is nothing on the left, it refers to what is directly on its right.
So in both cases the const refers to std::vector<int> (without the pointer). If you'd move it to the right of the *, it would refer to the pointer but that makes little sense as the pointer is returned by value.
const on the std::vector<int> means that the code which calls this function can only call methods or operators on the vector that are themselves declared const and that means they don't change the internal state of the vector. So the calling code could iterate over the elements of the vector (as const), but cannot call push_back to modify it.

 

[Anq]

Actually both lines mean the same. The const always refers to the object on the left, but if there is nothing on the left, it refers to what is directly on its right.
So in both cases the const refers to std::vector<int> (without the pointer). If you'd move it to the right of the *, it would refer to the pointer but that makes little sense as the pointer is returned by value.
const on the std::vector<int> means that the code which calls this function can only call methods or operators on the vector that are themselves declared const and that means they don't change the internal state of the vector. So the calling code could iterate over the elements of the vector (as const), but cannot call push_back to modify it.

That clears it up a lot, thanks. So it doesn't automatically mean the vector referenced by the pointer has to be constant, just that those who call the function are supposed to call only const methods of the vector.
This is the magic of a class, wrapping an internal mutable data as readonly when accessed by others.

I tried to compile this code and find the const_cast still necessary if the method is const. Making the method const is still the correct move, to declare the pointer as immutable, right?
Edit: Ah, I find out. When you don't have this const method and const pointer, you can't use the const_iterator. The compiler prohibits that. So making the method const is necessary.
(I was silly not to declare const std::vector when I use them...)
Edit: I can make the method not const and remove the const_cast part, while keeping the output vector const.
(Because the member vector m_aPrereqOrVicinityBonuses should never change address, making the method and the returning pointer const makes sense.)

Edit3: I'm surprised that <PrereqVicinityBonuses> doesn't require the python interface to interprete. CvGameTextMgr handled the help text creation so the pedia automatically displays the message. I suppose the things exposed to python are used to calculate and display more visuals than text.




Edit: On the merging procedure in copyNonDefaults():
I'm able to create a temporary vector, copy the contents from Base and Mod objects, in that order, and finally assign the field of the Mod object (the recipient if you understand the modular loading process) to this temporary vector.
What I'm trying to say is that, although the signature of the method name, getPrereqOrVicinityBonuses() const, implies that its return pointer is constant, it doesn't have to keep the address of the vector constant. The trick here is to const_cast the address to make others think it's constant.
By creating a temporary vector and finally assigning the field name (m_something) to it, I'm changing the address to the field. I'm reporting a new address but I'm able to persuade others the address jump didn't happen. Through class magic.

This is comparable to how we return a readonly vector, while we're still able to modify its data.

 

[Anq]

I have spotted another potential optimization. In the modular loading process, precisely in CvXMLLoadUtility::SetGlobalClassInfo(), when it comes to merging the base and mod objects, I think it's better to call Base.copyNonDefaults(Mod), instead of Mod.copyNonDefaults(Base).
For vector data, it is easier to reserve new space for the original vector, and copy data from the Mod's, rather than create a temporary vector and copy data from the two, in order to preserve the order (Base in front of Mod).
It may be no difference under the hood, because when you tell a vector to reserve new space, it still copies its data onto the new array, so the copy action cannot be avoided.
However, if you do Mod.copyNonDefaults(Base), you're copying the vector every time from Base because the base will be deleted, which can be prevented if we preserve the Base object and do the opposite.
How does this sound? Any caveats?

(The Mod object doesn't need to be explicitly deleted. It has been created on the stack, so when SetGlobalClassInfo() finishes, it's automatically released, in contrast to the Base object, which is a reference in the Global Context.)
Wrong. It has to be explicitly deleted by calling its destructor or simply delete through macro SAFE_DELETE. It is the pointer that's being released, but the Mod object is still there. pClassInfo is only a pointer. The objects are created on the heap and their address sent out through the `new` keyword, to pClassInfo. This is how the Base object can persist outside the scope.

 

[Thunderbrd]

I'm able to create a temporary vector, copy the contents from Base and Mod objects, in that order, and finally assign the field of the Mod object (the recipient if you understand the modular loading process) to this temporary vector.

I was thinking on this earlier and I know this is basically what I said to do. However, it occurred to me that this could allow duplicates, sometimes this may be fine but not for all tag uses. Would it be too time consuming to loop through each of the module's objects in the tag in question and check them each against each of the current active items in the core vector before simply then pushing them into the core vector? Am I making sense?

 

[Anq]

I was thinking on this earlier and I know this is basically what I said to do. However, it occurred to me that this could allow duplicates, sometimes this may be fine but not for all tag uses. Would it be too time consuming to loop through each of the module's objects in the tag in question and check them each against each of the current active items in the core vector before simply then pushing them into the core vector? Am I making sense?

@alberts2 's SetOptionalVector() and CopyNonDefaultsFromVector() template functions are now handling duplicates and sort them well, so no worries.

Basically when you constantly sort them, the order of which comes in first doesn't matter. Still copying is best avoided by calling Base.copyNonDefaults(Mod) instead of the other way around.

 

[raxo2222]

Would be removing unlimited specialists from civics good idea? For example pacifism (military civic) allows unlimited artists.

Depending on wonders, religion, civics and techs single specialist is as good as typical building without modifiers
If you want max culture base then pacifism is best choice as it allows unlimited artists.

Looks like cities contribute to national culture even after capping, so that fix works

 

[Thunderbrd]

Would be removing unlimited specialists from civics good idea? For example pacifism (military civic) allows unlimited artists.

I've often thought we could do that and furthermore make a new base citizen that represents an unemployed one and then make even 'citizens' limited, though many buildings would enable them, representing extra labor employment potential. When all the limited specialist slots run out and you simply have too much population to gainfully employ for your current economic infrastructure, they just end up being unemployed, which would basically be completely worthless citizens, not angry, not refusing to work really, just unable to. Maybe they would have -gold to represent the need to support them with welfare or something depending on the civics.

 

[Toffer90]

I've often thought we could do that and furthermore make a new base citizen that represents an unemployed one and then make even 'citizens' limited, though many buildings would enable them, representing extra labor employment potential. When all the limited specialist slots run out and you simply have too much population to gainfully employ for your current economic infrastructure, they just end up being unemployed, which would basically be completely worthless citizens, not angry, not refusing to work really, just unable to. Maybe they would have -gold to represent the need to support them with welfare or something depending on the civics.

I like that idea.
Also, a civic can instead of having unlimited X specialist, have something like 50% more X specialist.

 

[raxo2222]

@JosEPh_II you are one balancing civics, what do you think about removing unlimited specialists?
Won't hurt early game and may help balance late game, as there may be enough room for those.
This is tag responsible by this:

<SpecialistValids>
                <SpecialistValid>
                    <SpecialistType>SPECIALIST_X</SpecialistType>
                    <bValid>1</bValid>
                </SpecialistValid>
            </SpecialistValids>

 

[JosEPh_II]

From my point of view there has always been a problem with unlimited Specialist. The problem I have with it is that every time you can add a specialist the game will add it to the unlimited position. Then when you go into a city you have to re arrange How You Want the ratio to be. I don't want unlimited Engineers in my cities early game. And Right now Early game is plagued with every specialist being added, thru what ever mechanism, as an Engineer. You have to constantly shuffle your Specialist to what you want, under of course the constraints of what types you actually can have and use.

On the Subject of Specialists I would Love to have a Lock Mechanism to stop this automatic addition to the Unlimited position. I want to be able to set my specialists under the conditions present as to how and what kind and then set it and Lock it. Then when the next specialist is used I'm presented with the Choice, again under the constraints of what I'm allowed to have, to set them and then lock them. And not have them added automatically to Unlimited position.

Toffer's post I think would help address this constant reshuffling of Specialists to the Unlimited ones.

@raxxo2222,
And where is that code located?

Also Unlimited Artists comes well after Unlimited Engineers and Unlimited Priests. Your example is not as big a factor because of When you 1st can get Unlimited Artists vs the Other Unlimited ones, like Engineers, Priest, and Merchants.

In my opinion the Doctors, Investigators, Scientists need more boosting than the other 4 you and I have mentioned. These last 3 are a bit too limited now as is the one displayed above the Investigator and below the Celebrity ( can't think of it's name atm). And Doctor and Investigator specialists are vital ways to reduce the property control units overusage that the game is plagued with still. The reduction to disease and crime by both these 2 Specialists is Weak imhpo. Too weak to be of much if any help.

 

[JosEPh_II]

10596

1. Fixed some AI issues with damage from Features or Terrains.
2. Made the Worker AI aware of the Movement Cost changes from svn8795.
3. CvUnit::meetsUnitSelectionCriteria now check's if the Unit has a beneficial or non-beneficial Property Value for the Property specified in the Criteria.
4. Performance improvements wherever the BuildingInfo tags UnitClassProductionModifier, BuildingClassProductionModifier and GlobalBuildingClassProductionModifier are used.
5. Fixed some issues with the canConstruct caching with the GAMEOPTION_OUTBREAKS_AND_AFFLICTIONS enabled.
6. Replaced a few invalid usages of NO_UNIT as UnitId with FFreeList::INVALID_INDEX.
7. Added the damage from Terrains and Features to the help text display.
8. Movement and Route Movement cost's are now displayed in the Plot Hover.
9. The Gold for disbanding Units is now only displayed with the GAMEOPTION_DOWNSIZING_IS_PROFITABLE enabled.
10. Fixed the reward of Defensive Victory and Offensive Victory moves to Units.
11. Reworked the Routes Pedia.
12. Code cleanup and various smaller fixes.

Number 3 caused some issues with the Property Control AI e.g. anti crime units had been able to answer requests for anti disease units and so on............................
Number 4 speeds up opening the Buildings pedia, here it's down to 17 seconds from 24.

Yeah Yeah and Yeah!!! Thank you Alberts2. Thank you Thank you!

 

[Dancing Hoskuld]

However, one can not justify the bugs in some code that tend to overwrite parts of a default object with those from the modular ones. I'm sure if a modder wants to cancel some part of the default object without modifying the base, he'll specify it with <bForceOverWrite>1</”>. Also, I'd like to make overwrite easier without having to copy information you want to keep, by making the code in SetGlobalClassInfo not throw away the default entirely, but copy its info for the modder. This can be done by moving the switch to the copyNonDefaults() stage. How does this sound?

Just to make sure I am not reading what you put here incorrectly.

Firstly options,

If a modder wants to change some things in a way that is optional for the player they do it with modules that can be turned on or off either through the MLF system or by deleting (or moving) the relevant module folder.

This means that some module XML will be ignored completely in one run but not in another. I don't think this is an issue here, just including it for completeness.

To make one module work with other modules there are AND and OR Dependency tags that indicate if the object should be updated or not. There is a "bug" in this as it can only be used in a way that matches the order in which classes of objects are loaded. If I remember correctly you can have a building dependency for a unit but not a unit dependency for a building.

Again this happens before you are doing any updates for the objects so I am only including it for completeness. Updates that fail the dependency test wont get to the update stage.​

Secondly updates,

If a modder wants to update a particular base object they can just specify the object key (usually TYPE element) and the elements they want to change with the new values. They do not have to specify any other elements.

For example if I have a module which adds a new Mammoth (Colombian or maybe Mastodon) then I would include in the module an update to the Mammoth Training wonder that adds those animal herds to the list of Or prerequisite buildings but that would be all I change about the building in the XML.​

If they want to set the value of an element back to a default value then they have to use the bForceOverWrite element set to 1 and specify all the elements that have non default values in the object.​

Where I think I am confused about the above code change suggestion.

This means that if you have the base object and see that there are modifications/updates to it (at run time) then you can't assume that an element value in the base will be overwritten by something in the module; so it is not safe to set it to the default. That came out awkwardly.
​

Would be removing unlimited specialists from civics good idea? For example pacifism (military civic) allows unlimited artists.

Depending on wonders, religion, civics and techs single specialist is as good as typical building without modifiers
If you want max culture base then pacifism is best choice as it allows unlimited artists.

Looks like cities contribute to national culture even after capping, so that fix works

Yes and no. We have discussed this before. Yes it is way overpowered in C2C. A better option would be to have the civic allow a specific number of specialists just like buildings do.

I've often thought we could do that and furthermore make a new base citizen that represents an unemployed one and then make even 'citizens' limited, though many buildings would enable them, representing extra labor employment potential. When all the limited specialist slots run out and you simply have too much population to gainfully employ for your current economic infrastructure, they just end up being unemployed, which would basically be completely worthless citizens, not angry, not refusing to work really, just unable to. Maybe they would have -gold to represent the need to support them with welfare or something depending on the civics.

That is what the Citizen specialist is supposed to be about. IE we don't need a new one we just need to define an existing one better.

I like that idea.
Also, a civic can instead of having unlimited X specialist, have something like 50% more X specialist.

This might also work.

I don't think enough early buildings are allowing enough specialists.