This code is very closely related to Micropolis::doPopNum(). Maybe merge both in some way? (This function gets called much more often however then doPopNum(). Also, at the first call, the difference between thisCityPop and cityPop is huge.)

Member Micropolis::cityEvaluation ()

Handle lack of voting explicitly

: These strings should not be hard coded into the core simulator. The scripting language should look them up in translation files.

Member Micropolis::cityMonth

Remove, can be computed from Micropolis::cityTime.

Member Micropolis::cityYear

Remove, can be computed from Micropolis::cityTime.

Member Micropolis::collectTax ()

Do something with z? Check old Mac code to see if it's used.

Check old Mac code to see if it's ever set, and why.

Apparently taxFlag is never set to true in MicropolisEngine or the TCL code, so this always runs.

Break out so the user interface can configure this.

Member Micropolis::decRateOfGrowthMap ()

Limiting rate should not be done here, but when we add a new value to it.

Member Micropolis::destroySprite (SimSprite *sprite)

Break the connection between any views that are following this sprite.

Member Micropolis::doAirplaneSprite (SimSprite *sprite)

Remove local magic constants and document the code.

absDist gets updated by Micropolis::getDir(), which is not always called before reading it (or worse, we just turned towards the old destination).

Member Micropolis::doAnimation

Currently always true. Should connect it to a frontend option.

Member Micropolis::doBridge (const Position &pos, MapTile tile)

What does this function return?

Discover the structure of all the magic constants.

Member Micropolis::doBudgetNow (bool fromMenu)

Simplify this code. Instead of this nested mess, make a sequence of assigning funds to road, fire, and police.

Why are we not subtracting from yumDuckets what we spend, like the code below is doing?

Member Micropolis::doBusSprite (SimSprite *sprite)

Remove local magic constants and document the code.

Member Micropolis::doCommercial (const Position &pos, bool zonePower)

Make zonePwrFlg a boolean.

Member Micropolis::doCopterSprite (SimSprite *sprite)

Remove local magic constants and document the code.

Member Micropolis::doDisasters ()

Decide what to do with the 'nothing happens' disaster (since the chance that a disaster happens is expressed in the DisChance table).

Member Micropolis::doFire (const Position &pos)

Needs a notion of iterative neighbour tiles computing.

Use a getFromMap()-like function here.

Extract constants of fire station effectiveness from here.

Member Micropolis::doFlood (const Position &pos)

Use some form of rotating around a position.

Member Micropolis::doIndustrial (const Position &pos, bool zonePower)

Make zonePwrFlg a boolean.

Member Micropolis::doMessages

Not currently used, should hook it up.

Member Micropolis::doMonsterSprite (SimSprite *sprite)

Remove local magic constants and document the code.

Member Micropolis::doNotices

Not currently used, should hook it up.

Member Micropolis::doShipSprite (SimSprite *sprite)

Remove local magic constants and document the code.

Member Micropolis::doSimInit ()

Create constants for initSimLoad.

Member Micropolis::doTornadoSprite (SimSprite *sprite)

Remove local magic constants and document the code.

Member Micropolis::doTrainSprite (SimSprite *sprite)

Remove local magic constants and document the code.

Member Micropolis::doWinGame ()

This may not be called. Call it when appropriate.

Member Micropolis::drawMonth (short *hist, unsigned char *s, float scale)

Figure out why we copy data.

Member Micropolis::drawStadium (const Position &center, MapTile z)

Merge with zonePlop()-like function.

Member Micropolis::explodeSprite (SimSprite *sprite)

Add a 'bus crashed' message to MessageNumber.

Member Micropolis::findPerimeterRoad (Position *pos)

We could randomize the search.

Member Micropolis::findPerimeterTelecom (const Position &pos)

Decide whether we want telecomm code.

Member Micropolis::fireAnalysis ()

Comment seems wrong; what's a firerate map?

Member Micropolis::forestTool (short x, short y, ToolEffects *effects)

bulldozer should be free in terrain mode or from a free tool.

Member Micropolis::getAssessedValue ()

Make function return the value, or change the name of the function.

Member Micropolis::getCityClass (Quad cityPop)

Put people counts into a table.

Member Micropolis::getCrimeRate (int x, int y)

Use world coordinates instead (use crimeRateMap.worldGet() instead).

Member Micropolis::getDir (int orgX, int orgY, int desX, int desY)

Remove local magic constants and document the code.

Member Micropolis::getFireCoverage (int x, int y)

Use world coordinates instead (use fireStationEffectMap.worldGet() instead).

Member Micropolis::getLandValue (int x, int y)

Use world coordinates instead (use landValueMap.worldGet() instead).

Member Micropolis::getPoliceCoverage (int x, int y)

Use world coordinates instead (use policeStationEffectMap.worldGet() instead).

Member Micropolis::getPollutionDensity (int x, int y)

Use world coordinates instead (use pollutionDensityMap.worldGet() instead).

Member Micropolis::getPopulationDensity (int x, int y)

Use world coordinates instead (use populationDensityMap.worldGet() instead).

Member Micropolis::getPowerGrid (int x, int y)

Use world coordinates instead (use powerGridMap.worldGet() instead).

Member Micropolis::getRateOfGrowth (int x, int y)

Use world coordinates instead (use rateOfGrowthMap.worldGet() instead).

Member Micropolis::getScore (const short problemTable[PROBNUM])

Should this expression depend on CVP_NUMPROBLEMS?

Member Micropolis::getTrafficDensity (int x, int y)

Use world coordinates instead (use trafficDensityMap.worldGet() instead).

Member Micropolis::graph10Max

Write-only variable. Can it be removed?

Member Micropolis::graph120Max

Write-only variable. Can it be removed?

Member Micropolis::historyInitialized

Nobody uses this variable. Can it be removed?

Member Micropolis::initGame ()

we seem to have several of these functions.

Member Micropolis::initSprite (SimSprite *sprite, int x, int y)

Make derived classes for each type.

Move code to (derived) SimSprite methods.

Member Micropolis::landTool (short x, short y, ToolEffects *effects)

: Is this good? It is not auto-bulldoze!!

: Handle result value (probably)

Member Micropolis::loadCity (const std::string &filename)

In what state is the game left when loading fails?

String normalization code is duplicated in Micropolis::saveCityAs(). Extract to a sub-function.

Member Micropolis::makeFlood ()

Use Direction and some form of XYPosition class here

Member Micropolis::makeMeltdown ()

Randomize which nuke plant melts down.

Member Micropolis::makeMonster ()

Make monster over land, because it disappears if it's made over water. Better yet make monster not disappear for a while after it's created, over land or water. Should never disappear prematurely.

Member Micropolis::moveObjects ()

It uses SimSprite::name[0] == '\0' as condition which seems stupid.

Micropolis::destroySprite modifies the Micropolis::spriteList while we loop over it.

Member Micropolis::networkTool (short x, short y, ToolEffects *effects)

Is this ever used?

Member Micropolis::newMap

Write-only variable, can be removed?

Member Micropolis::newMapFlags [MAP_TYPE_COUNT]

Write-only variable, can be removed?

Member Micropolis::populationDensityScan ()

The tempMap1 has MAP_BLOCKSIZE > 1, so we may be able to optimize the first x, y loop.

Member Micropolis::putBuilding (int leftX, int topY, int sizeX, int sizeY, MapTile baseTile, bool aniFlag, ToolEffects *effects)

We should ask the buildings themselves how they should be drawn.

Member Micropolis::putDownForest (short mapH, short mapV, ToolEffects *effects)

Auto-bulldoze deducts always 1.

Member Micropolis::putDownLand (short mapH, short mapV, ToolEffects *effects)

Auto-bulldoze deducts always 1.

Auto-bulldoze costs should be pulled from a table/constant.

Member Micropolis::putDownNetwork (short mapH, short mapV, ToolEffects *effects)

Auto-bulldoze costs should be pulled from a table/constant.

Member Micropolis::putDownPark (short mapH, short mapV, ToolEffects *effects)

Add auto-bulldoze? (seems to be missing).

Member Micropolis::resetEditorState ()

What should be done with this empty function?

Member Micropolis::resetMapState ()

What should be done with this empty function?

Member Micropolis::roadTool (short x, short y, ToolEffects *effects)

Merge roadTool, railroadTool, wireTool, and parkTool functions.

Member Micropolis::saveCity ()

This is a no-op if the Micropolis::cityFileName is empty. In that case, we should probably warn the user about the failure.

Member Micropolis::saveCityAs (const std::string &filename)

String normalization code is duplicated in Micropolis::loadCity(). Extract to a sub-function.

Member Micropolis::saveFile (const std::string &filename)

Report error saving file.

Member Micropolis::setCrimeRate (int x, int y, int rate)

Use world coordinates instead (use crimeRateMap.worldSet() instead).

Member Micropolis::setFireCoverage (int x, int y, int coverage)

Use world coordinates instead (use fireStationEffectMap.worldSet() instead).

Member Micropolis::setLandValue (int x, int y, int value)

Use world coordinates instead (use landValueMap.worldSet() instead).

Member Micropolis::setPoliceCoverage (int x, int y, int coverage)

Use world coordinates instead (use policeStationEffectMap.worldSet() instead).

Member Micropolis::setPollutionDensity (int x, int y, int density)

Use world coordinates instead (use pollutionDensityMap.worldSet() instead).

Member Micropolis::setPopulationDensity (int x, int y, int density)

Use world coordinates instead (use populationDensityMap.worldSet() instead).

Member Micropolis::setPowerGrid (int x, int y, int power)

Use world coordinates instead (use powerGridMap.worldSet() instead).

Member Micropolis::setRateOfGrowth (int x, int y, int rate)

Use world coordinates instead (use rateOfGrowthMap.worldSet() instead).

Member Micropolis::setSmoke (const Position &pos, bool zonePower)

Why do we assign the same map position twice?

Add #SMOKEBASE into aniTabA and aniTabB tables?

Why do we assign the same map position twice?

Why div 8? Industry is 9 tiles long!!

Member Micropolis::setTrafficDensity (int x, int y, int density)

Use world coordinates instead (use trafficDensityMap.worldSet() instead).

Member Micropolis::setValves ()

Break the interesting values out into public member variables so the user interface can display them.

Break the tax table out into configurable parameters.

Make configurable parameters.

Member Micropolis::simHeat ()

Why is Micropolis::cellSrc not allocated together with all the other variables?

What is the purpose of this function?

KILL the define.

Member Micropolis::simLoadInit ()

What purpose does this serve? Weird...

Member Micropolis::simPaused

Variable has reversed logic, maybe use sim_running instead?

Member Micropolis::simRandom ()

Use Wolfram's fast cellular automata pseudo random number generator.

Member Micropolis::simTick ()

What is the purpose of this function? (also in relation with Micropolis::simUpdate()).

Member Micropolis::simUpdate ()

What is the purpose of this function? (also in relation with Micropolis::simTick()).

Member Micropolis::take10Census ()

Rename to takeMonthlyCensus (or takeMonthlySnaphshot?).

A lot of this max stuff is also done in graph.cpp

Member Micropolis::taxFlag

Apparently taxFlag is never set to true in MicropolisEngine or the TCL code.

Don should check old Mac code to see if it's ever set.

Variable is always 0. Decide whether to keep it, and if yes, create means to modify its value

Member Micropolis::testForConductive (const Position &pos, Direction2 testDir)

Re-use something like Micropolis::getFromMap(), and fold this function into its caller.

Member Micropolis::tickCount ()

Figure out what a 'tick' is.

Member Micropolis::tilesAnimated

Not currently used, should hook it up.

Member Micropolis::toolDown (EditingTool tool, short tileX, short tileY)

: Multi player: This sound should only be heard by the user who called this function.

Member Micropolis::tryOther (int Tpoo, int Told, int Tnew)

Figure out what this function is doing.

Remove local magic constants and document the code.

Member Micropolis::turnTo (int p, int d)

Remove local magic constants and document the code.

Member Micropolis::updateUserInterface ()

Keeping track of pending updates should be moved to the interface (the simulator generates events, the interface forwards them to the GUI when possible/allowed.

Send all pending update messages to the user interface.

Member not_reached (int line, const char *fname)

Generalize error handling/exiting of the simulator.

Class SimSprite

SimSprite::name is never set to anything else than "", and only used to detect a non-removed non-active sprite (in a non-robust way).

Member Tiles

Add TILE_ prefix

Make LOW/BASE and LAST/HIGH consistent everywhere?

Figure out what sprite groups really exist (maybe we can learn more by examining the actual sprites, and/or by using hexadecimal or bite-wise notation?)

Add comments for each sprite (0–1023)

Class ToolEffects

Extend the class for storing messages and sounds.

Member ToolResult

Make the rest of the code use this instead of magic numbers.

Member WORLD_W_2

Make a Map class that keeps its 2x2 square storage details internally, so the code doesn't need to bother with it.

Member WORLD_W_4

Make a Map class that keeps its 4x4 square storage details internally, so the code doesn't need to bother with it.

Member WORLD_W_8

Make a Map class that keeps its 8x8 square storage details internally, so the code doesn't need to bother with it.