V36

[strategyonly]

Without those movement changes the same turn took only a few minutes. I have to see if i can find that save and upload it here.

Definitely, thx

 

[Thunderbrd]

In the right conditions those unit movements changes cause huge slowdowns that can't happen. It might not be visible in your game using the internal profiler and that is exactly what i was saying a few months back. It only appears as noise in the profiler logs until you have the right conditions and then you are able to see it really slowing down the turn time. The main problem is that the code you have added causes too much recursive looping through units, the same thing happened in the healer build evaluation.

You end up with far too many calls to CvUnit* getUnit(IDInfo unit). That leads too alot of spin time because of the CriticalSections needed for the multithreading. I'am not sure but i think the internal profiler doesn't show the spin time so you can only see it in the turn times.

I had a save somewhere that had a bit over 50 minutes turn time and after my changes it was down to ~20 minutes. Without those movement changes the same turn took only a few minutes. I have to see if i can find that save and upload it here.

I can believe that. Unfortunately I'm not sure if there's a better way to accomplish the same effect. Again, perhaps checkPlot might be able to take on a 3rd function run and that might be a lot less time-costly. I'll have to get that save and play around with that as an alternative.

The loop you're talking about is in bool CvUnit::canCoexistWithEnemyUnit(TeamTypes eTeam, const CvPlot* pPlot) const
right here:

	if (pPlot != NULL)
	{
		CLLNode<IDInfo>* pUnitNode = pPlot->headUnitNode();
		bool bFound = false;
		while(pUnitNode != NULL)
		{
			CvUnit* pUnit = ::getUnit(pUnitNode->m_data);
			pUnitNode = pPlot->nextUnitNode(pUnitNode);

			if (pUnit && pUnit != this && GET_PLAYER(pUnit->getOwnerINLINE()).getTeam() == eTeam)
			{
				if (isEnemy(eTeam) || isAlwaysHostile(pPlot) || pUnit->isAlwaysHostile(pPlot))
				{
					bFound = true;
					if (!canUnitCoexistWithEnemyUnit(pUnit,pPlot))
					{
						return false;
						break;
					}
				}
			}
		}

		if (bFound)
		{
			return true;
		}
	}

Which causes the potential opponent to be evaluated for certain factors that may allow the unit to co-exist with it in bool CvUnit::canUnitCoexistWithEnemyUnit(const CvUnit* pUnit, const CvPlot* pPlot) const.

This probably does create a lot of cycling through units and calling that function (which is fairly simple but contains all the main rule changes I'm hoping to somehow preserve.)

The alternative I'm suggesting would be to use an alternative PUF mechanism BUT I don't think that would fully work either because it would existing PUFs like PUF_isEnemy would be thrown off by this change in assumption. It was VERY convenient to place the additional rules here because just about everything eventually refers to it.

Don't suppose you have any suggestions as to how it could be rewritten to be more optimized without having to miss out on the ability to give a unit a condition that could mean that it's not 'attackable' just because its visible and its an enemy?

EDIT:
OMG! I just started really looking into how often canCoexistWithEnemyUnit is called and WOW! It certainly shows why there would be an issue here. It's yet another oversimplification but in this case its now causing HUGE problems. It's called as a way units can be disconsidered from a host of checks I don't believe it should be the reason to disconsider those units for. For example, a unit that can be co-existed with (in general) is considered to not be a unit that counts as one of another team in a plot count. My intention here is dramatically flawed by this assumption in many places in the code and is called for far more often than I would've suspected it to be.

I'm not sure but this could maybe be fixed by a bit more VERY thorough review of when, where, and WHY canCoexistWithEnemyUnit is called and perhaps restructure it so that ONLY those calls I really WANT calling for the movement rule changes will. That's going to take some serious time to work out I think.

 

[Reisk@]

Sometime later it might also be smart to analyze that how, why and when this happened.

Just so that it doesn't happen again with everyone seemingly oblivious to it.

 

[JosEPh_II]

How's it going T-brd? Is alberts2 giving you a hand on this?

And how long does it look like v36 will be delayed?

JosEPh

 

[Thunderbrd]

I've not had much time outside of the weekends to look into this. Laborday weekend I got out of the house and explored Bryce National Park in Utah which was really cool. Last weekend I worked on it and it was not only slow progress but it was rife with changes making things actually slower. Some of the discussion since then has led me to look into some things that may be able to be improved on without slowing things further and possibly without losing the mechanisms I wanted to work with in the first place. However, THAT is going to be a serious effort to sort out properly.

I just lost my job (again - gotta love these damned call centers where no position is safe and hiring and firing is done on the slightest whim) so I MAY have additional time to work on this now - probably even more when I do find another position - this last one was taking up 12 hrs a day. However, next week and the next week after that we have family visiting so RL will definitely get in the way of further work on a solution during that time. We'll see how much I can get done between now and then.

EDIT: VERY nice to have ya back Joe!

@Alberts2: If you can get me that save you were using I can test it against a very simple change I believe will make a very big impact on turn times provided there are large stacks in play. I believe there were many thousands of checks that cycled the canUnitCoexistWithEnemyUnit function that could be very simply averted. It won't eliminate all of them but most of the ones that should be taking up a ton of unnecessary processing time. Again, a very simplistic fix but it may well be what was needed.

EDIT: NVM the simple solution doesn't quite work as intended. Will take more evaluating.

 

[JosEPh_II]

<snip>

I just lost my job (again - gotta love these damned call centers where no position is safe and hiring and firing is done on the slightest whim) so I MAY have additional time to work on this now - probably even more when I do find another position - this last one was taking up 12 hrs a day. However, next week and the next week after that we have family visiting so RL will definitely get in the way of further work on a solution during that time. We'll see how much I can get done between now and then.

Dang T-brd that's rough! Hope job opportunities are better in Vegas than in western IL.

EDIT: VERY nice to have ya back Joe!

@Alberts2: If you can get me that save you were using I can test it against a very simple change I believe will make a very big impact on turn times provided there are large stacks in play. I believe there were many thousands of checks that cycled the canUnitCoexistWithEnemyUnit function that could be very simply averted. It won't eliminate all of them but most of the ones that should be taking up a ton of unnecessary processing time. Again, a very simplistic fix but it may well be what was needed.

EDIT: NVM the simple solution doesn't quite work as intended. Will take more evaluating.

If you think it would help I can post my current turn from my Crime test game that is in the ren era.

JosEPh

 

[Thunderbrd]

I don't think another Ren era save would show the issue based on the one I see here.

 

[Thunderbrd]

I'm about to commit a dll that passes all MY tests. It should be much faster AND preserve the movement rules and ease of further manipulating them as intended. It loops through units FAR less than the previous method.

However, I'm not sure if there's an enhanced capacity for MAFs in this (or maybe even less). I'm also unsure if there are going to be some unanticipated bugs and odd behaviors in unit interactions or AI choices that this could bring on. BUT it MIGHT even solve some of the previous 'odd behaviors and AI choices' we've been seeing such as the animals never attacking 0 str units. Might.

What this will need is extensive playtesting and stress testing. Again, it passes all of my initial tests but the amount of situations that could be impacted is mind boggling. It passes my own profiling speed tests but again, I don't have a save that would show the difference quite as tremendously as some very latestage games may.

Anyhow, I URGE Alberts2 to take a look at the changes as they are logged on the SVN commit. All I ask is... does the newer method introduce any problems you can think of that I might not have considered?

So that's it... I've done what I believe I can for this issue but there may be small edits remaining.

Note: I also repaired an issue with Build Ups and the fix may have introduced a very slight slowdown at times. So this must be considered if you're looking at the profile reports. If a group were to awaken it was supposed to wipe out all buildup promos on that group, not just the head unit, so I had to introduce a quick group loop to go through and erase all buildups in an awakening group. That's all. I don't THINK this awakening action gets called TOO obsessively by the AI... hope not.



So... short story is... test test test test test! I'll be watching the bug reports carefully.

 

[Dancing Hoskuld]

I probably should point out that I never experienced any slow down like the others have been mentioning.

 

[Thunderbrd]

Only VERY advanced games would. But testing is still required here for making sure nothing very weird is taking place in unit interactions.

 

[Dancing Hoskuld]

I am testing it.

The bug SO reported and I fixed was probably not a "real" v36 bug but to do with the mod he had in. We now appear to have duplicate quarry like improvements for the period between mining and masonry. This means I need to remove one of them. I think mine does what was intended better than the old so I will look into removing the old. Or changing the old and removing the new. The only problem is the name I called it "Early Quarry" while the old is still a "Stone Tool Workshop" neither seems quite right to me.

edit The AI is making good use of the fact that workers and other zero strength wont be attacked.

 

[strategyonly]

edit The AI is making good use of the fact that workers and other zero strength wont be attacked.

Yes they are, and i like it, lol

 

[Toffer90]

edit The AI is making good use of the fact that workers and other zero strength wont be attacked.

And once the player wage war against an AI the workers are easy pickings as they do not notice any threats. Easy to capture barbarian gatherers almost make the training of gatherers obsolete in the prehistoric.

I have also experienced barbs randomly walking onto workers/captives destroying them in the process, this is not even reported to the player in any way; it used to be a battle scene where the worker do an animation where they give up and a red text message in the log, now they just disappears. This usually happens in narrow passages where there is one road and the barb is traveling it in the opposite direction than the worker; they meet and before you know it the worker ceased to exist.

 

[Thunderbrd]

Strange about those workers. I'll have to run some tests to see if I can figure out what's happening.

EDIT: I think it has to do with an AI adjustment to attack AIs or something.

There's a bool CvUnitAI::AI_poach() function that is designed to help the AI units determine whether they want to attack such unable to defend units but it's not in use anywhere. I wonder if and when and where it was.

There COULD be something odd going on with the way the AI was once taught to see there as being a potential unit to attack in that space and now they don't with the new movement rule changes eliminating units that can't defend from that count. I tried to find where that would be... I think the old way simply looked to see if there was a visible unit on the plot first so I may simply need to switch it over to visible ATTACKABLE unit (not to eliminate units that can't defend from that list.) So it's probably the movement rule changes in general. But it will still take some sorting out.

EDIT:
Yeah, that had something to do with it. That and that there's very few places in the Unit AI codes where the AI will consider such an attack. However, where it generally is I've corrected and will commit to the SVN tomorrow.

 

[Toffer90]

Yeah, that had something to do with it. That and that there's very few places in the Unit AI codes where the AI will consider such an attack. However, where it generally is I've corrected and will commit to the SVN tomorrow.

 

[Redwallzyl]

any idea when 36 will be released? I'm thinking i might get back into the map modding when it comes out.

 

[Dancing Hoskuld]

any idea when 36 will be released? I'm thinking i might get back into the map modding when it comes out.

No idea when. There is at least one serious issue, workers not being attacked, still left to resolve.

I will be introducing two new terrain types after it does Lake and Lake Shore for fresh water lakes. They are equivalent to Ocean and Coast in nature. I could add them into v36 and not implement them ie not have them appear in game until after v36 for the map makers.

 

[Thunderbrd]

Workers should be attacked now. Generally. The motive should exist as normal at this point.

I suggest leaving the release for a few weeks just to test the SVN for a bit to make sure we don't have any other truly serious issues to resolve. However, there are always going to be some issues lurking around. Such as in the latest bug thread post. Not all of these should be an imperative to resolve before release because it could have us unendingly frozen if we seek to solve them all. The main thing I'm looking for here is unit interaction oddities. But so far it seems good. And I'd love it if Alberts2 would run the profile test on the save he has that showed a huge slowdown on the old movement rule methods compared to the ones in place now.

I'll be mostly out of commission here between tomorrow and Friday of the next week as far as modding goes. Family visiting. So if nothing major comes up between now and then I'd say we should go ahead and release so we can start getting to those things we know we'd like to make forward progress on.

 

[Noriad2]

No idea when. There is at least one serious issue, workers not being attacked, still left to resolve.

What about the bug where conquered cities have double production of everything (money hammers, tech, xp, resources etc) ?
Fine-tuning civics, tech costs, unit production costs etc. is pointless if this major bug isn't addressed.

 

[Harrier]

I will be introducing two new terrain types after it does Lake and Lake Shore for fresh water lakes. They are equivalent to Ocean and Coast in nature. I could add them into v36 and not implement them ie not have them appear in game until after v36 for the map makers.

Yes. I would like you to do that, so I could start working on updates to my maps also.

You would need to let us know what the new types are.

i.e.

TerrainType=TERRAIN_LAKE
TerrainType=TERRAIN_SHORE