Cities Defenses wich damage adjacent units are doing way too much damage

[Thunderbrd]

So I'm taking a look at the code on this. A few bullet points of observations:

* I'm wrong about how it processes the damage. When buildings are added to the city that have this adjacent damage potential, they total up on an Adjacent Damage Percent variable and every round the city 'attacks' all adjacent enemy units. There's a one in four chance of striking. The damage dealt is relative to the CURRENT HP (edited) of the unit. If 50% total adjacent damage, and the unit has 60 HP left, it will deal 30 to that unit, leaving the unit with 30 remaining. This should mean that it's very uncommon for units to be completely killed outright from this effect at all. Maybe something else entirely is afoot.

* This means that when buildings process in and out, they should be removing any amounts from obsoleting or replaced buildings. I will check on this variable to ensure that but usually it's the same code line that adds the value to the city as the code line that removes it, with the difference in the cycle being whether the building is being processed in or out. It's possible it COULD not be processed out when obsoleting and that would be an explanation of the issue as some tags do this intentionally and if the processing statement on this variable is nested where those tags are being processed, that would explain the issue. I'll know here very shortly... no that shouldn't be an issue unless this is a much bigger bug that's applying to a LOT of things it shouldn't be applying to, like XP for units and all sorts of things.

* That pretty much leaves one suspicion remaining and that would be that in this era we start getting total values in cities capable of going over 100% and that particular city is doing exactly that. If this is incorrect, which I'll see soon, the digging into the code on this save is going to get a LOT more complicated!

* I have actually modded the code - first time in a while here! I added a total Adjacent Damage Percent line to the defense help hover tooltip (hover over the rook symbol in the upper right in the city to see this.) With a nanospy in a few japanese cities, I was able to see that the totals are ranging up to 80% but I'm not seeing it going above that.

Now... that's a lot, yes. But it doesn't explain how units are vanishing at all. I even stepped through the code and watched the damage being dealt and it seemed to be assigning it properly.

void CvCity::doAttack()
{
    PROFILE_FUNC();

    if (getAdjacentDamagePercent() > 0)
    {
        if (GET_TEAM(getTeam()).getAtWarCount(false) > 0)
        {
            bool abInformPlayer[MAX_PLAYERS];
            for (int iI = 0; iI < MAX_PLAYERS; iI++)
            {
                abInformPlayer[iI] = false;
            }
            for (int iI = 0; iI < NUM_DIRECTION_TYPES; ++iI)
            {
                CvPlot* pAdjacentPlot = plotDirection(getX(), getY(), ((DirectionTypes)iI));
                if (pAdjacentPlot != NULL)
                {
                    for (CvPlot::unit_iterator unitItr = pAdjacentPlot->beginUnits(); unitItr != pAdjacentPlot->endUnits(); ++unitItr)
                    {
                        CvUnit* pLoopUnit = *unitItr;

                        if (pLoopUnit->getTeam() != getTeam())
                        {
                            if (GET_TEAM(getTeam()).isAtWar(pLoopUnit->getTeam()))
                            {
                                //    Koshling - changed city defenses to have a 1-in-4 chance of damaging each unit each turn
                                if (pLoopUnit->baseCombatStr() && GC.getGame().getSorenRandNum(4, "City adjacent damage") == 0)
                                {
                                    int iDamage = pLoopUnit->currHitPoints();
                                    iDamage *= getAdjacentDamagePercent();
                                    iDamage /= 100;

                                    pLoopUnit->changeDamage(iDamage, getOwner());
                                    if (!abInformPlayer[pLoopUnit->getOwner()])
                                    {
                                        MEMORY_TRACK_EXEMPT();

                                        abInformPlayer[pLoopUnit->getOwner()] = true;
                                        CvWString szBuffer = gDLL->getText("TXT_KEY_MISC_UNITS_DAMAGED", getNameKey());
                                        AddDLLMessage(pLoopUnit->getOwner(), false, GC.getEVENT_MESSAGE_TIME(), szBuffer, NULL, MESSAGE_TYPE_INFO, pLoopUnit->getButton(), CvColorInfo::red(), pLoopUnit->getX(), pLoopUnit->getY(), true, true);
                                    }
                                }
                            }
                        }
                    }
                }
            }
        }
    }
}

Can the programmers among us see anything incorrect here? Extra looping through units or?

The Japanese city your save starts off next to has an 80% total which is pretty intense. It was actually far worse than Fukushima which was nowhere near as high.

I'm thinking something else is causing the deaths here... by chance do you have the opportunity fire bug option on?



As a side note: I do think it's a bit too high. I scrolled through to try to total up sources for a while and noticed that the Fire Abatis is even still in place. Hmm... Should that perhaps be obsolete by then or no?

We might want to consider these buildings and a deeper analysis of the potential amounts they can build up to and reign them in a little. When it DOES get over 100% it's going to be problematic and this 80% was possible a bit too early for that much I think.

 

[sunrisereader]

How is rounding done for low health units like healers could the 80% be rounded up to the full health a healer/mechanic has?

 

[Thunderbrd]

How is rounding done for low health units like healers could the 80% be rounded up to the full health a healer/mechanic has?

All units have a base of 100HP without Size Matters being on. Rounding will only round down to the nearest integer. Thus only if the damage is over 99 but not as far as 100 would it round out to 100. On SM, Healers may have 10 or even less HP potentially, but it would still be unusual to kill - possibly impossible because even if the unit had 1 HP, it's likely to round down the amount of damage it will do to 0.

IIRC, Opportunity Bombard CAN kill and I'm thinking it may explain what is being experienced here.

 

[Joij21]

Perhaps he is misattributing the lost units to city damage when in fact they were killed off by counterattacking units? I've noticed ever since the speed/performance updates that it no longer shows the animations of counterattacking forces between turns. They only indication of being attacked would be the teeny tiny news text that appears at the top of the screen, quite easy to miss. Also if his camera was already hovering over the city when he pressed the turn button, then he would never see the screen pan over to the location as to indicate being attacked.

 

[Thunderbrd]

I've noticed ever since the speed/performance updates that it no longer shows the animations of counterattacking forces between turns

That's a game option setting but I haven't tried running it on the settings where it should show enemy moves recently so wouldn't know if that's been officially killed off or not.

They only indication of being attacked would be the teeny tiny news text that appears at the top of the screen, quite easy to miss.

Wondered that too but his units should be too strong for the enemy to hardly even hurt really, unless something is very very wrong with the best defender mechanism, which it may be. I know it's been put to some suspicion recently.

 

[Joij21]

That's a game option setting but I haven't tried running it on the settings where it should show enemy moves recently so wouldn't know if that's been officially killed off or not.

Wondered that too but his units should be too strong for the enemy to hardly even hurt really, unless something is very very wrong with the best defender mechanism, which it may be. I know it's been put to some suspicion recently.

Ok I just tried out the second save arthus posted and something definitely smells of funk here.

Before

Here you can see multiple units in his stack with full health.


After a few turns.

Also each turn the units have been getting one shot each turn since those remaining still have full health.

I later checked with the nano spy and they have a cannon turret and bombard turret, I couldn't find anything else in the long list of buildings. Maybe those two buildings are particularly bugged?

 

[Thunderbrd]

Well that confirms something odd is afoot. I don't see how those individual buildings could be uniquely bugged in the way the code is setup but I will investigate the game on the second save later.

I'd noticed that it looked like none of the units there after spending a round next to the city had any varied amounts of damage and that's very strange as well and you've confirmed that observation. The code doesn't suggest that would happen.

Again though, is the Bug option Opportunity Bombard on?

 

[Joij21]

No, otherwise I would be receiving messages about being bombarded. The only message I'm receiving is that units are damaged due to defense buildings.

 

[Thunderbrd]

No, otherwise I would be receiving messages about being bombarded. The only message I'm receiving is that units are damaged due to defense buildings.

Hm... that city has nowhere near enough damage output to kill units outright (UNLESS it was able to build another def building by the time the units arrived - that might explain it). The plot has definitely thickened here. I'll take a look at that save later.

 

[Thunderbrd]

So this is interesting. My process of elimination and logic has found the cause here. More questions now to answer but this is certainly the first:

This is Fukushima during the 2nd save. They have built something that put their Adjacent Damage total up over 110%. Ouch.

So... the next thing is to catalog all the sources.

The next thing is to figure out if replacements or obsoletions are failing.

And the next thing would be to spreadsheet out all buildings that add this Adjacent Damage and figure out how to make it grow in a more sane manner over time. It should never quite reach 100%

Now, there's a few possible CODE solutions:
1) We could apply a law of diminishing returns to this value. Aka, 1 pt of iAdjacentDamagePercent from a building = 1% total damage until you reach 50% total in the city, then each further point = .5 more until 75% total after which then each point = .25 more and so on so that 100% can never be obtained. This would be a good way to address things if we really cannot see how to design these XML values in a sane way throughout the entire tech tree. It's also a little misleading to the player once it gets past that initial 50% - if they do an audit in the upper range numbers we'll get players asking why it didn't add as much from a new building as it suggested it should.

2) Give an absolute limitation in the globals. This is what we do with bombard defense (which I think we should eventually change to a diminishing return instead) because past a point, it can make cities completely immune to bombardment. We cap it in the globals but the problem here is that this means the progression on these values can only go so far while continuing to represent a degree of improvement. After a point, the added #s are just making themselves look impressive but aren't actually improving things at all.

3) Make it a total random between 0% and the max total when it DOES hit a unit. This could be a much more interesting way to approach this and would work fine with values in excess of 100% because exceeding values would just be more possible random value results that would be >100%. This could still be much more lethal than we'd want it to be of course.

4) Allow units an ability to resist this damage somehow. Armor seems to be a good way to add that to units but that's part of another system with its own intentions and it would be very easy for that system to keep the needs of this approach out of balance or vice versa. I'm not against applying armor to resist this effect eventually but Armor is also not yet fully planned out in full AND is primarily intended as part of the Equipment option in application - and as an option which may or may not be 'on', probably isn't the best way to address this problem.

5) Deepen the system ( a lot ) in larger ways.

1, 2 and 3 have the most immediate fix potential and I'm REALLY leaning towards implementing #1 if nobody is willing to remap these building XML values and replan them into something more sane. As y'all know, I'm in the middle of remapping unit base stat stuff and this seems like it needs a more immediate resolution than to wait for that to be finished before addressing this.


I guess we might need to run a poll on this - in the meantime I'd like to hear opinions from y'all. No promises I'll agree or take time for longer projects now but at least discussing the best immediate and long term plans is warranted.

I'll take a moment here to figure out what buildings are present that are tallying into this total...

EDIT:
Ok, so I've confirmed that only the Bombard Tower and Cannon Turret are listed here, as stated, 30 and 25%. So THAT means our entire game is a complete trainwreck where obsoleted/replaced buildings aren't being removed from the cities right now.

I'm not sure I have enough skill to fix this.


I'm currently running a recalc on this save to see if the adjacent damage total corrects itself.

EDITEDIT: Nope. Crap... it's somehow even worse then. Now I'm going to need to run the recalc while tracking that particular city ID. How to find the city ID again...


EDITEDITEDIT:

    if (bObsolete || m_recalcBuilding != MAX_INT || !GET_TEAM(getTeam()).isObsoleteBuilding(eBuilding) && (bReplacingNow || m_paiBuildingReplaced == NULL || m_paiBuildingReplaced[eBuilding] == 0))
    {

This code is nesting the processing call for this tag, among so many, if not ALL other building tags.

I suspect that the notes above here point towards the problem:
// We don't need to process the building effects in or out if it is
// * obsolete (unless GOING obsolete explicitly now)
// * has already been replaced, except in the middle of a modifier recalc where we might be processing it out
// due to that replacement being re-detected

What really has to be happening here is that replaced buildings must be continuing to add their values to the city, both during a recalc process and when replaced not being processed out.

This is all within this function:
void CvCity:rocessBuilding(BuildingTypes eBuilding, int iChange, bool bObsolete, bool bReplacingNow, bool bReligiouslyDisabling)



OK, wait, I think I found the problem - it looks like recalculations aren't taking replacements into account at all and all replaced buildings are staying calculated in after a recalc. *shudder* Testing a change in the recalc process that would add that step back in - how it was removed I have no idea but there's a lot of changes here in the way some things are handled since the last time I really evaluated building in/out processing.

Will come back to let y'all know if this is a success.

Theories:
bReplacingNow may not be firing correctly.

The || and && statements in this code is not nested very explicitly so I'm not sure if
&& (bReplacingNow || m_paiBuildingReplaced == NULL || m_paiBuildingReplaced[eBuilding] == 0)
shouldn't maybe be
|| (bReplacingNow || m_paiBuildingReplaced == NULL || m_paiBuildingReplaced[eBuilding] == 0)

I'm just thinking aloud here...

So an IF statement runs left to right and if it finds true then it takes that true result immediately.

Therefore this reads:

if (bObsolete ||

= If the building is bObsolete, which I assume means the building is in the process of obsoleting... hoping this doesn't mean if the building actually IS obsolete then make sure to run the processing statements that follow... if that's the case then we are processing in ALL obsolete buildings as a first and foremost priority which would really be so wrong its crazy but would certainly answer to this. --- ok yes it just means that this process call was made to update a building as a result of it being obsoleted. So this could be right.

 

[arthas125]

So this is interesting. My process of elimination and logic has found the cause here. More questions now to answer but this is certainly the first:

View attachment 560681
This is Fukushima during the 2nd save. They have built something that put their Adjacent Damage total up over 110%. Ouch.

i didn't even know that part of the UI existed otherwise i would have brought it up earlier to show it

4) Allow units an ability to resist this damage somehow. Armor seems to be a good way to add that to units but that's part of another system with its own intentions and it would be very easy for that system to keep the needs of this approach out of balance or vice versa. I'm not against applying armor to resist this effect eventually but Armor is also not yet fully planned out in full AND is primarily intended as part of the Equipment option in application - and as an option which may or may not be 'on', probably isn't the best way to address this problem.

is it too hard to make stronger units simply take less damage from these defenses? it would also make low era buildings less effective against units of higher strenght wich come later, so the enemy would be forced to upgrade said buildings in order to defend properly, unlike in my save in wich medieval/classical era defensive buildings are fending off reissanance/industrial era units wich is ridiculous, this would also make high promoted units more effective against said damage, since the only promotion i've seen that helps against this is the self repair/heal promo line and that's only because it increases the max HP of the units, i always play without any combat mods (so its a big difference, so currently a lvl 30 unit without those promos dies just as fast as a lvl 0 unit against this sort of damage, maybe adding a resistance to this kind of damage via promotions, if its possible, would work nicely aswell

 

[Thunderbrd]

i didn't even know that part of the UI existed otherwise i would have brought it up earlier to show it

As I stated earlier, I added this line to the defense help hover I'm showing - it wasn't there originally (though you SHOULD get familiar with the defense help hover if you aren't yet!)

is it too hard to make stronger units simply take less damage from these defenses? it would also make low era buildings less effective against units of higher strenght wich come later, so the enemy would be forced to upgrade said buildings in order to defend properly, unlike in my save in wich medieval/classical era defensive buildings are fending off reissanance/industrial era units wich is ridiculous, this would also make high promoted units more effective against said damage, since the only promotion i've seen that helps against this is the self repair/heal promo line and that's only because it increases the max HP of the units, i always play without any combat mods (so its a big difference, so currently a lvl 30 unit without those promos dies just as fast as a lvl 0 unit against this sort of damage, maybe adding a resistance to this kind of damage via promotions, if its possible, would work nicely aswell

Yeah pretty much. I mean its possible but would take some creative math. I definitely want the end combat mod stuff to include some resistance to this effect. Maybe I'll end up applying resistance to collateral damage to it, which would make a lot of pending effects more meaningful, particularly new defensive buildups that are planned starting from trench warfare stuff forward.

 

[tantoniou]

So THAT means our entire game is a complete trainwreck where obsoleted/replaced buildings aren't being removed from the cities right now.

Actually I noticed something similar with obsolete buildings and the flammable property. After taking cities from neighbors in various games, and no matter how many flammable buildings I would demolish in them, these cities would have that property significantly higher than my 'native' ones.

Another thing was that you can't demolish buildings that become obsolete like barbed wire, but its graphic stays on (the square-romb shaped barbed wire around the city), and I don't know about its effect in the city. I could only remove it through the worldbuilder.

(Wouldn't it be better if Toffer could apply his awesomely functional building UI to the demolish screen so we can filter the existing buildings much much easier and faster? )

 

[Thunderbrd]

Actually I noticed something similar with obsolete buildings and the flammable property. After taking cities from neighbors in various games, and no matter how many flammable buildings I would demolish in them, these cities would have that property significantly higher than my 'native' ones.

Another thing was that you can't demolish buildings that become obsolete like barbed wire, but its graphic stays on (the square-romb shaped barbed wire around the city), and I don't know about its effect in the city. I could only remove it through the worldbuilder.

(Wouldn't it be better if Toffer could apply his awesomely functional building UI to the demolish screen so we can filter the existing buildings much much easier and faster? )

I was hoping we were only dealing with replaced buildings but if we're ALSO talking about obsolete ones... ueugh.

 

[raxo2222]

I was hoping we were only dealing with replaced buildings but if we're ALSO talking about obsolete ones... ueugh.

I bet something was messed up with building replacement/obsoletion when building classes were removed.

 

[Thunderbrd]

I bet something was messed up with building replacement/obsoletion when building classes were removed.

I don't think that was it... it looks more like when some reprogramming and refactoring was done regarding replacements/obsoletions but I'm not sure to be honest. I'm not too worried about how or who is responsible. Just need to figure out why it's wrong and how to fix it. ugh...

 

[JosEPh_II]

I don't think that was it... it looks more like when some reprogramming and refactoring was done regarding replacements/obsoletions but I'm not sure to be honest. I'm not too worried about how or who is responsible. Just need to figure out why it's wrong and how to fix it. ugh...

Good Hunting!

 

[Toffer90]

I don't think that was it... it looks more like when some reprogramming and refactoring was done regarding replacements/obsoletions but I'm not sure to be honest. I'm not too worried about how or who is responsible. Just need to figure out why it's wrong and how to fix it. ugh...

I'll prioritize helping you in this endeavor.

 

[Thunderbrd]

I'll prioritize helping you in this endeavor.

Thank you... this one is proving to be a tough nut to crack. I tried adding a function intended to check replacements to the recalc and that failed. (calculateBuildingReplacements() Admittedly, it was a hail mary shot at a fix last night before bed.

Looking at the bit that puts building values back in during a recalc, it doesn't look to ever care about replacements and in the processing, replacements seem to only be handled properly IF it is specifically saying something about them in the function call to process the building... but I might be misunderstanding some things still.

This is all that happens to process buildings back in after a recalc:

    for (m_recalcBuilding = 0; m_recalcBuilding < GC.getNumBuildingInfos(); m_recalcBuilding++)
    {
        if (getNumRealBuilding((BuildingTypes)m_recalcBuilding) > 0)
        {
            // Process back the buildings we physically have. This will generate free buildings as it goes.
            // Tech reprocessing will be called later which will re-obsolete those that need it.
            processBuilding((BuildingTypes)m_recalcBuilding, 1);
        }
    }
    //    After processing all buildings set the indicator that reprocessing is not in progress any more
    m_recalcBuilding = MAX_INT;

The processBuilding call is ONLY ever considering the add or subtract building stats boolean in the parameters and isn't even looking at whether the building is obsolete or replaced so I thought MAYBE this was the problem... it seems deeper than that.

 

[Thunderbrd]

I THINK I figured out what I'm gonna have to do with this soon as I have a little time to do it. Had some thoughts in stillness on the issue and think I mighta sorted out how to correct things. Wish me luck.