Advanced Civ

keldath · Jun 26, 2024

Ah so you got some updates lurking around...i see . Then i shall wait on your new updates. I kinda finished my newest.

Perhaps in my game there were some more conditions.
Ill pack up my version with the save. Youll be able to debug its path (like you suggested,i guess its the isdue and with the flags).
Although my patch check for canmove worked,its just a plaster.

Retreat you say,
Ok then.

Tnx for the screenshot.

edit:

if you like it, i made it easy for you ->
heres Doto with the loop :

Doto114Beta

MediaFire is a simple to use free service that lets you put all your photos, documents, music, and video in a single place so you can access them anywhere and share them everywhere.

www.mediafire.com

the save game is in the mod's folder. use autoplay (ctrl+shift+z for 2+ turns and youll get the error).

f1rpo · Jun 29, 2024

@keldath: It's this (little) known issue with the K-Mod pathfinder:

Spoiler :

I knew there was some rare issue, but I didn't remember where I left a comment about it and that it had to do with danger avoidance. I'll see if I can at least catch the issue a little earlier, if only through an assertion that makes the modder aware that it's a known issue. Specifically, in your screenshot, if the AI considers entering the Rice(?) tile SE of Nanjing from the SE before investigating the path along the roads near Shanghai, then it will assume that no valid path to the Rice tile exists because the only path it has looked at would let the worker end its turn on the Rice where it can be attacked by the Spearman in Shanghai. I.e. the roaded path that reaches Nanjing in a single turn is then never considered. And it seems that, when deciding whether to move to Nanjing (AI_retreatToCity) in the first place, the path via the roads is considered first. Apparently, the reuse of some pathfinding data in the city selection loop affects the order in which the paths are explored – which shouldn't normally be a problem.

Perhaps this can be fixed properly without a significant performance penalty. I guess I'd just need to move the special case of a unit that needs to avoid danger out of the canStepThrough function into a separate function, maybe "isValidStep2" (because it's really something that the isValidStep function should handle, but that one is supposed to be super fast), to be called separately after canStepThrough has succeeded. And if isValidStep2 returns false, only one edge leading to the dangerous tile gets discarded, not the tile itself. Well, I'd have to run some performance tests afterwards, and I think there'd still be a similar potential issue with the cost function ...

keldath · Jul 2, 2024

hey man,
so, this is a known issue . interesting huh.
for now ill keep my can move check until you get some magic in.

maybe i should do this only for worker type of unit?

also,
yeah would be helpful to know it such a case prior to the loop handler. harder to debug.

f1rpo · Jul 2, 2024

I've pushed my bandaid solution to GitHub: Git commit
It makes the unit skip its turn rather than ignore the danger. It's true that there should be some safe path if a move mission was started, but it's not clear that ignoring all danger will cause the unit to take that path, i.e. there could be equally fast or faster unsafe paths. Staying in place of course also isn't necessarily safe, but still seems like the saner option in general. And I'm only looking for units that can't fight and units that have been using the avoid-danger flag.
I've also written a more proper fix; still need to test that for correctness (fortunately, I still have the original K-Mod pathfinder in the codebase for verification purposes) and performance.

keldath · Jul 3, 2024

Very nice,
I see lots of updates.

So you prefer the worker not to move at all?
That an option so it wont cost tome on the loop .
Hope you get good results on the proper fix.

Gj, ty for all the updates.

keldath · Jul 4, 2024

@f1rpo
well here is a new thing i never saw before.

post merge of all of our commits -> running auto ai, causes a super strange behavior - i dont see the game changes at all, as if its frozen.
but, the game does run, somehow as break points stop the game and also every several minutes the game view updates to a static picture.
its like the game runs in the background and every x time updates the game.

i saw you change something about graphic,

what do you think this is?
related to this commit maybe:
BUG option for lower screen update rate in between turns

f1rpo · Jul 4, 2024

keldath said:
related to this commit maybe:
BUG option for lower screen update rate in between turns

Sounds like it. Is the update rate set to 12 (i.e. 4 updates per second) on the General tab of the BUG menu? That should be the default; maybe somehow doesn't get set properly if some file wasn't merged or perhaps if I didn't commit a file ... Setting a breakpoint in CvGame::update should also clear up whether this is the problem.

keldath · Jul 5, 2024

Ok it was set to 0.
So this is to speed up single players?
Wont work in mp?

Oxidized · Jul 5, 2024

f1rpo said:
There seems to be a bit of history behind that odd treatment of extra collateral damage

Interesting, thanks for the history overview. Seeing how complex the mechanic was and how it was changed towards the end of BTS support makes it seem obvious why something like this was overlooked.

f1rpo said:
Thinking of some mod-mod perhaps wanting to re-enable Barrage for Tanks (which still have a collateral damage limit and target limit set in XML), in a quick test, a Tank with Barrage1 dealt 4 points of damage to a defending Archer, putting it at a displayed strength of 2.9/3

I did some brief tests with enabling Barrage for units that didn't deal collateral damage which would then grant them collateral damage. The UI needs some tweaks for it and probably some AI/game logic somewhere but otherwise it didn't seem that interesting from what I saw.

f1rpo said:
Those PDFs of balance changes? I had wanted to give those another overhaul, but got overwhelmed by the complexity of the whole puzzle - reconciling closeness to BtS with game balance, ease of implementation and some notion of historicity, and, on that last point, kept becoming aware of further details of the original game that are nonsensical, as well as new rationales for justifying some of the nonsense I used to consider unacceptable ...

A few of the proposed mechanics are a bit complex, but there are a few that play fairly well. I think the +yield rate for [foreign/domestic/any] [maritime/any] trade routes that you proposed for Temple of Artemis, Harbor/Cothon, Custom House/Feitoria is the most complex mechanic I've implemented yet. I took the tech costs you suggested which makes the game much slower in terms of tech progression that normal speed feels like epic but it does help spread out technological development with Musketmen having a longer period of viability.

I haven't made any changes to how religions are founded (i.e., requiring a specific religion in a city) since I usually play with the Pick Religions option, but I did find some interesting test matches where Barbarians founded religions by discovering the techs before any other civ. A quick search shows that others have had this happen so I guess it's an "intended" feature although they don't make any attempts to spread the religion even to other Barbarian cities. I haven't narrowed down which tech they're getting to before the others, but it does make for an interesting narrative where civs discover a new land and find an indiginous religion.

f1rpo · Jul 5, 2024

@keldath: Oh, that's my bad then. I've set the default to "0", assuming that this would mean the 0th choice, but it actually means 0 updates. Will change that to "12". Yes, should have no effect in multiplayer. Same as in C2C; I assume that, if they haven't figured out how to do this in multiplayer, it'll be difficult. Probably both players would have to use the same update rate to stay in sync.

You've probably seen this option in other mods under the name "Speed Up AI Turns" or something similar. C2C recently tweaked it so that the screen gets updated every few seconds rather than freeze entirely. I still can't imagine using this in my own games, but it might help someone slog through the late game on a super-Huge map. Although the speedup doesn't seem great; seems that skipped updates increase the time spent per executed update. I might want to use it for smoke tests on AI Auto Play where I mostly look out for failed assertions. Although just minimizing the game may already eliminate the bulk of the time spent on UI updates ...

f1rpo · Jul 5, 2024

Oxidized said:
I did some brief tests with enabling Barrage for units that didn't deal collateral damage which would then grant them collateral damage. The UI needs some tweaks for it and probably some AI/game logic somewhere but otherwise it didn't seem that interesting from what I saw.

I agree that it isn't just a matter of removing the isPromotionValid check to make this work well, so I haven't worried myself about the case of 0 base damage – if someone wants this, they'll have to figure out all the necessary DLL changes. I've gone with a quick and simple addition of base collateral damage and modifiers from Barrage for AI purposes.

Interesting about the trade route yield. I hadn't thought about how to implement that. Maybe adding to the trade profit would be easier than adding a particular yield type at the end. Although "+x Trade Profit" would be confusing to read. I guess, when generating the game text, one could check whether only one yield type has a positive iTradeModifier in Civ4YieldInfos.xml and accordingly replace "Trade Profit" with the respective yield icon.

Tech costs: And that's with the BBAI era-based modifiers at 0 (Civ4EraInfos.xml)? I wanted to get rid of those modifiers and rather balance the pace on a per-tech basis.

Religions: I've just checked in a random savegame in the early Medieval era – the Barbarians were generating 8 research per turn from the commerce of their worked tiles. The bulk of the Barbarian tech progress generally comes from diffusion in CvTeam::doBarbarianResearch, and those research points should never make them the first discoverer of any tech, but their "indigenous" research rate could in theory let them pioneer a religion tech. However, AdvCiv explicitly excludes Barbarians in CvGame::doHolyCity. So, in AdvCiv, as far as I can tell, they can only obtain a Holy City through conquest. (And about the restrictions on founding religions - I don't think I had thought those through.)

Oxidized · Jul 6, 2024

f1rpo said:
Religions: I've just checked in a random savegame in the early Medieval era – the Barbarians were generating 8 research per turn from the commerce of their worked tiles. The bulk of the Barbarian tech progress generally comes from diffusion in CvTeam::doBarbarianResearch, and those research points should never make them the first discoverer of any tech, but their "indigenous" research rate could in theory let them pioneer a religion tech. However, AdvCiv explicitly excludes Barbarians in CvGame::doHolyCity. So, in AdvCiv, as far as I can tell, they can only obtain a Holy City through conquest.

I just checked and in the games where it was happening, CvGame::doHolyCity wasn't the issue, it was CvTeam::setHasTech in the section starting here. A few sections in that function don't check if the player is barb or a minor civ so it might be possible that they end up with other tech-related benefits.

f1rpo · Jul 6, 2024

Oh, I see. I had somehow gotten it into my head that religions are generally founded only once per game turn (by doHolyCity). I must've known better at some point, thanks for setting me straight again. doHolyCity is (mostly?) for handling free tech if I recall correctly now. I guess no one gets to found the religion if the Barbarians are excluded in setHasTech; probably better to leave this as it is.

devolution · Jul 7, 2024

Hi @f1rpo

I'm currently busy merging AdvCiv with WTP and had a look at the following code:

C++:

int CvPlayerAI::AI_getWaterDanger(CvPlot const& kPlot, int iRange,
    int iMaxCount) const // advc.opt
{
    PROFILE_FUNC();
    FAssert(iMaxCount > 0); // advc.opt (use MAX_INT for infinity)
    FAssert(iRange >= 0); // advc: -1 no longer used for default range

    int iCount = 0;
    for (SquareIter it(kPlot, iRange); it.hasNext(); ++it)
    {
        CvPlot const& p = *it;
        if (!p.isWater() || /* advc.opt: */ !p.isUnit() ||
            !p.isAdjacentToArea(p.getArea()))
        {
            continue;
        }
...

Shouldn't the check

C++:

  !p.isAdjacentToArea(p.getArea()) be !p.isAdjacentToArea(kPlot.getArea())

instead ? In the original code p would always be adjacent to its own area I believe.

f1rpo · Jul 7, 2024

That looks like sloppy, erroneous refactoring on my part. In BtS:

Code:

if (pLoopPlot->isWater())
{
	if (pPlot->isAdjacentToArea(pLoopPlot->getArea()))
	{

I probably didn't expect the call argument to be referenced at the start of the second condition and turned that into p. So I assume that it should be !kPlot.isAdjacentToArea(p.getArea()). But, then, should we really count all dangerous units located anywhere in areas that border kPlot? The function had only one call location in BtS (now AI_isAnyWaterDanger in AdvCiv): Check a 4-tile radius around the city for enemy ships. So kPlot was a land plot and, yes, checking all plots in that range that belong to adjoining water areas seems sensible. BBAI has added calls in some CvUnitAI::AI_...seaMove functions, and those pass a water plot to AI_getWaterDanger. Perhaps not what the function was intended for, but, if a plot is adjacent to its own area, as you write, – and never adjacent to a different water area, then it should work out correctly. So just restoring the BtS behavior should be best. And the current error in AdvCiv should only mean that cities can get afraid of enemies in nearby but nonadjacent water areas, e.g. a city at an inland sea that is also near the ocean coast getting spooked by a passing Barbarian ship.

Edit: Well, a 1-plot lake will not be adjacent to its own area. I see that all adjacent plots get checked. So perhaps:

Code:

			(kPlot.isWater() ? !kPlot.sameArea(p) :
			!kPlot.isAdjacentToArea(p.getArea()))

Makes clearer what is intended, performance should be unaffected since I've already added that early unit check.

devolution · Jul 7, 2024

Interesting, in WTP AI_isAnyWaterDanger is only used for water plots. In fact the MOVE_IGNORE_DANGER uses a domain check like this to detemine domain-approporiate danger.

Code:

            if (kGroup.getDomainType() == DOMAIN_SEA)
            {
                if (kPlayer.AI_getWaterDanger(pPlot, -1, true, true, false) > 0)
                    return false;
            }

            if (kGroup.getDomainType() == DOMAIN_LAND)
            {
                if (kPlayer.AI_getPlotDanger(pPlot, 2, false) > 0)
                    return false;
            }

I see that in AdvCiv the generic plot danger function (i.e. AI_getPlotDanger ) is used for both domains, which may be a bit redundant since there is quite a bit of other checks that may not be suited for domain water like the border danger stuff. Ships generally have more movement points than land units, so an efficient and specialized function that uses a greater range than land may be warranted. I suppose that there are some cases for water danger that is not checked at all, one example being the threat of enemy ships passing from one water area to another through an enemy city / fort. A close water analogue of the "ambush check for workers", I think

f1rpo · Jul 7, 2024

OK, I'll try to check if time can be saved; thanks. And I've edited my post before seeing your reply. But just for a nitpick.

f1rpo · Jul 7, 2024

Seems that Tholal was thinking the same thought: Git commit ("Workboats will now check for Water Danger rather than Any Plot Danger")

That's already adopted in AdvCiv. I think it's now mostly the pathfinder that uses plotDanger for sea units. Of course, avoiding danger is not as common for ships in BtS as it is in Colonization. And my inclination has been rather to merge waterDanger into plotDanger because it's a fairly complex problem wrt. accuracy and performance. Looks like my plotDanger function currently mostly has more options (i.e. optional parameters) than waterDanger, including bCheckBorder, which is enabled by default and also used for water units by the pathfinder. Plus an additional check for CvUnit::canMoveOrAttackInto. That function handles several special cases that don't usually apply to water – but they could in principle, and the TerrainImpassables checks are actually fairly relevant for water. Otoh, waterDanger has that same/adjacent-area check – with the canal loophole that you've pointed out but probably still worth having. I have half a mind to put an isWater check in plotDanger, let that enable an area check and have it override bCheckBorder. And then retire the waterDanger functions. But it's too much effort to test for me, at this point, and Col may well have reason to use laxer, faster checks for water units. Edit: plotDanger does have a same-area check already, just not adjacent-area. I guess it really shouldn't replace waterDanger when checking for water units near cities.

keldath · Jul 19, 2024

hey there,
got a strange assert.
after ai defend, group move , after the setxy was fired, a changeMillataryHappines was fires with -1.
but the "oldCity" was already on 0.
i did not change settings of that part

maybe something about the path that moved through a city or something? that asks to remove the value to a city despite the unit wasnt there to raise the value in the first place?
callstack attached.

f1rpo · Jul 19, 2024

A vassal agreement could be responsible. I think this commit (v1.10; from last fall) requires military happiness to be updated/ recalculated when a vassal agreement is signed or canceled. This is probably the cause. Recalculating should be easier to do; I'll try to fix it that way.

Edit: Pushed the bugfix to GitHub (link). But this won't necessarily fix your savegame. (If a vassal agreement is indeed the cause.)

Advanced Civ

LivE LonG AnD PrOsPeR

plastics

LivE LonG AnD PrOsPeR

plastics

LivE LonG AnD PrOsPeR

LivE LonG AnD PrOsPeR

plastics

LivE LonG AnD PrOsPeR

Chieftain

plastics

plastics

Chieftain

plastics

Prince

plastics

Prince

plastics

plastics

LivE LonG AnD PrOsPeR

Attachments

plastics

Similar threads