Pollution bug <- NAILED

darkpanda

Dark Prince
Joined
Oct 28, 2007
Messages
823
Although I never put it at the center of my focus, stories about the "pollution bug" have been kept roaming at the back of my mind ever since I became active on those forums, and read people mentioning it here and there.

Being more or less done with the City Process routine (I'm not done yet, in fact, but a bi tired ;)), I thought it would be more fun to investigate this so-called pollution bug one way or another.

So, first thing, I looked up the forums for related threads, and here is what I found:

Some of the above links contain detailed game logic, extracted from "Rome on 640K a Day" regarding pollution, namely how known Techs influence pollution, how a city's power source influence chimneys, how population influence pollution, as well as shields production (or did it?).

Anyhow, I had already identified a very small routine in CIV.EXE, that I bluntly called "AddPollution(x, y)". It seems this routine is used anytime pollution should be added to the map (nuke, city process, ...).

Back-tracking the call from the City Process routine, I could finally dig out what I believe to be the cause of this famous "bug".

Here the raw piece of assembly, for the die-hards out there:
Spoiler :
Code:
		seg007:633F
		seg007:633F     seg007_633F:                            ; CODE XREF: cityProcess_cityID_mode_?1_?2_?3+62F7j
		seg007:633F 10C                 mov     ax, cityShieldsProd_dseg_705C
		seg007:6342 10C                 cwd                     ; AX -> DX:AX (with sign)
		seg007:6343 10C                 mov     cx, dseg_6C18_CityPowerType
		seg007:6347 10C                 idiv    cx              ; Signed Divide
		seg007:6349 10C                 sub     ax, 14h         ; Integer Subtraction
		seg007:634C 10C                 mov     [bp+var_pollutionProd], ax
		seg007:6350 10C                 mov     ax, 1Ch
		seg007:6353 10C                 imul    [bp+arg_cityID] ; Signed Multiply
		seg007:6356 10C                 mov     bx, ax
		seg007:6358 10C                 mov     al, CityData.ActualSize[bx]
		seg007:635C 10C                 cbw                     ; AL -> AX (with sign)
		seg007:635D 10C                 imul    pollutionFactor_dseg_C7A2 ; Signed Multiply
		seg007:6361 10C                 cwd                     ; AX -> DX:AX (with sign)
		seg007:6362 10C                 xor     ax, dx          ; Logical Exclusive OR
		seg007:6364 10C                 sub     ax, dx          ; Integer Subtraction
		seg007:6366 10C                 mov     cx, 2
		seg007:6369 10C                 sar     ax, cl          ; Shift Arithmetic Right
		seg007:636B 10C                 xor     ax, dx          ; Logical Exclusive OR
		seg007:636D 10C                 sub     ax, dx          ; Integer Subtraction
		seg007:636F 10C                 add     [bp+var_pollutionProd], ax ; Add
		seg007:6373 10C                 mov     bx, cityOwner
		seg007:6377 10C                 shl     bx, 1           ; Shift Logical Left
		seg007:6379 10C                 mov     ax, perCivTechCount[bx]
		seg007:637D 10C                 imul    difficultyLevel ; Signed Multiply
		seg007:6381 10C                 cwd                     ; AX -> DX:AX (with sign)
		seg007:6382 10C                 sub     ax, dx          ; Integer Subtraction
		seg007:6384 10C                 sar     ax, 1           ; Shift Arithmetic Right
		seg007:6386 10C                 sub     ax, 100h        ; Integer Subtraction
		seg007:6389 10C                 neg     ax              ; Two's Complement Negation
		seg007:638B 10C                 push    ax
		seg007:638C 10E                 call    random_N        ; Call Procedure
		seg007:638C
		seg007:6391 10E                 add     sp, 2           ; Add
		seg007:6394 10C                 mov     cx, [bp+var_pollutionProd]
		seg007:6398 10C                 shl     cx, 1           ; Shift Logical Left
		seg007:639A 10C                 cmp     cx, ax          ; Compare Two Operands
		seg007:639C 10C                 jg      short seg007_63A1 ;

This code block is one of the first condition to be assessed when checking whether pollution should be spawned.

Re-writing the condition in a single formula, it gives this:
IF ( 2 * CityPollution > Random(256 - CityOwnerTechCount * difficultyLevel / 2) ) THEN AddPollution

In the above, CityPollution is computed as explained in other threads:
CityPollution = CityShields / CityPowerType - 20 + CitySize * PollutionFactor

CityPowerType depends on the power infrastructure in the city (default/power plant = 1, hydro/nuke plant = 2, recycling center = 3)
PollutionFactor depends on the "polluting" techs known by the player (INDUSTRIALIZATION=1, AUTOMOBILE=2, MASS PRODUCTION=3 and PLASTICS=4), and whether the city has a MASS TRANSIT (= 0).

So we can see that multiple combination are possible, but the key to this bug lies in the RANDOM() call: what if CityOwnerTechCount * difficultyLevel / 2 is more than 256?

Then the RANDOM() call has a negative argument, for which it will always return zero.

In other words, as soon as one gets (256*2/difficultyLevel) Techs, pollution starts to get triggered at every turn.

Let's make a list:
  • Chieftain = 0: (256*2/0) = +infinity -> pollution bug never triggered
  • Warlord = 1: (256*2/1) = 512 -> pollution bug triggered after Future Tech. 444 (512 - 68 non-future techs)
  • Prince = 2: (256*2/2) = 256 -> pollution bug triggered after Future Tech. 188 (256 - 68 non-future techs)
  • King = 3: (256*2/3) = 170 -> pollution bug triggered after Future Tech. 102 (170 - 68 non-future techs)
  • Emperor = 4: (256*2/4) = 128 -> pollution bug triggered after Future Tech. 60 (128 - 68 non-future techs)

The theoretical numbers seem to more or less match what people have reported in the various threads above, although some other unknown conditions may add variations.

Anyhow, for me, the riddle is solved.

The riddle in the riddle that isn't solved, though, is: was this done on purpose? I guess we'll never know... ;)

Now, how would you guys out there suggest to patch it?
 
I can't speculate on what might be wrong, but this formula suggests that if a Mass Transit and Recycling center are both present, a city of any size can produce up to 60 shields before 2*CityPollution > 0. I guess if we're dealing with a &#8805; or the engine is allowed to generate a signed random value (you'd know better than me) then I think the bug would occuring without fail through that formula.

This is a magnificent find by the way. One patch proposal would be to throw a line returning the lesser of CityOwnerTechCount and 68 and use that value in place of the direct TechCount— freezing the effect of future techs.

But if the formula is exactly right, then I think the in-game, unmodded work-around is more than sufficient: ensure that cities have the Transit and Recycle and aren't breaking the ceiling with shields. If you can get to future tech 60 than surely you can afford a recycling center...
 
Isn't it possible (and realistic) to just not take future technologies into account when calculating total advances? I would hope that the future brings us cleaner and greener technologies.
 
...or the engine is allowed to generate a signed random value...

I originally thought that the CIV.EXE's Random routine would simply return 0 if the argument is negative, but my Java port of this routine will actually return negative values when the argument is negative...

If it returns negative values, then it makes the bug all the more prominent since even when benefiting from 20 free chimneys, 2*CityPollution might be higher than the Ranom result!

Needs to be checked in-game...
 
I originally thought that the CIV.EXE's Random routine would simply return 0 if the argument is negative, but my Java port of this routine will actually return negative values when the argument is negative...

If it returns negative values, then it makes the bug all the more prominent since even when benefiting from 20 free chimneys, 2*CityPollution might be higher than the Ranom result!

Needs to be checked in-game...

So as things stand, I suppose this would be a prime suspect on why the bug is occurring. Speaking from some experience (future tech 64 on my part) this bug is very prominent indeed, but one big issue is lack of documentation. Few games are played out to this point and of those next to none have been walked through carefully for testing.
 
Isn't it possible (and realistic) to just not take future technologies into account when calculating total advances? I would hope that the future brings us cleaner and greener technologies.

Yes this is what Tristan_C is proposing by replacing "CityOwnerTechCount" with "MIN(CityOwnerTechCount, 68)"... But to implement this in assembly is difficult, mainly because of the lack of room to put the patch bytes.
 
Now, how would you guys out there suggest to patch it?

The simplest patch would be to either replace the difficulty with 0 (always chieftan), or to change one of the modifiers to push the bug back further (divide by 4 or 6), but those both reduce all pollution everywhere, affecting game difficulty.
IF ( 2 * CityPollution > Random(256) ) THEN AddPollution
or
IF ( 2 * CityPollution > Random(256 - CityOwnerTechCount * difficultyLevel / 4) ) THEN AddPollution

A cool solution (doubtful to be able to be done in place) would be to take the absolute value before calling the rand(), so that future techs eventually reduced pollution. There would still be a period where pollution would be rampant...
IF ( 2 * CityPollution > Random(abs(256 - CityOwnerTechCount * difficultyLevel / 2) ) ) THEN AddPollution

Something that might be possible and more in line with what the developers wanted, would be using PollutionFactor*multiplier instead of CityOwnerTechCount, where multiplier would probably be 16 or 17 (researching plastics gives you the same value as having all techs researched). Now, I'm familiar with assembly (from a CS class almost 10 years ago), but far from fluent, so I'm not entirely sure if PollutionFactor is available for use at that point in the calculations. It would also be nice if mass transits didn't factor in (otherwise having one would give you Chieftan difficulty pollution levels), but I think it would be awesome if it could be implemented this way.
IF ( 2 * CityPollution > Random(256 - PollutionFactor * difficultyLevel * 8) ) THEN AddPollution
 
Would you not be able to add a procedure at the end of the file and rewrite the function there? Then just call that.
 
Random(256 - CityOwnerTechCount * difficultyLevel / 2)
As we know (bump), on Emperor this returns 0 at Future Tech 60.

This RNG call as described returns a range,
[0,-2] at FT 61
[0,-4] at FT 62
[0,-6] at FT 63
...and so on.

A city with 0 production and a Mass Transit is defined as having -20 pollution as per the CityPollution formula... -20 being the integer 'buffer' added when computing pollution from shields.
So if I am reading everything correctly so far, when you reach FT 71 (range [0, -22]), even a 100% clean city has a chance of adding a pollution tile.

As an aside, and just to be clear, at FT 71 a city generating 60 shields, with Recycling and Transit, shows exactly 0 smokestacks on the city screen. But it only has about a 4% chance of not adding a pollution tile. With higher FT the chance of polluting continues to approach but not reach 100% as it becomes less and less likely the RNG will roll you a 0 in range [0, -x].

This leads up to a question. What happens at FT 189?

At FT 189 the CityOwnerTechCount (which includes the 68 non-future techs) is over 256. Does it overflow? It tells the engine to return the range [0, -258] on paper, but does it? If it's possible to bug this value twice, then it's possible to research beyond the pollution bug and eliminate it.
 
Hi all!
Darkpanda have done all hard work of investigations in CIV sources.

In my opinion authors did not tested this lines of code and resulting gameplay.
So I decided to solve this bug with little patch of CIV.exe.

You can find with any hex-editor such bytes:
Code:
99 2B C2 D1 F8 2D 00 01 F7 D8 50
and replace with this new:
Code:
99 99 D1 F8 2D 00 01 99 33 C2 50
.

I also make an patch-file for use with JCivEd-program. Put this file in 'patches' folder of this program.

With this patch, you can research as many FutureTechs as you want. Pollution bug will not happen. Usually when you have many technologies discovered, pollution was triggering every turn in every city, although you have building MassTransit in this cities, and have zero 0 smokestacks on the city screen. Even so, with this patch you will continue to get pollution as formerly - if city have any smokestacks you will get the possibility of pollution near this city.
Civilization authors did not checked negative result of SUBstraction operation in probability formula of pollution, and did not make the absolute value before Random() formula, resulting in 100% chance of pollution if you have many techs.

My realization does not resolve all problems i think, because when you get many many techs discovered (for ex. 400 or more) - the chance of pollution in your 'really dirty' cities will be very-very little. And more you get techs - less chance of pollution in this city with smokestacks.

This Bug can also be corrected with another ways - reconstruction of author's formula. For ex. with maximizing the chance of pollution when civilization have 256 techs or more. Replacing the right part with value "1 or 2 or 3" of expression: IF (2*CityPollution > Random(256 - CityOwnerTechCount*difficultyLevel/2) ) THEN AddPollution. But such solution we cannot implement in the existing 3-4 bytes.... i think...


P.S.
and little explanation what I changed. A only make an ABSolute of value before RANDOM.
Now it is:
IF (2*CityPollution > Random(ABS(256 - CityOwnerTechCount*difficultyLevel/2)) ) THEN AddPollution.

Previus code:
Code:
seg007:6381                 cwd
seg007:6382                 sub     ax, dx
seg007:6384                 sar     ax, 1
seg007:6386                 sub     ax, 100h
seg007:6389                 neg     ax

With patch:
Code:
seg007:6381                 cwd
seg007:6382                 cwd
seg007:6383                 sar     ax, 1
seg007:6385                 sub     ax, 100h
seg007:6388                 cwd
seg007:6389                 xor     ax, dx

P.S. updated patch file with FRENCH ver.
 

Attachments

  • patch050_no_pollution_bug_with_many_FutureTech.xml
    3.2 KB · Views: 52
Last edited:
hi.

my small mod for your mod : I added the patch for my civ version in your XML

big thanks for this patch :)

Code:
<data versions="FR47405"> <parts>
       <part>   <offset> 0x13D02 </offset>
              <bytes>99 99 D1 F8 2D 00 01 99 33 C2 50</bytes>
           <original>99 2B C2 D1 F8 2D 00 01 F7 D8 50</original> </part>
   </parts></data>
 
That's great news for all score freaks out there. I guess they aren't flooding the thread with happy replies because they're too busy pushing caravans towards future tech 1024.
 
future tech 1024
Mamma mia, in Civ Genesis I use only 1 byte for future tech counter (but on the other hand, every civ has it). I thought no one in their right mind will ever need more than 255 Future Tech... Well, right mind and fanatics of 30-year old game is not compatible things.

And about various fixes, I thought about them and probably they all kinda not fully satisfactory without changing of number of bytes . Maybe best solution is to change logic within random() for negative values? If it always will return 0 in this case, everything should be ok, I think? EDIT: there's no easy way to do this either.
 
Last edited:
. I thought no one in their right mind will ever need more than 255 Future Tech...
Original CIV has the WORD for number of FutureTechs. In many games I get more than 250 techs..

In original CIV there all with WORD. But many operations (multiply|Divide) are signed(O_o) even when manipulating such values (that cannot be negative, and should always be positive,unsigned).

About Random() - hmmm , I dont know. Such obfustraction to only positive randome() can be used when you absolutly know all code. What If there some places that really use negative randoms?
 
I've just hit this bug. Going for a high score - currently 1890AD and ~275% on emperor, but that's going to fall fast if I cant find a fix for this as my settlers will not be able to keep up with the cleanup.
 
They're too busy pushing caravans towards future tech 1024.

Not that I'm anywhere near future tech 1024, but I want to see what happens when an emperor discovers future tech 1103.
With future tech 1103 or 1104, the number of shields goes past 32767 and overflows a 16-bit integer.
My guess is that is where additional discoveries become impossible (except for building Darwin's Voyage which I'm saving for the occasion).
 
Top Bottom