Quick Modding Questions Thread
- Thread starter kiwitt
- Start date
And there's another question. I'm one step further, this time it's related to the plot danger function.
This is from Sanguo's performance improvements, which I think I got from the community patch or better BTS AI or something like that. In particular, it's about the following line:
The whole purpose of the function is to determine if there are units in surrounding tiles that threaten a given plot. The general principle is to see if the units are close enough to reach the tile in a turn, but instead of actually calculating a path it just takes the immediate distance, which I think is for performance reasons and fine by me. However, I cannot understand why the line in red exists. As far as I can tell, isValidRoute only checks if the route on the tile is usable by the unit:
However, it never checks if the route actually connects to the target plot and even then, why would that only increase the range by one?
To give some context, I am debugging the following situation:
The selected worker is interrupted every turn because the game considers it threatened. Debugging indicates that it's because of the units in Danibaan (I am at war with them), which are two tiles away from the worker. All units in the city have one move, which should be enough to consider the worker safe even if the peak wasn't there.
So why does the +1 for standing on a road exist? Just because it might be connected by road to the target tile and it would be too expensive to actually check?
Is there something smart I can do to prevent the game from excessively warning me in this case without removing some legitimate warnings when there actually is a road connecting the tiles, without doing some expensive pathing since this function is called rather often?
Code:
int CvPlayerAI::AI_getPlotDanger(CvPlot* pPlot, int iRange, bool bTestMoves) const
{
PROFILE_FUNC();
// Sanguo Mod Performance, start, added by poyuzhe 08.12.09
if (iRange == -1 || iRange > DANGER_RANGE)
{
iRange = DANGER_RANGE;
}
if (pPlot->getPlayerDangerCache(getID(), iRange) != MAX_SHORT)
{
return pPlot->getPlayerDangerCache(getID(), iRange);
}
else
{
CLLNode<IDInfo>* pUnitNode;
CvUnit* pLoopUnit;
CvPlot* pLoopPlot;
int iCount;
int iDistance;
int iBorderDanger;
int iDX, iDY;
CvArea *pPlotArea = pPlot->area();
iCount = 0;
iBorderDanger = 0;
/*if (iRange == -1)
{
iRange = DANGER_RANGE;
}*/
for (iDX = -(iRange); iDX <= iRange; iDX++)
{
for (iDY = -(iRange); iDY <= iRange; iDY++)
{
pLoopPlot = plotXY(pPlot->getX_INLINE(), pPlot->getY_INLINE(), iDX, iDY);
if (pLoopPlot != NULL)
{
if (pLoopPlot->area() == pPlotArea)
{
iDistance = stepDistance(pPlot->getX_INLINE(), pPlot->getY_INLINE(), pLoopPlot->getX_INLINE(), pLoopPlot->getY_INLINE());
if (atWar(pLoopPlot->getTeam(), getTeam()))
{
if (iDistance == 1)
{
iBorderDanger++;
}
else if ((iDistance == 2) && (pLoopPlot->isRoute()))
{
iBorderDanger++;
}
}
pUnitNode = pLoopPlot->headUnitNode();
while (pUnitNode != NULL)
{
pLoopUnit = ::getUnit(pUnitNode->m_data);
pUnitNode = pLoopPlot->nextUnitNode(pUnitNode);
if (pLoopUnit->isEnemy(getTeam()))
{
if (pLoopUnit->canAttack())
{
if (!(pLoopUnit->isInvisible(getTeam(), false)))
{
if (pLoopUnit->canMoveOrAttackInto(pPlot))
{
//if (!bTestMoves)
//{
//iCount++;
//}
//else
//{
int iDangerRange = pLoopUnit->baseMoves();
iDangerRange += ((pLoopPlot->isValidRoute(pLoopUnit)) ? 1 : 0);
if (iDangerRange >= iDistance)
{
iCount++;
}
//}
}
}
}
}
}
}
}
}
}
if (iBorderDanger > 0)
{
if (!isHuman() && !pPlot->isCity())
{
iCount += iBorderDanger;
}
}
pPlot->setPlayerDangerCache(getID(), iRange, std::min(iCount, MAX_SHORT));
return std::min(iCount, MAX_SHORT);
}
//return iCount;
// Sanguo Mod Performance, end
}
Code:
iDangerRange += ((pLoopPlot->isValidRoute(pLoopUnit)) ? 1 : 0);
Code:
bool CvPlot::isValidRoute(const CvUnit* pUnit) const
{
if (isRoute())
{
if (!pUnit->isEnemy(getTeam(), this) || pUnit->isEnemyRoute())
{
return true;
}
}
return false;
}To give some context, I am debugging the following situation:
Spoiler :
The selected worker is interrupted every turn because the game considers it threatened. Debugging indicates that it's because of the units in Danibaan (I am at war with them), which are two tiles away from the worker. All units in the city have one move, which should be enough to consider the worker safe even if the peak wasn't there.
So why does the +1 for standing on a road exist? Just because it might be connected by road to the target tile and it would be too expensive to actually check?
Is there something smart I can do to prevent the game from excessively warning me in this case without removing some legitimate warnings when there actually is a road connecting the tiles, without doing some expensive pathing since this function is called rather often?
Last edited:
devolution
Prince
You could always add a check for the tile being impassable as an additional condition. However, I just had a look at AI_getPlotDanger in K-Mod v1.46 which is is quite different from the BBAI one. Why don't you use it instead and then tweak the behavior, it appears more sophisticated to me...
Edit:
K-Mod is using the stepfinder, which should deal with the impassable peak case, I guess this is exactly what you want.
Edit:
K-Mod is using the stepfinder, which should deal with the impassable peak case, I guess this is exactly what you want.
Last edited:
Looks to me like K-Mod uses ::stepDistance, same as the Sanguo snippet: CvPlayerAI#L4482 [Edit: changed to a permalink.] K-Mod disables the caching mechanism though; see comments above CvPlayerAI::isSafeRangeCacheValid.
This is based on the visibility of pLoopUnit->getTeam. I don't think the pathfinder has an option for assuming another team's point of view, but it could probably be added to the K-Mod pathfinder.
Currently, the danger functions don't even check if pLoopPlot->isVisible, which should definitely be done for human players, nor pLoopPlot->getRevealedRouteType (should replace isRoute). This isn't going to take care of the tiles in between pLoopPlot and pPlot though, which could be unrevealed or fogged.
Another issue is that the canMoveOrAttackInto call in AI_getPlotDanger will check isMadeAttack, which shouldn't matter for an attack on the enemy's next turn. I've fixed that a while ago by passing a flag to canMoveOrAttackInto that tells it not to check isMadeAttack.
There are at least two other plot danger functions that follow largely the same logic as AI_getPlotDanger: AI_getAnyPlotDanger (in my tests, this one wakes up the Worker) and AI_isPlotThreatened. Changes to AI_getPlotDanger should arguably also be applied to those. (In my own code base, I see also AI_getWaterDanger, which never used a cache, and AI_getUnitDanger, an unused BBAI function.)
I think so. But if we check isHuman, then performance shouldn't be a problem, and we can call:
Code:
pLoopUnit->generatePath(pPlot, MOVE_MAX_MOVES, false, NULL, 1)Currently, the danger functions don't even check if pLoopPlot->isVisible, which should definitely be done for human players, nor pLoopPlot->getRevealedRouteType (should replace isRoute). This isn't going to take care of the tiles in between pLoopPlot and pPlot though, which could be unrevealed or fogged.
Another issue is that the canMoveOrAttackInto call in AI_getPlotDanger will check isMadeAttack, which shouldn't matter for an attack on the enemy's next turn. I've fixed that a while ago by passing a flag to canMoveOrAttackInto that tells it not to check isMadeAttack.
There are at least two other plot danger functions that follow largely the same logic as AI_getPlotDanger: AI_getAnyPlotDanger (in my tests, this one wakes up the Worker) and AI_isPlotThreatened. Changes to AI_getPlotDanger should arguably also be applied to those. (In my own code base, I see also AI_getWaterDanger, which never used a cache, and AI_getUnitDanger, an unused BBAI function.)
Last edited:
devolution
Prince
That's a nice and detailed answer!
Btw, some mods (Rise of Mankind \ C2C ) have added the capability to generate paths for "hypothetical" units which could be used to detect threats more reliably. Then we could finally deal with the "double-woodsman promoted warrior steals a worker" tactic...
Btw, some mods (Rise of Mankind \ C2C ) have added the capability to generate paths for "hypothetical" units which could be used to detect threats more reliably. Then we could finally deal with the "double-woodsman promoted warrior steals a worker" tactic...
Looks like the plot danger logic is a lot less robust than I thought. Do you really think isHuman() would cut down performance costs enough? It's still just reduction by a constant factor. I need to look later into how the K-Mod code works to understand the differences.
I'm assuming that most of the calls are coming from the various CvUnitAI::AI_*Move functions, which should never(?) be called on human units. Also, a generatePath call with iMaxPath=1 shouldn't take that long. It might even be OK for the AI, though with railroads and helicopters ...
Tentative implementation:
Spoiler :
I've noticed that CvPlayerAI::AI_isPlotThreatened already checks the path (regardless of isHuman). A K-Mod comment there says: "Use a temp pathfinder, so as not to interfere with the normal one." I'm not really seeing the need for that in AI_isPlotThreatened, but I guess it's generally safer. So I'd also change CvUnit::generatePath as follows:
Spoiler :
iMaxPath tells the pathfinder that the caller is only interested in tiles that the unit can reach in that many turns. So, in your screenshot, iMaxPath=1 shouldn't check what the shortest path around those mountains is for the enemy Archer, it should really only look at the two adjacent Plains tiles. I'm not all too familiar with the pathfinding code either, but from what I remember about the A* algorithm, that's how it should work. Here's the K-Mod code that enforces iMaxPath.
Hm. It looks like the BtS pathfinder doesn't have this parameter. I think this means e.g. that, if the destination is unreachable, the pathfinder will go through all tiles that the unit can reach (in any number of turns). That could be one of the main reasons why karadoc replaced the pathfinder. (The changelog only says that "[w]ith the new pathfinding engine, and related changes, the game now runs significantly faster.") Well, for the human danger checks, it shouldn't really matter. The thing with the temporary pathfinder may also be impossible without K-Mod, but, again, for human units, I don't see why it would be necessary.
if the warrior is there, threatening a worker, then no hypothetical unit should be needed, no? For my own mod, I needed a function to measure path lengths in between cities. At first, I had actually called generatePath on whatever units happened to be present in the city, so the RoM code would've been an improvement. By now, I have a proper solution based on an abandoned BBAI function (CvPlot.cpp#L3238).
Spoiler :
Interesting. Though ...Btw, some mods (Rise of Mankind \ C2C ) have added the capability to generate paths for "hypothetical" units which could be used to detect threats more reliably. Then we could finally deal with the "double-woodsman promoted warrior steals a worker" tactic...
if the warrior is there, threatening a worker, then no hypothetical unit should be needed, no? For my own mod, I needed a function to measure path lengths in between cities. At first, I had actually called generatePath on whatever units happened to be present in the city, so the RoM code would've been an improvement. By now, I have a proper solution based on an abandoned BBAI function (CvPlot.cpp#L3238).Oh, nevermind. There's quite a rat's tail of changes that comes with merging the K-Mod pathfinder, and I've already concluded that it is much smarter to port my mod into K-Mod than doing the reverse. I guess instead of attempting an intermediate solution I can live with the issue until I can actually do that merge.
Underdog Ally
Chieftain
- Joined
- Feb 22, 2019
- Messages
- 63
Hello, I am new to mods (and Civfanatics) and was wondering how difficult it is to create a new unit. At the moment I want to make a land troop transport such as a truck or helicopter that can carry infantry units. Or, perhaps a mod which is primarily oriented on additional late game stuff. I do not want 8,000 new units or a mod such as caveman2cosmos which takes me 6 months to play. I just want more unit variety within the modern era. As in different types of units, not extra eras or upgrades. Any suggestions would be mighty helpful. c:
Today I come with a possibly very basic C++ question but things just don't work how I would expect them to. I have this method:
It doesn't really matter why it exists, but for some context, this is a reimplementation of getCultureTimes100 that instead of just returning the actual value of m_iCultureTimes100 converts a percentage of it (i.e. 0-100%) from all other player to one "culture conversion" player's culture. Usually culture the conversion player is NO_PLAYER and the conversion rate is 0, resulting in the same behaviour as before (getActualCultureTimes100 just returns the value of m_iCultureTimes100 as before).
The problem here is that for long games on marathon, int values in m_iCultureTimes100 (or getActualCultureTimes100) can become so large that 100 times that overflows the integer. This isn't a problem for this function because it's multiplied by a less or equal number than it is afterwards divided by, so the ultimate result will always fit in an int.
Changing the order of operations can introduce rounding errors. Instead what I want is basically just that the calculation is performed on long, then casted to int again in the end because I know the result will fit in int. I tried to enforce this in multiple ways:
or even
But whatever I do, I still get the incorrect result due to integer overflow. What am I missing here?
Code:
int CvCity::getCultureTimes100(PlayerTypes ePlayer) const
{
if (plot()->getCultureConversionPlayer() == ePlayer)
{
return (getActualTotalCultureTimes100() - getActualCultureTimes100(ePlayer)) * plot()->getCultureConversionRate() / 100 + getActualCultureTimes100(ePlayer);
}
return getActualCultureTimes100(ePlayer) * (100 - plot()->getCultureConversionRate()) / 100;
}The problem here is that for long games on marathon, int values in m_iCultureTimes100 (or getActualCultureTimes100) can become so large that 100 times that overflows the integer. This isn't a problem for this function because it's multiplied by a less or equal number than it is afterwards divided by, so the ultimate result will always fit in an int.
Changing the order of operations can introduce rounding errors. Instead what I want is basically just that the calculation is performed on long, then casted to int again in the end because I know the result will fit in int. I tried to enforce this in multiple ways:
Code:
return (long)getActualCultureTimes100(ePlayer) * (100l - (long)plot()->getCultureConversionRate()) / 100l;
Code:
long lActualCultureTimes100 = (long)getActualCultureTimes100(ePlayer);
long lConversionRate = (long)plot()->getCultureConversionRate()
return lActualCultureTimes100 * (100l - lConversionRate) / 100l;devolution
Prince
A bit short on time so I don't have time to review this code, but be aware that you probably want "long long" or int64_t to hold a 64-bit result. Long is only guaranteed to be able to hold (at least) a 32-bit result.
Last edited:
I'll give it a try, but isn't 32 bit enough for x * 100 if x fits in a 16 bit int?
Edit: it works when casting to long long at least, but that's still a bit (no pun intended) surprising to me.
Edit2: confirmed, the solution that works with long long does not with long. Learned somethin again. Thanks!
Edit: it works when casting to long long at least, but that's still a bit (no pun intended) surprising to me.
Edit2: confirmed, the solution that works with long long does not with long. Learned somethin again. Thanks!
Last edited:
Sorry if I stole attention from your question, but if you need general information about adding units I suggest just looking into the tutorials section, which covers that kind of thing. I think several mods have already done the land transport unit in particular (e.g. the Humvee unit in History Rewritten, iirc) where you can look it up as an example.
But! I also have another question for myself. Lately, when debugging crashes using the debug DLL and Visual Studio, I often would run into the problem that the crash happens, Visual Studio shows the popup with "unhandled exception" or whatever the detected issue is. But then it's impossible to get the focus from the BtS application to VS to actually click the "break" button and get to the stacktrace. In fact, it's usually impossible to get the focus away from BtS at all (the cursor remaining BtS's spinning globe) and I have to kill the VS process to shut both applications down, which of course also means I lose the debug information I was originally looking for.
Is there anything I can do about this or has anyone similar experiences at all? I'm on VS2010 and Win7 if that's relevant.
Is there anything I can do about this or has anyone similar experiences at all? I'm on VS2010 and Win7 if that's relevant.
I can only reproduce this by debugging in fullscreen mode – so I never do that. I can get out of fullscreen through Ctrl+Alt+Del (on Windows 8.1) via the Task Manager. BtS retains the mouse, but the keyboard works (e.g. Enter to break and Alt+D for the VS debug menu). However, when VS2010 stops responding, it seems impossible to kill the BtS process through the Task Manager – "End Task" has no effect.
Yep that's my process too, switch task via the task manager. But I didn't consider that keyboard may work when mouse doesn't, will try that, thanks! I am also running BtS in full screen out of habit, windowed mode makes sense when debugging.
