Following what we've been talking in the 'I want to help this mod' thread, I started this thread so we can come together with ideas for redesigning the Trade Routes System.
For non-modders:
Simply put, it takes too long when the number of routes per city gets high enough, and the number of cities as well. As an example, if we have 4 players each with 100 cities, imagine each city with the ability to make 10 Trade Routes, every player is allowed to make trade routes with each other, and every city is connected. This means that when you pass your turn, for each of the players (4) we will first organize all 100 cities so the best are first. Then for each of these cities, we will check all cities that it can establish a trade route with (400 possibilities) and organize them from best to worst, but discarding anything that goes over the number of trade routes for the city (10) after evaluating the trade value of each other city. Finally, with the best 10 cities to establish a trade route for each city, it is done. This means it'll take: 4(players)*100(cities)*400(total connected cities)*X(time to organize the best 10 trade routes to make)*MaxTradeRoutes(currently 24, I'll talk about this later in this post) = 3,840,000 instructions for the computer to process (times X, but as X is completely unpredictable, I didn't put it in the result).
For modders:
Guys, this is a sick algorithm really:
For each Player (in CvPlayer.cpp and using the same values in the example above):
First for all cities of a given player clear max trade routes in each city (e.g. 100*24 = 2400 instructions)
Then for each city we check its supposed position in an organized list(min 100 max 5050 instructions)
Only after this we start calculating trade routes for each city.
Now in CvCity.cpp:
24 cleaning instructions, as iMaxTradeRoutes is supposed to be 24.
Then we clear Trade Routes for the city again (more 24 instructions), and we enter the possible trade routes list
So for each Player(4) for each city(100 of each):
It finally loops for each available trade route (10) and assign the cities in order, discarding the overflow, which gives a maximum of 3955 instructions (the city itself is discarded in the initial if statement, and any city of another team is discarded if there already exists a trade route between this player and that city).
After that it loops through Max Trade Routes (24) to calculate the yields of all set Trade Routes
------------------------------------------------------------------------
My plan is to rethink the entire system, although I'm not full of ideas. I prefer to try to transform an idea into code then to give an idea by myself. And I know this may take some time, so while we discuss ideas here, there are a few stuff that I can adress here that maybe could make the current system a little better:
Regarding process speed:
I can't see a reason to loop through Max Trade Routes when a simple loop through Trade Routes is enough. There are several loops through Max Trade Routes that are only concerned with actual trade routes, and ignore each entry that is NULL (each index between Trade Routes and Max Trade Routes -1). This is just a tip of the iceberg in process time, but maybe it'll improve a bit and it's easy to fix. The only thing that pops to mind against this idea is multi-processing. Maybe iTradeRoutes can change between calculating which will be the cities to make routes and assigning the yields to each route. I'm unfamiliar with multi-processing, I've started to learn about it this semester on college.
Regarding gameplay:
The problem with Teams: Consider this part
This happens while we're checking if the city being looped is a possible candidate to establish a trade route with (by the perspective of the city object we're currently on). This only fails if the loop city isn't in the same Team as object city and loop city already has a route with any city of object city's owner.
I don't know everything that makes a player a part of the same team of another player, but I remember in LoR that the Permanent Alliance treaty created a paradox, and could be seriously exploited in favor of the player who establishes such contract, because on one hand your perm buddy is in the same team as you, so his cities can be used several times for trade routes just like your own cities, but on the other hand they were considered foreign cities when calculating yields, which gave them 100% boost in value if I remember correctly. I didn't venture into C2C's code to see if here it's still like this, but it's a mistake that can propagate to many other forms of contract (like Vassal State, or any other possibility that exists or that may exist in the future that could mess with team and foreign identities regarding Trade Routes).
The problem with Order:
This one I've seem in Realms Beyond forums a long time ago. Because of the same order of players being looped for any city, Player 1 has the most advantage while the last player has no advantage. The advantage comes in the form of Trade Routes Slots being filled with your own cities rather then with foreign cities. Imagine a city belonging to player 1 being object city. When we loop through the possible cities to make a route with, we first go through all Player 1 cities and only then we check foreign cities. This means that if any ties occur between a national city and a foreign city, the national will be preferred as it got the slot first. As national cities never leave the pool of available cities, this can mean an increment in for player 1, while the last player will first loop through all foreign cities and only then he will check his own cities, and in case of any ties, the foreign cities will be preferred because they are already filling the available slots.
This isn't gamebreaking, but can be used in Player's 1 advantage for competitive games. The people in the Realms Beyond forums reached that conclusion and fixed this for their tournaments.
Regarding gamedesign:
I feel confused with the concept of Max Trade Routes. Everytime the program checks what's the number of Max Trade Routes it gets a DEFINE and sums it with a modifier. This modifier comes from traits. Now, I'm unaware if there are active leader traits that give trade routes or max trade routes, but this seems wrong to me. Imagine I want to test C2C with no trade routes. Then I'd go to global defines to change it there to 0. But suddenly one or more players have trade routes and others don't. Unless I'm aware of this strange concept in CvPlayer.cpp, I'll believe the game is broken. And if I don't discover there are indeed trade routes in the game I tried to make without them, my test will probably be fake and I won't know it. In vanilla (or maybe LoR) Max trade routes were 8, and if I remember correctly only one civilization could get all 8 because of a UB. But every other civilization had Max at 8, even if it couldn't achieve it in game.
For non-modders:
Simply put, it takes too long when the number of routes per city gets high enough, and the number of cities as well. As an example, if we have 4 players each with 100 cities, imagine each city with the ability to make 10 Trade Routes, every player is allowed to make trade routes with each other, and every city is connected. This means that when you pass your turn, for each of the players (4) we will first organize all 100 cities so the best are first. Then for each of these cities, we will check all cities that it can establish a trade route with (400 possibilities) and organize them from best to worst, but discarding anything that goes over the number of trade routes for the city (10) after evaluating the trade value of each other city. Finally, with the best 10 cities to establish a trade route for each city, it is done. This means it'll take: 4(players)*100(cities)*400(total connected cities)*X(time to organize the best 10 trade routes to make)*MaxTradeRoutes(currently 24, I'll talk about this later in this post) = 3,840,000 instructions for the computer to process (times X, but as X is completely unpredictable, I didn't put it in the result).
For modders:
Guys, this is a sick algorithm really:
Spoiler :
For each Player (in CvPlayer.cpp and using the same values in the example above):
First for all cities of a given player clear max trade routes in each city (e.g. 100*24 = 2400 instructions)
Code:
for (pLoopCity = firstCity(&iLoop); pLoopCity != NULL; pLoopCity = nextCity(&iLoop))
{
pLoopCity->clearTradeRoutes();
}
Code:
void CvCity::clearTradeRoutes()
{
CvCity* pLoopCity;
int iI;
int iMaxTradeRoutes = GC.getDefineINT("MAX_TRADE_ROUTES") + GET_PLAYER(getOwnerINLINE()).getMaxTradeRoutesAdjustment();
for (iI = 0; iI < iMaxTradeRoutes; iI++)
{
pLoopCity = getTradeCity(iI);
if (pLoopCity != NULL)
{
pLoopCity->setTradeRoute(getOwnerINLINE(), false);
}
m_paTradeCities[iI].reset();
}
}
Code:
for (pLoopCity = firstCity(&iLoop); pLoopCity != NULL; pLoopCity = nextCity(&iLoop))
{
iTotalTradeModifier = pLoopCity->totalTradeModifier();
pCityNode = cityList.head();
while (pCityNode != NULL)
{
pListCity = getCity(pCityNode->m_data);
if (iTotalTradeModifier > pListCity->totalTradeModifier())
{
cityList.insertBefore(pLoopCity->getID(), pCityNode);
break;
}
else
{
pCityNode = cityList.next(pCityNode);
}
}
if (pCityNode == NULL)
{
cityList.insertAtEnd(pLoopCity->getID());
}
}
Now in CvCity.cpp:
24 cleaning instructions, as iMaxTradeRoutes is supposed to be 24.
Code:
for (iI = 0; iI < iMaxTradeRoutes; iI++)
{
paiBestValue[iI] = 0;
}
Code:
for (iI = 0; iI < MAX_PLAYERS; iI++)
{
if (GET_PLAYER(getOwnerINLINE()).canHaveTradeRoutesWith((PlayerTypes)iI))
{
for (pLoopCity = GET_PLAYER((PlayerTypes)iI).firstCity(&iLoop); pLoopCity != NULL; pLoopCity = GET_PLAYER((PlayerTypes)iI).nextCity(&iLoop))
{
Code:
if (pLoopCity != this)
{
if (!(pLoopCity->isTradeRoute(getOwnerINLINE())) || (getTeam() == GET_PLAYER((PlayerTypes)iI).getTeam()))
{
if (pLoopCity->plotGroup(getOwnerINLINE()) == plotGroup(getOwnerINLINE()) || GC.getDefineINT("IGNORE_PLOT_GROUP_FOR_TRADE_ROUTES"))
{
// BUG - Fractional Trade Routes - start
#ifdef _MOD_FRACTRADE
iValue = calculateTradeProfitTimes100(pLoopCity);
#else
iValue = calculateTradeProfit(pLoopCity);
#endif
// BUG - Fractional Trade Routes - end
for (iJ = 0; iJ < iTradeRoutes; iJ++)
{
if (iValue > paiBestValue[iJ])
{
for (iK = (iTradeRoutes - 1); iK > iJ; iK--)
{
paiBestValue[iK] = paiBestValue[(iK - 1)];
m_paTradeCities[iK] = m_paTradeCities[(iK - 1)];
}
paiBestValue[iJ] = iValue;
m_paTradeCities[iJ] = pLoopCity->getIDInfo();
break;
}
}
}
}
}
After that it loops through Max Trade Routes (24) to calculate the yields of all set Trade Routes
------------------------------------------------------------------------
My plan is to rethink the entire system, although I'm not full of ideas. I prefer to try to transform an idea into code then to give an idea by myself. And I know this may take some time, so while we discuss ideas here, there are a few stuff that I can adress here that maybe could make the current system a little better:
Regarding process speed:
I can't see a reason to loop through Max Trade Routes when a simple loop through Trade Routes is enough. There are several loops through Max Trade Routes that are only concerned with actual trade routes, and ignore each entry that is NULL (each index between Trade Routes and Max Trade Routes -1). This is just a tip of the iceberg in process time, but maybe it'll improve a bit and it's easy to fix. The only thing that pops to mind against this idea is multi-processing. Maybe iTradeRoutes can change between calculating which will be the cities to make routes and assigning the yields to each route. I'm unfamiliar with multi-processing, I've started to learn about it this semester on college.
Regarding gameplay:
The problem with Teams: Consider this part
Code:
if (!(pLoopCity->isTradeRoute(getOwnerINLINE())) || (getTeam() == GET_PLAYER((PlayerTypes)iI).getTeam()))
This happens while we're checking if the city being looped is a possible candidate to establish a trade route with (by the perspective of the city object we're currently on). This only fails if the loop city isn't in the same Team as object city and loop city already has a route with any city of object city's owner.
I don't know everything that makes a player a part of the same team of another player, but I remember in LoR that the Permanent Alliance treaty created a paradox, and could be seriously exploited in favor of the player who establishes such contract, because on one hand your perm buddy is in the same team as you, so his cities can be used several times for trade routes just like your own cities, but on the other hand they were considered foreign cities when calculating yields, which gave them 100% boost in value if I remember correctly. I didn't venture into C2C's code to see if here it's still like this, but it's a mistake that can propagate to many other forms of contract (like Vassal State, or any other possibility that exists or that may exist in the future that could mess with team and foreign identities regarding Trade Routes).
The problem with Order:
This one I've seem in Realms Beyond forums a long time ago. Because of the same order of players being looped for any city, Player 1 has the most advantage while the last player has no advantage. The advantage comes in the form of Trade Routes Slots being filled with your own cities rather then with foreign cities. Imagine a city belonging to player 1 being object city. When we loop through the possible cities to make a route with, we first go through all Player 1 cities and only then we check foreign cities. This means that if any ties occur between a national city and a foreign city, the national will be preferred as it got the slot first. As national cities never leave the pool of available cities, this can mean an increment in for player 1, while the last player will first loop through all foreign cities and only then he will check his own cities, and in case of any ties, the foreign cities will be preferred because they are already filling the available slots.
This isn't gamebreaking, but can be used in Player's 1 advantage for competitive games. The people in the Realms Beyond forums reached that conclusion and fixed this for their tournaments.
Regarding gamedesign:
I feel confused with the concept of Max Trade Routes. Everytime the program checks what's the number of Max Trade Routes it gets a DEFINE and sums it with a modifier. This modifier comes from traits. Now, I'm unaware if there are active leader traits that give trade routes or max trade routes, but this seems wrong to me. Imagine I want to test C2C with no trade routes. Then I'd go to global defines to change it there to 0. But suddenly one or more players have trade routes and others don't. Unless I'm aware of this strange concept in CvPlayer.cpp, I'll believe the game is broken. And if I don't discover there are indeed trade routes in the game I tried to make without them, my test will probably be fake and I won't know it. In vanilla (or maybe LoR) Max trade routes were 8, and if I remember correctly only one civilization could get all 8 because of a UB. But every other civilization had Max at 8, even if it couldn't achieve it in game.