Current Fix for the Forest Spam Bug

I've been going through the diffs between the unofficial patch source (0.19) and the original 3.17 source to see which changes I wanted to incorporate into my personal game core and I don't understand the fix for the forest spam bug in CvGame::normalizeAddExtras(). (i.e. The game filling your starting location with forests before it tries to place extra resources.)

Based upon my reading of SevenSpirits' details for fixing it, the forest-adding code is supposed to be moved from its original placement before the resource-adding loop to a later position either after the resource-adder or further down after the hill adder. Quoting part of the linked post:

SevenSpirits said:

I think the best fix is to move the forest-adding code to below the resource-placing code.
[...]
To implement this change, you just need to cut and paste a block of code. Here is the block in question:

Spoiler :

Code:

				shuffleArray(aiShuffle, NUM_CITY_PLOTS, getMapRand());

				for (iJ = 0; iJ < NUM_CITY_PLOTS; iJ++)
				{
					if (GET_PLAYER((PlayerTypes)iI).AI_foundValue(pStartingPlot->getX_INLINE(), pStartingPlot->getY_INLINE(), -1, true) >= iTargetValue)
					{
						break;
					}
					
					{
						pLoopPlot = plotCity(pStartingPlot->getX_INLINE(), pStartingPlot->getY_INLINE(), aiShuffle[iJ]);

						if (pLoopPlot != NULL)
						{
							if (pLoopPlot != pStartingPlot)
							{
								if (pLoopPlot->getBonusType() == NO_BONUS)
								{
									if (pLoopPlot->getFeatureType() == NO_FEATURE)
									{
										for (iK = 0; iK < GC.getNumFeatureInfos(); iK++)
										{
											if ((GC.getFeatureInfo((FeatureTypes)iK).getYieldChange(YIELD_FOOD) + GC.getFeatureInfo((FeatureTypes)iK).getYieldChange(YIELD_PRODUCTION)) > 0)
											{
												if (pLoopPlot->canHaveFeature((FeatureTypes)iK))
												{
													pLoopPlot->setFeatureType((FeatureTypes)iK);
													iCount++;
													break;
												}
											}
										}
									}
									
									iFeatureCount += (pLoopPlot->getFeatureType() != NO_FEATURE) ? 1 : 0;
								}
							}
						}
					}
				}

It's near the beginning of CvGame::normalizeAddExtras(). It starts with the first mention in that function of shuffleArray.

There are three loops in the function preceded by shuffleArray. The second one is for resources and the third for hills. Paste the indicated code after either the second or the third such loop.

Note that the code I mentioned is preceded by
int iFeatureCount = 0;
Leave that line where it is and (trust me) it will all work out. There's another reference to iFeatureCount in the function but it is conveniently unreachable (and inconsequential).

Click to expand...

Note the phrases "move the forest-adding code" and "cut and paste." However, in the current unofficial patch source, that section of code seems not to have been moved but instead copied as it exists in both its original position and in one of the recommended spots after the resource function.

Further muddying the waters, Bhruic talked about a different fix in a reply (#1310) to the above quoted post. After reading his comment and looking at that source, that fix did involve essentially copying the forest loop but it also reinstated the randomizer so that the first pass-through didn't cover every tile. Since the second version of the forest loop matches Bhruic's second loop it seems like this fix was intended. However, in the sources for 0.19, the randomizer clause is still commented out meaning this is different from the Bhruic fix too.

So which of the following is right?

1. This patch implemented Bhruic's fix. If so, shouldn't the randomizer clause for the first pass be reinstated?
2. This patch implemented SevenSpirits' suggestion. If so, shouldn't the first forest-adding loop be removed?
3. This patch implemented its own fix different from the previous two. If so, could somebody explain it to me?
4. None of the above. In that case, I'm hopelessly confused...

You aren't the only one.

I am confused. Solver's 0.19 matches Better AI 0.35. Are they both wrong?

Denniz said:

I am confused. Solver's 0.19 matches Better AI 0.35. Are they both wrong?

Click to expand...

I believe that part of Better AI 0.35 was simply copied from Solver 0.19 since I don't see it specifically mentioned in the posted Changelog; I don't follow that project real closely, however and haven't looked at the source.

As for the main question of whether it is wrong, the purpose of this topic is to get that question answered. It looks to me like the unofficial patch doesn't actually do anything to fix the problem since the pre-resource forest loop is in-place and unmodified. Thus the BFC can still get completely filled with forests before the resource-adding loop executes and the newly-added second forest loop shouldn't have any effect. SevenSpirits' suggestion was to move the forest loop and Bhruic's fix involved uncommenting the randomizer so only part of the BFC would be forested the first time through; both methods leave at least some of the land tiles free for resource-adding. I don't have nearly the SDK-modding experience Solver has, though, so it's possible that a different fix was implemented that I'm just not seeing. Thus, the questions posed at the start of this topic.

Interesting ... I did just copy over Solver's code for Better BTS AI, so unfortunately I can't lend any insight in to what was or was not intended initially. Thanks for bringing this up, I haven't really looked into how these fixes were done before. Just based on the short descriptions you included certainly something needs to change, I like the sound of Bhuric's solution of turning on the randomizer as a middle ground between the current state and just moving the foresting code.

I was wondering why the over forested starts were still happening ...

On a loosely related note, my guess is Solver is a bit overloaded right now with RL and perhaps crunch beta testing for Colonization ... perhaps someone could step up and create an unofficial unofficial patch. I, of course, would advocate people use Better BTS AI but understand that somewhat experimental changes may not be for everyone.

Posting so I can find this thread again later.

Is there a mod out there that fixes this issue yet?

There's absolutely no difference between the official 3.19 patch and the latest unofficial patch when it comes to this part of the code.

Is there a problem?

The forest placing code really is there twice, but the first time there's this line
if (getSorenRandNum((iCount + 2), "Setting Feature Type") <= 1)
which should prevent that 100% forest spam.