Trading units is kind of broken? upd: and my proposal how to fix

[valergrad]

Hi all,

I've recently returned to this genius mod ROM:AND and started a new game.
As usually i am playing at deity difficulty with one city on giant map with turned off tech diffusion and without a realistic timescale , and this means that I am trying to get as much as possible from each possibility. And still far away from AI - for example now I just've researched ship building and built my first galley and some AI's are already close to space ( this is due to another bug , anyway, I'll create for this separate post ).

And apparently, as I see I can get a huge naval army and a lot of technologies with just one galley.
For example, I've started trading with De Gaulle ( DG in this post).

First I traded my Galley with DG on his Galleon - it's already kind of joke as Galley is much cheaper and worse than Galleon.
Then I traded this Galleon with his Galleon and Flute - so, now I have Galleon and a Flute.
Then I continued to trade like this until I got from from him all his navy - 3 galleons, galleas and flute.
Then I started getting Technoligies. I traded several ships on a Scripture ( with a very profitable exchange rate, like 1 hammer = 6 bulbs or something) , and then returned Galleons back with the same technic
Then I got Sculpture and Ethics with the same way - and returned the fleet again.
Continuing this I could probably get all navy, all siege units and all technologies from all civilizations just in several moves.

So, this is definitely broken, and probably requires some investigation.
What do you think? I started to look at this function for calculating the trade value of the unit - why it is so inconsistent.
First I check A new dawn.log, just beginning of a trading process:

[194507.734] AI Unit Value For Galley is 462
[194544.406] AI Unit Value For Galleon is 462
[194546.781] AI Unit Value For Galley is 607
[194546.781] AI Unit Value For Galleon is 462
[194549.438] AI Unit Value For Galley is 607
[194549.438] AI Unit Value For Galleon is 462
[194607.094] AI Unit Value For Galley is 607

[194707.109] AI Unit Value For Galleon is 858
[194707.109] AI Unit Value For Fluyt is 732
[194707.109] AI Unit Value For Galleon is 858
[194707.109] AI Unit Value For Galleon is 858
[194707.109] AI Unit Value For Galleass is 244
[194715.875] AI Unit Value For Galleon is 858
[194715.875] AI Unit Value For Galleon is 462
[194715.875] AI Unit Value For Fluyt is 732
[194715.875] AI Unit Value For Galleon is 858
[194715.875] AI Unit Value For Galleass is 244

So, we already see that AI Unit Value for Galley and Galleon is the same - 462.
But later it changed for Galley even to 607 and all this values continue changed in an almost randomly looking at first sight way. I have taken logs from this just one trade window , got all distinct values, ordered records and got this:

Ballista is 501
Ballista is 732
Bombard is 1135
Bombard is 1161
Bombard is 1848
Bombard is 2310
Bombard is 897
Brigantine is 1650
Brigantine is 825
Cannon is 1656
Cannon is 3709
Caravel is 495
Fluyt is 396
Fluyt is 561
Fluyt is 732
Galleass is 184
Galleass is 244
Galleass is 303
Galleass is 66
Galleon is 1042
Galleon is 462
Galleon is 653
Galleon is 858
Galley is 462
Galley is 607
Great General is -13

So, during the same trade window AI estimate for Galleass, for example, was floating from 66 to 303.
For Galleon - from 462 to 1052.
For Galley - from 462 to 607 ( ridiculously high, because apparently you can get a tech with 452 bulbs just on galley).
And how it got in trades Great General ( this was never on the table ) I even have no idea, and no idea why it's cost is negative.

Now we are looking into a code.

For this kind of units it calculates like this ( CvPlayerAI::AI_militaryUnitTradeVal ) :

UnitTypes eBestUnit = bestBuildableUnitForAIType(pUnit->getDomainType(), eAIType);

int iBestUnitAIValue = AI_unitValue(eBestUnit, (UnitAITypes)GC.getUnitInfo(eUnit).getDefaultUnitAIType(), getCapitalCity()->area());
int iThisUnitAIValue = AI_unitValue(eUnit, (UnitAITypes)GC.getUnitInfo(eUnit).getDefaultUnitAIType(), getCapitalCity()->area());

// Value as cost of production of the unit we can build scaled by their relative AI value
iValue = (iThisUnitAIValue * GC.getUnitInfo(eBestUnit).getProductionCost())/std::max(1,iBestUnitAIValue);

// Normalise for game speed, and double as approximate hammer->gold conversion
iValue = (2 * iValue * GC.getGameSpeedInfo(GC.getGameINLINE().getGameSpeedType()).getTrainPercent()) / 100;

So, we take getProductionCost of the best unit and adjust it on the coefficient depending of ratio between unitAIValue of this unit and unitAIValue of the best buildable unit of that domain. And later normalize this based on our speed and multiply by 2 * 3.3 = 6.6 as train percent on normal speed is 330.

This sounds reasonable on a first sight. So, if some unit is really outdated, we don't value it too much - e.q. Galleys and the era of Galleons should not cost probably even their hammers
But it is wrong and I'll show - why.

For example, in our case. Fluyt, Galleon, Galley, Galleas all have domain DOMAIN_SEA.
Their possible AI's are: UNITAI_ASSAULT_SEA. ESCORT_SEA, SETTLER_SEA, MISSIONARY_SEA, SPY_SEA.
ATTACK_SEA, RESERVE_SEA, EXPLORE_SEA, ASSAULT_SEA.
Galley's cost is 25, Galleas - 65, Fluyt - 85, Galleon - 130.
Combat values are: 3, 10, 10, 16
Moves are: 2 , 2, 3, 4.
Cargo for them are: 2, 0, 3, 4.

Now I am checking function AI_unitValue - it is really big and complicated. But if we will read only the lines that are applicable to our case we see. First we add some value for AI preference.

iValue = 1;

iValue += GC.getUnitInfo(eUnit).getAIWeight();

This doesn't matter in our case as both leaders doesn't have preference for sea units.

Later we add into this values based on units parameters:

case UNITAI_ASSAULT_SEA:
case UNITAI_SETTLER_SEA:
case UNITAI_MISSIONARY_SEA:
case UNITAI_SPY_SEA:
iValue += (iCombatValue / 2);
iValue += (GC.getUnitInfo(eUnit).getMoves() * 200);
// Thomas SG - AC: Advanced Cargo START
{
iValue += (std::min(2,GC.getUnitInfo(eUnit).getTotalCargoSpace()) * 300);
}
// Thomas SG - AC: Advanced Cargo END
/************************************************************************************************/
/* BETTER_BTS_AI_MOD 05/18/09 jdog5000 */
/* */
/* City AI */
/************************************************************************************************/
// Never build galley transports when ocean faring ones exist (issue mainly for Carracks)
iValue /= (1 + AI_unitImpassableCount(eUnit));

So, first we need to know combat value for the unit. This is the function:

int CvGameAI::AI_combatValue(UnitTypes eUnit)
{
int iValue;

iValue = 100;

if (GC.getUnitInfo(eUnit).getDomainType() == DOMAIN_AIR)
{
iValue *= GC.getUnitInfo(eUnit).getAirCombat();
}
else
{
iValue *= GC.getUnitInfo(eUnit).getCombat();

// UncutDragon
// original
//iValue *= ((((GC.getUnitInfo(eUnit).getFirstStrikes() * 2) + GC.getUnitInfo(eUnit).getChanceFirstStrikes()) * (GC.getDefineINT("COMBAT_DAMAGE") / 5)) + 100);
// modified
iValue *= ((((GC.getUnitInfo(eUnit).getFirstStrikes() * 2) + GC.getUnitInfo(eUnit).getChanceFirstStrikes()) * (GC.getCOMBAT_DAMAGE() / 5)) + 100);
// /UncutDragon
iValue /= 100;
}

iValue /= getBestLandUnitCombat();

return iValue;
}

So, basically it is just combat value divided on getBestLandUnitCombat(). What is interested about this function, that it takes into account all units from all players in the game ( and not only land units, actually...). So, one person that will get some strong unit dramatically decrease the combat strength estimate of all players even if they have no idea of that unit or that player. It would be hard for me to know it, but fortunately there is interface for it in cygame, so by console command I've got it and it is 50 in the current game.

So, for Galley we will get: 1 + 200/50/2 + 400 + 600 = 1003 ( so, combat value as you see - doesn't matter at all under this conditions! )
Galleass: 1 + 1000/50/2 + 400 + 0 = 411 ( and now Galley is twice as valuable as Galleas because of ignoring combat value... )
Fluyt: 1 + 1000/50/2 + 600 + 600 = 1211
Galleon: 1 + 1600/50/2 + 800 + 600 = 1417.

I have checked this with console:

v_area = gc.getPlayer(12).getCapitalCity().area()
gc.getPlayer(12).AI_unitValue(194, 28, v_area)
1004
gc.getPlayer(12).AI_unitValue(201, 28, v_area)
1417

Where unit 194 is Galley and 201 is Galleon.


Now, I trade propose DG to trade Galley to Galleon and I get this:

[208604.578] AI Unit Value For Galley is 904
[208604.578] AI Unit Value For Galleon is 462

How game engine converts now ( 1004, 1417) into ( 904, 462 ) I have no idea.
It has no sense at all. The final value should be something close to the construction values - 25 and 120.
And after the trade if I try to trade Galleon with Galleass I get this:

[210670.234] AI Unit Value For Galleass is 66
[210670.234] AI Unit Value For Galleon is 1280

For now I have to do some pause as I need some sleep. I can not debug this as I don't have Visual Studio 2008, and I am out of fantasy what can I check in Python.
But I think that using complicated function AI_unitValue was a very bad idea , with simple getProductionCost you would get much more stable and fair estimate. And this AI_unitValue with all it's random numbers like:
add 200 for movement, add 300 for cargo space, add 1 for an improvement etc. looking like giving mostly random number without any sense.

 

[valergrad]

The only idea is that problem is in function CvCityAI::AI_bestUnitAI that is used for determining 'the best buildable unit' for the player. It is another ridiculously large function ( i starting to understand why ROM:AND is so slow... ) that has a lot of logic related with case, where units are built on the food. And De Gaulle actually has a 'standing army' civic at the moment.

 

[valergrad]

Okay, I've managed to install Visual Studio Community 2022.
Debugging is hanging the whole computer for some reason, but I've added some logging, and this is what I've got.
With this function:

else
{
int iBestUnitAIValue = AI_unitValue(eBestUnit, (UnitAITypes)GC.getUnitInfo(eUnit).getDefaultUnitAIType(), getCapitalCity()->area());
int iThisUnitAIValue = AI_unitValue(eUnit, (UnitAITypes)GC.getUnitInfo(eUnit).getDefaultUnitAIType(), getCapitalCity()->area());

GC.getGameINLINE().logMsg("AI unit estimate start");
GC.getGameINLINE().logMsg("AI Unit initial Value. Best unit %S is %d", GC.getUnitInfo(eBestUnit).getDescription(), iBestUnitAIValue);
GC.getGameINLINE().logMsg("AI Unit initial Value. Current unit %S is %d", GC.getUnitInfo(eUnit).getDescription(), iThisUnitAIValue);

// Value as cost of production of the unit we can build scaled by their relative AI value
iValue = (iThisUnitAIValue * GC.getUnitInfo(eBestUnit).getProductionCost())/std::max(1,iBestUnitAIValue);

GC.getGameINLINE().logMsg("AI Unit initial Value. Best unit %S production cost is %d", GC.getUnitInfo(eBestUnit).getDescription(), GC.getUnitInfo(eBestUnit).getProductionCost());
}

GC.getGameINLINE().logMsg("Train percent is %d", GC.getGameSpeedInfo(GC.getGameINLINE().getGameSpeedType()).getTrainPercent());
// Normalise for game speed, and double as approximate hammer->gold conversion
iValue = (2 * iValue * GC.getGameSpeedInfo(GC.getGameINLINE().getGameSpeedType()).getTrainPercent()) / 100;
}

GC.getGameINLINE().logMsg("AI Unit Value For %S is %d", GC.getUnitInfo(eUnit).getDescription(), iValue);
GC.getGameINLINE().logMsg("AI unit estimate end");
return iValue;

This is what I've got:

[1404.968] AI unit estimate start
[1404.968] AI Unit initial Value. Best unit Galley is 502
[1404.968] AI Unit initial Value. Current unit Galleon is 1417
[1404.968] AI Unit initial Value. Best unit Galley production cost is 25
[1404.968] Train percent is 330
[1404.968] AI Unit Value For Galleon is 462
[1404.968] AI unit estimate end
[1404.968] AI unit estimate start
[1404.968] AI Unit initial Value. Best unit Privateer is 1019
[1404.968] AI Unit initial Value. Current unit Galley is 1004
[1404.968] AI Unit initial Value. Best unit Privateer production cost is 140

So, now the math is okay.
Value for Galleon is based on 'Galley' as best unit of this type, so it will be : 1417 * 25 / 502 = 70, that is later multiplied on 6.6 because of the game speed ( this is another problem, why trainPercent is 330, but unit cost is actually multiplied on 1.8? ) that give us final estimate for Galleon as 462.

Value for Galley is based on 'Privateer' as best unit of this type, so it will be: 1004 * 140 / 1019 = 137 ( rounded), and multiplied on 6.6 it give us 904.
So, basically there are two problems.
First is that 'best buildable unit' function seems returns nonsence. 'bestBuildableUnit' as Galley when you already can build Galleons ... oh, gosh. Yes, it is cheaper, but guy who decided to use 'bestbuildableunit' in this logic probably haven't thought about cost efficiency.

Second is that even without this function AI_unitValue seems doesn't have a lot of sense. In it's own reality it estimate Galley as 1004, and Privateer as 1019. Because Galley has 2 cargo slots that are weights 300 hammers each ( don't forget that this estimate is actually in hammers later! ). And strength doesn't matter at all, because somewhere on the map there is unit with strength 50 ( How it appeared I'll investigate next time ).



To resume. How it can be fixed?
Previously I had some experience in writing chess AI. And the rule that every AI engineer knows is that :

'Simple, even stupidly simple logic, but without any bugs in implementation, in the real world works much, much better then complicated, clever logic that was never tested and is full of bugs'.

That's why all this mods actually have very bad AI, despite on many descriptions in the mod features about 'improving AI'. People write complicated, clever logic, but as it was never tested properly - it is full of bugs, and this means that this logic works much worse that even simple logic, that was there before.

Probably we can never expect good testing in this fan mods, as everybody wants to develop a new features, not a test an old ones.
That's why I think this logic we should have as simple, as possible.
My proposal is to change it to:

iValue = GC.getUnitInfo(eUnit).getProductionCost() ;
iValue = (2 * iValue * GC.getGameSpeedInfo(GC.getGameINLINE().getGameSpeedType()).getTrainPercent()) / 100;
return iValue;

The justification is very simple: when we are trading unit, it doesn't matter how important this unit is for us. It is not even matter how important this unit for the second side of the trade.
What is matter only: how this unit is good in general. And production cost already shows this pretty well, so no need to overcomplicate this by additional logic. Any additional logic like 'give bonus for speed', 'give bonus for cargo' etc. will just cause that clever opponent ( i.e. human ) will build unit with low cost and high estimate and trade AI with it.
Also, this will make this function 100 times faster. If we have such utterly complicated ( and wrong ) logic everywhere else, this means that this mod can be really make faster and with improved AI at the same time.

If we don't want it to be simple, we can add one thing that actually matter and still leave it simple and stable: number of promotions. No doubts that unit with 5 promotions is better then unit with 0 of them. So, to prevent AI of exchanging promoted units to not promoted ones -> may be we can update and add, for example, 10 or 20% of getProductionCost() for each promotion that unit has.

Two things that may be we can discuss as well.
1. Why we multiply this on TrainPercent i.e. 3.3 on Deity, when cost of unit is actually depends on something else ( in my game it is coefficient 1.8 e.g. Galley costs 45 instead of 25, Workboat - 54 instead of 30 etc. )?
2. Is 1 hammer is really cost 2 gold? By this rate it means that most profitable for you would be build siege units ( they have buildings with 100% modifier ) and get incredible amount of gold/science bakers from that.
3. Do we need to add here 'penalty' for Game levels? I mean, when you trading Techs on Deity you usually need to propose AI much better conditions, i.e. trade Tech with 1000 bakers for 500 bakers etc. Looks like this doesn't apply to unit trading, or am I wrong here?

 

[Vokarya]

I like this, but I don't have the DLL-level modding skills needed to implement it. I agree that simplifying mechanics is at least worth looking at. I like what we did to trade routes because that was way too complex, and I'm still trying to figure out how the "financial trouble" mechanic works in Revolutions.

 

[valergrad]

I like this, but I don't have the DLL-level modding skills needed to implement it.

Thank you for you answer!
Working with DLL in ROM:AND is actually much simpler then with Python ( because of how Python code written...) - and as I've seen you can do changes in Python, that means that you can in DLL easily.
I can show you in some google meet session - but you just need to download/install Visual Studio first. Or I can do this myself. I am currently passively thinking what actually would be the best function for units trade value.

Btw, in the meantime, whilst I've been looking at barbarian thing, I've got the answer on my first question:

1. Why we multiply this on TrainPercent i.e. 3.3 on Deity, when cost of unit is actually depends on something else ( in my game it is coefficient 1.8 e.g. Galley costs 45 instead of 25, Workboat - 54 instead of 30 etc. )?

This is because unit cost in the game is also multiplied on the coefficient based on the era, the game started. For example, I've started from ancient and coefficient for that era is 0.55.
But this function for unit trade value somehow 'forgot' about this coefficient, and 'thinks' that all units costs almost twice more then they really are. Another reason why trade in AND so overestimate units.
If we fix it together with removing BestUnit logic then, for example, Galley will have value 108, instead of, for example, that we have - 1004.

108 sounds much more reasonable, while 1004 was total out of sense. Just for comparison, 'Ship Building' tech in this game costs 768 beakers.

While I've been passively thinking about this... I think if we want to improve it, we also can think of another thing.
Why you get unit immediately in your capital when you've bought it?
For example, imagine Giant Earth Map and Japan that is buying Galley from England. It will immediately appear in Kioto. This have no sense - at that time that travel would take years, if possible at all.
So, I think that when you buying unit - it should stay where it was, you're just getting control. No 'teleportation'.
So, you if you bought unit from far-far-away kingdom, it will be your own task to return them to your territory to help with defending, for example. And you'll pay support cost for distant units all that time. And probably they will not return alive, as there are many barbarians on the way, you know...

Of course, you can immediately imagine how human player can exploit this. For example, Japan buys units from England, next turn it declare war to them and attack with that units as they will be already near England's border.
I can imagine two solutions to prevent this. One is easier to implement, second is more realistic and interesting.



1. When you buy units from somebody you have 10-turns peace agreement, as we have now, for example, when you asking for a gift. Of course clever human player can try to exploit this as well. For example, to defend himself from attack of a strong AI, just buy/sell or just gift to him a cheap unit every 11 turns. But we already have this exploit in place with gifts - and it is even in Vanilla, look like that became part of the game.

2. When you declare war to somebody, all units that you've bought from him for the last N turns 'betray' you and returns under the control of the civ that sold them. This is actually what happened many times in history. So, you'll have possibility to start war even with somebody who just sold you units, just doesn't count on them in this future war.