DLL coding thread

[Kailric]

That is slow a bit slow, but sort of needed. The static water plotgroup could tell that two cities are connected, but the trader can't move through the pirate blocking the chokepoint. That can only be fixed by making real water plotgroups for each player. That might actually make sense as it would make sense as it would only spread across explored plots.

There could be a third plotgroup system for costal plots. I wonder if that is overdoing it though.

It does beg the question about "chokepoints" and the AI. As a Player could just steer around a blockade attempt. The AI however, would break automate once it spied an Enemy unit I would presume, so then the Player could decided what to do about it. Which reminds me, I need to add in the break automation code for AI Traders as I haven't done that yet. I left that out so it wouldn't mess with testing.

Looking at the code I remember one issue. I added code for Plotgroups to store all None Player city info, so you can tell what foreign Cities are connected to your Plotgroups. The code loops these cities only. This won't work well for Ships as some cities they could trade with won't be included. So, for Ships to be AI Traders I would need to write code for them. I would have to cycle all Player Cities and do qualifying checks to see if the city is accessible.

 

[Nightinggale]

It does beg the question about "chokepoints" and the AI. As a Player could just steer around a blockade attempt. The AI however, would break automate once it spied an Enemy unit I would presume, so then the Player could decided what to do about it. Which reminds me, I need to add in the break automation code for AI Traders as I haven't done that yet. I left that out so it wouldn't mess with testing.

Let's deal with the AI once it's working for humans, but yeah there could be an issue here.

Looking at the code I remember one issue. I added code for Plotgroups to store all None Player city info, so you can tell what foreign Cities are connected to your Plotgroups. The code loops these cities only. This won't work well for Ships as some cities they could trade with won't be included. So, for Ships to be AI Traders I would need to write code for them. I would have to cycle all Player Cities and do qualifying checks to see if the city is accessible.

enum DllExport DomainTypes
{
	NO_DOMAIN = -1,

	DOMAIN_SEA,
	DOMAIN_LAND,
	DOMAIN_IMMOBILE,

#ifdef _USRDLL
	NUM_DOMAIN_TYPES
#endif
};

Looks to me like we could make a plotgroup for each domain type. We might want to skip DOMAIN_IMMOBILE though. If we take that approach, we already solved the issue regarding adding space domain and stuff like that.

Maybe making a plotgroup for immobile will not really be an issue. The plotgroup spread will just return false for all plots. That would be easier to code if we just add a JIT array of domain types.

 

[Kailric]

Do we even need a DOMAIN_IMMOBILE? It isn't used for anything currently, in fact if it did get used it would cause a failed assert here in CvUnitAI.cpp...

case DOMAIN_IMMOBILE:
	FAssert(false);
	break;

I think we could safely remove it to make things simple, I count 14 places its used in the DLL. Mostly referring to units that can not move. We could make other means if we want immobile units than to make a whole domain for them. Besides, Yield Units are immobile and they are not associated with that domain.

 

[Nightinggale]

It's actually a very good question if DOMAIN_IMMOBILE is needed. Possibly not, though we would have to consider what to do with units, which can't move, but can do other stuff. Think units, which can bombard, but has to be moved by transports. It also goes for the concept of units, which provides a free promotion if they are in the cargo hold.

I don't think yield units are even supposed to be on the map, meaning they will not trigger movement code. Speaking of yield units, I have a plan, that will remove them from XML. Instead they will be generated at startup based on yield XML data. This will make it easier to add new yields. Once cargo yields aren't hardcoded in the DLL anymore, yield XML will be the only place to set up yields.

It's not that tricky to generate something like that on the fly. CivEffects branch is generating a default CivEffect, which has 1 in all allow settings, which should be available from the start. This simplifies the cache to disallow everything by default as there is always at least one enabling CivEffect for everything. Since this was so easy and fast to make, we should remember this approach for simplifying XML setup in the future for... well possibly all XML files

 

[Kailric]

Just for reference sake, Yield Units do appear on the map, when you select one that is on a Transport they appear, but you can not move them. I think once I set their movement to 1 just to see what would happen and they moved around just fine. So, apparently just setting a Units movement to 0 will keep them from moving.

 

[Nightinggale]

Yield units are rather passive. I'm sure other units could suffer from not having movement points. However rather than setting a domain to not move, we could simply set a bool. The unit could then have 1 move and hence one action each turn, but canMove(plot) will always return false, meaning they can't move on their own power.

 

[Kailric]

Here's an interesting comment..

// XXX at some point test the game with and without this function...
bool CvUnitAI::AI_plotValid(CvPlot* pPlot)

I wonder if they ever tested it? If not, should we

 

[Nightinggale]

Here's an interesting comment..

// XXX at some point test the game with and without this function...
bool CvUnitAI::AI_plotValid(CvPlot* pPlot)

I wonder if they ever tested it? If not, should we

That function is used in 49 places in vanilla and presumably the same in M:C.

Looking at this function and a few of the locations where it is used, it looks like the AI will behave precisely the same way if this function always return true. This function is used early in a chain of if sentences.

I think the idea is that if an AI unit tries to figure out what it can do, it has this quick access to tell if it might be able to enter a plot. For instance there is an enemy unit next to it and it tries to figure out if attacking it is possible and a good idea. This function will tell that the ship can't move onto land without checking anything else meaning it will use less CPU time on this task before it gives up.

If this function returns false, it saves some time, but if it is true, this function is a waste of time because everything in it will be checked properly later on in the if chain.

I think the comment refers to figuring out if it use more time than it saves. My 6th sense tells me that it is a good idea to keep this function. I think Firaxis ended up keeing it based on tests 10 years ago and I suspect that the speed improvement from this function is better today than when Firaxis made it due to CPU speed vs memory latency in modern computers.

 

[Kailric]

Ok, cool, I thought it was interesting none the less. I'm going with your 6th sense and leaving it as is.

 

[Nightinggale]

I just had an inspiration and went for it. InfoArrays are now able to read floats from XML.

What it actually does is it adds a bool to the read arguments (default false), which tells if it should read int or float. If it reads floats, it reads the string, copies it into another string until the ".". The dot itself isn't copied, but it then carries on and adds the next two characters. If the dot is missing, it is assumed to be at the end and if there aren't two characters after the dot, the missing characters are added as "0". The resulting string is then converted to an int using standard C++.

This is a string approach to convert a float to an int and we are 100% sure we will not get float rounding errors. Since it will end up with an int, the InfoArray will not work differently from before after it has read the XML. The only difference is how to enter numbers in XML.

One example is the units consuming yields. Right now if 5 units together use one yield each turn, it has to be entered as they consume 20 each (as 100 is one total yield). InfoArrays will now allow us to enter 0.2 (where it's easier to see that it needs 5 to be a complete 1). Internally it's still converted to 20, but internally has less demand for human readability as the XML files have.

I'm not using it yet, but I ran a quick test and it converted 0.2 into 020, which was then converted to the int 20. Looks ok based on that.

 

[Kailric]

Ok, trying to add Leader Specific Diplomacy Responses but the XML is set up to use defaults regardless...

I've added..

<Response>
  <Civilizations/>
  <Leaders>
    <Leader>
      <LeaderType>LEADER_THE_ANGLO_POPE</LeaderType>
      <bLeaderType>1</bLeaderType>
    </Leader>
  </Leaders>
  <Attitudes/>
  <DiplomacyPowers/>
  <DiplomacyText>
    <Text>AI_DIPLO_POPE_DECLARE_WAR_1</Text>
    <Text>AI_DIPLO_POPE_DECLARE_WAR_2</Text>
    <Text>AI_DIPLO_POPE_DECLARE_WAR_3</Text>
  </DiplomacyText>
</Response>

and the dll still reads the default as well for the Pope...

<Response>
	<Civilizations/>
  <Leaders/>
	<Attitudes/>
	<DiplomacyPowers/>
	<DiplomacyText>
		<Text>AI_DIPLO_DECLARE_WAR_1</Text>
		<Text>AI_DIPLO_DECLARE_WAR_2</Text>
		<Text>AI_DIPLO_DECLARE_WAR_3</Text>
		<Text>AI_DIPLO_DECLARE_WAR_4</Text>
		<Text>AI_DIPLO_DECLARE_WAR_5</Text>
		<Text>AI_DIPLO_DECLARE_WAR_6</Text>
		<Text>AI_DIPLO_DECLARE_WAR_7</Text>
		<Text>AI_DIPLO_DECLARE_WAR_8</Text>
		<Text>AI_DIPLO_DECLARE_WAR_9</Text>
		<Text>AI_DIPLO_DECLARE_WAR_10</Text>
	</DiplomacyText>
</Response>

the dll then uses...

// Leaders
	pXML->SetVariableListTagPair(&m_abLeaderHeadTypes, "Leaders", GC.getNumLeaderHeadInfos(), false);

void CvDiplomacyResponse::setLeaderHeadTypes(int i, bool bVal)
{
	FAssertMsg(i < GC.getNumLeaderHeadInfos(), "Index out of bounds");
	FAssertMsg(i > -1, "Index out of bounds");
	m_abLeaderHeadTypes[i] = bVal;
}
bool CvDiplomacyResponse::getLeaderHeadTypes(int i)
{
	FAssertMsg(i < GC.getNumLeaderHeadInfos(), "Index out of bounds");
	FAssertMsg(i > -1, "Index out of bounds");
	return m_abLeaderHeadTypes[i];
}

So, what can I do to prevent LeaderTypes from using the default responses.

 

[Nightinggale]

So, what can I do to prevent LeaderTypes from using the default responses.

I think the answer lies in bool CvDiplomacyResponse::read(CvXMLLoadUtility* pXML).

Right now it has the code

pXML->SetVariableListTagPair(&m_abLeaderHeadTypes, "Leaders", GC.getNumLeaderHeadInfos(), false);

After that if we add code like this:

bool bHasTrue = false;
for (int i = 0; !bHasTrue && i < GC.getNumLeaderHeadInfos(); ++i)
{
    bHasTrue = m_abLeaderHeadTypes;
}


if (!bHasTrue)
{
    // all is false, which mean everything is true. Set all to true.
    for (int i = 0; i < GC.getNumLeaderHeadInfos(); ++i)
    {
        m_abLeaderHeadTypes[i] = true;
    }
}
bool* abTemp = NULL;
pXML->SetVariableListTagPair(&abTemp , "LeadersDisabled", GC.getNumLeaderHeadInfos(), false);

// disable all leaders, which are set to be disabled
for (int i = 0; i < GC.getNumLeaderHeadInfos(); ++i)
{
    if (abTemp[i])
    {
        m_abLeaderHeadTypes[i] = false;
    }
}
SAFE_DELETE_ARRAY(abTemp);

That should do it, though it would be easier to do with InfoArrays as it would require less lines of code and would execute faster.

LeadersDisabled should be added to schema and more or less be a copy paste of Leaders.

Using Leaders and LeadersDisabled when filling out XML will then allow setting code for "this group" and "everybody but this group".

 

[Nightinggale]

I added something for this in AI_Traders, though it ended up being different from what I first proposed.

My change does the following when reading a single DiplomacyInfo

1. Makes a bool array (AA) of length numLeaderHeads and set everything to true
2. When a response is read, loop the LHs for it. If a LH is true, set that index in AA to false
3. Once all responses have been read, loop all responses
4. If a response doesn't have any LHs set to true, copy AA into LHs for that response

The result is that if Leaders are empty for a response, it will apply to all leaders, which haven't been mentioned in any response at all. In other words, adding a response and then make it apply to a single LH will automatically exclude that LH from the general response. I figured that is likely the setup that would be most useful. If people define a response for a specific LH, it is unlikely that this LH should ever use the general response.

There is one exception to the rule. If all LHs have been mentioned, the result for the general response will be all false, which is the same as everybody. As long as nobody adds responses for barbarians, then that shouldn't be an issue.

I haven't tested this ingame, but walking though the code with the debugger looks like it does precisely what I planned it to do.

If the code isn't activated, then you better quickly turn off the XML cache in the ini file. I disabled the DLL's calls to XML caching in CivEffect (forcing it to be off), but naturally that will not apply before the branches are merged.

 

[Kailric]

If people define a response for a specific LH, it is unlikely that this LH should ever use the general response.

Right, and if they still wanted this they can simply copy and paste the general responses to that leader.

There is one exception to the rule. If all LHs have been mentioned, the result for the general response will be all false, which is the same as everybody. As long as nobody adds responses for barbarians, then that shouldn't be an issue.

The Barbarian actually has a LeaderHead if that is what you are referring to, so it shouldn't cause a problem. You never see his LeaderHead in the game, but on the Military Advisor Screen there is a Silhouetted Leader Icon for Barbarian units. I used a darkened image if Arnold Swartzerdude as Conan

If the code isn't activated, then you better quickly turn off the XML cache in the ini file. I disabled the DLL's calls to XML caching in CivEffect (forcing it to be off), but naturally that will not apply before the branches are merged.

Not understanding this?

Anyway, I just tried out your changes and it seems to work perfect. Both new leaders only used their new Diplomacy texts in my initial tests (unless by the odd chance they both randomly picked the correct ones for both tests, which hasn't happened yet).

Thanks much for solving that Night, once again you prove that we are all noobs and need to give you and a little bit of and

 

[Nightinggale]

I have added CivEffect as a JIT array type and decided that it should be in savegames for maximum savegame preservation. However because CivEffect is actually just a list of other arrays, all the data is already in the savegame. I managed to be able to recreate the entire CivEffect array (currently almost 100 types/strings) by adding a single int. Even though I have done lots of stuff to keep the savegame filesize down, reducing 100 strings to a single int has to be the best size reduction ever.

I also changed the CivEffect code to use JIT array types. Everything before NUM_CIVEFFECT_TYPES is CivEffects and they are assigned IDs in that order. This mean telling the generic CivEffect code about a new CivEffect file is simply adding it to the enum and the rest of the DLL will figure out what to do.

While working on the JIT array string saving, I spotted an unexpected opportunity, which I made good use of and it will serve us well in the future. I added a fixed string for each type in JIT_ARRAY_TYPES. When saving the strings, the string for the array type is saved as well. On load, this string is then used to find the new index in JIT_ARRAY_TYPES and it will write a conversion table for that index instead of the index when it was saved. This mean order doesn't matter anymore and we can add/remove/reorder that array as much as we like without breaking savegames. That's one step closer to my goal of never breaking savegames again (though adding this broke savegames )

Thanks much for solving that Night, once again you prove that we are all noobs and need to give you and a little bit of and

Well, that might be fitting. After all the forum decided that I'm a Deity (seriously, I didn't write that myself. One day it just appeared )

 

[Lib.Spi't]

you are officially the highest level of difficulty! Cue mystic music!

 

[Nightinggale]

I just had an idea regarding JIT arrays. I will write it here to both tell about it and as a reminder to myself.

Right now saving a JIT array saves the index of the highest non-default value. It then saves all values from the start until it reaches the last non-default. On load, the array is set to only default values meaning anything not present in the savegame will stay at default. This mean it will save 10 values if only the first 10 are used, even if the array has 87 elements. This approach is written to reduce savegame file size.

I have been thinking about this for a while. What I don't like about it is what if the only non-default is element 86? It then saves 85 default values, which are useless and could be skipped.

Here is what I came up with:
We limit the length to 15 unsigned bits (32k).

JIT arrays should be saved in blocks. Each block starts with an info variable, which is an unsigned 32 bit int. This int contains start index and end index, allowing it to save say index 4 to 7. The actual data is then saved after that, just like now.
The info variable has 2 bits free. One of them is then used to tell if this is the last block in the array. If the last bit is not set, once the block has been read, a new block is read using the very same code.

This will allow us to save elements 4 through 7 and then 81 through 87 without wasting space on the default values in between. In fact no default values will ever be saved and the savegame file size should be significantly reduced.

The only problem is that things will go wrong if arrays are too long, but we already have a restriction of 65k and I think 32k should be more than enough. Still it would make sense to assert right after reading the XML files if this limit is exceeded. The same goes for max number of JIT array types (256). It's unlikely that we will exceed this, but I prefer to assert if it happens even if it is unlikely.

 

[Nightinggale]

If a function has a type as return type, that function should return that type.

int getIdea(bool Research, PlayerTypes ePlayer = NO_PLAYER) const;

This is bad by design because it mean that when it's called, it becomes:

(CivicType)getIdea()

Typecasting the return value is by definition bad. I changed the return type to TechTypes, but the compiler is perfectly happy using this value as CivicType due to typecasting. If no typecasting took place, the compiler would cause an error whenever it is used for the wrong type.

While it can't be completely avoided, typecasting should be reduced to a minimum and preferably never on return types.

 

[Nightinggale]

I added

template<class T>
CvXMLLoadUtility::GetChildXmlStringValByName(T* val, const TCHAR* szName)

It is used just like GetChildXmlValByName. The difference is that it works on enum types. I started using it with UnitClassTypes. It looks up the string in XML and then it looks up the index in the XML file in question, meaning the two line step from vanilla is now a one line step using this function.

I started using it for some UnitClassTypes, which then is stored as such in CvCivEffectInfo and the get functions return UnitClassTypes as well. I then removed the typecasts used where the get functions were called from. Now in the future if we change the return type of one of them because we changed something, the compiler will complain where it is called.

My current goal is to remove every single typecast to CivicTypes because half of them are wrong as it is right now.

 

[Nightinggale]

I had a major breakthough regarding looping enum values. I have never liked the way it is done where an int is looped and then typecast into an enum type. After some research I came up with this:

template <class T>
static inline T& operator++( T& c )
{
	c = (T)(c + 1);
	return c;
}

This enables the ++ operator on all enum types. It's now possible to write code like

for (TechTypes eTech = FIRST_TECH; eTech < GC.getNumTechInfos(); ++eTech)

The only problem is that it will then no longer start with 0, which mean I had to update the enum to look like this:

enum TechTypes
{
	FIRST_TECH = 0,
	NO_TECH = -1,
	// other enum values goes here.
};

Now it should be easier to write code for looping all types in an XML file. Also by avoiding the typecast the code will be less prone to bugs.

The reason why the enum goes 0, -1 is that when the next value is added, it will be +1 compared to the last, meaning it will start with 0 as we would expect.


Another idea regarding enums, that I came up with is to write a perl script, which reads the XML files, then adds the types to the enums using comments to define where to write the enum values. While we shouldn't hardcode those values in the DLL, adding them prior to debugging could be helpful because then instead of telling you that the TechTypes = 4, it will tell that it is MEDIEVAL_TECH_BLOCKPRINTING when you hover the mouse over the variable name. That makes it easier to figure out what goes on.
The extra enum values should be easy to clear too before committing. That gives an interesting assignment to the script, but the easier to read debugger would be worth this work.