Opened 7 years ago

Last modified 6 years ago

#4805 closed defect

Remove extreme amounts of rmgen duplication — at Version 1

Reported by: elexis Owned by: elexis
Priority: Must Have Milestone: Alpha 23
Component: Maps Keywords:
Cc: Patch:

Description (last modified by elexis)

The random map scripts have been created by copying code instead of moving it to a helper function and calling it multiple times.

Global copies:

  • 50-60 copies of the initial base and starting resources placement code. This means if we want nomad on all maps, add retry loops to check for resource collisions with the map or iberian walls, all copies have to be changed (refs #4600, #4796).
    • Includes dozens of copies of slightly modified radial, linear or river player placement code.
  • Several hundred of createArea calls that copy a comment explaining the arguments in every call, could become twice as short and not violate the variable declaration scope if they would inline variables. Many createArea calls also contain tileClass painters that are useless, since they are overwritten by a later paintTerrainBasedOnHeight (every painter in unknown*js).
  • About 1400 of comments that are identical to the log entry one line below (removed in r20144)
  • Two dozen of copies that use the P_FOREST mechanism to create forests
  • Two dozen of copies of a riverPlacement code.
  • Dozens of mine, tree and grass patch placement functions that can probably be abstracted.
  • A workaround is copied to every PathPlacer call that is supposed to not end shortly before the map border (most importantly for rivers like on Lorraine Plain or on the mountains in the Unknown maps with the Passes variant).

Copies within maps:

  • Unknown, Unknown Land and Unknown Nomad (r12545, r12654, r12747) are a very long copy of each other with different intransparent bugfixes (r14162) applied to different parts of the different maps.
  • Snowflake Searocks copied the entire map generation code about 8 times with negligibly different constants for different mapsizes
  • Corsica & Sardinia (r11266) copies the island generation code for Corsica and Sardinia
  • Cycladic Archipelago (r11137) has a copy to create an Island for the player islands and the central islands
  • Fortress (r11361) comes with twice the number of starting units and achieves that by calling a rmgen function and then appending a copy of that function ot the map.
  • Wild Lake (r19704) copies the starting location code from Caledonian Meadows
  • Islands copies the island generation code (but I didn't find a way to abstract cleanly)
  • Lorraine Plain, English Channel and the Unknown maps copy the tributary generation code

Change History (1)

comment:1 by elexis, 7 years ago

Description: modified (diff)

Didn't check anything before r13948.

Alpha 20:

In r17728:
Move duplicate code of the area- and object-group placing methods to retryPlacing and two coordinate-randomization functions.

In r17910:
Remove unused variables, duplicate comments and trailing whitespace from volcanic lands.

Alpha 21:

In r17981:
Remove triple X comment pollution from maps.

In r18142 by FeXoR:
Removing function euclid_distance as duplication of getDistance.

In r18816:
Don't place chicken inside civic centers and unify chicken placement code.

In r18871:
Remove duplicate fish placement from english channel.

In r18872:
Remove unused clWater and clRiver tile class from maps that don't have water nor rivers.

In r18873:
Remove duplicate comments and add more progress updates.

Alpha 22:

In r19236:
Remove non-functional shallow generation code from the Unknown random maps.

In r19282: Replace more than 30 duplicate grass tuft placement calls with a single function call.

In r19305:
Use pickRandom in random map scripts.
Patch By: bb

In r19453:
Remove useless terrain painting that is overwritten with the paintTerrainBasedOnHeight calls following that.

In r19721:
Delete rmgen randInt duplicate. Patch By: bb

In r19807:
Move duplicate rmgen functions from caledonian meadows and wild lake to the rmgen libraries.

Alpha 23:

In r20144:
Remove 1400 useless random map script comments.

In r20146:
Remove 23 copies of the random mapgen terrain init loop (including 16 copies of unused x and z vars).

In r20148:
Replace 55 playerarray sorting loops copies with calls to two random mapgen helper functions. Rename randomizePlayers as it paradoxically called sortPlayers on the result.

In r20149:
Unify 36 copies of the radial playerplacement loop of random mapscripts. Trim unused PlayerAngle, StartAngle and baseRadius variables.

In r20151:
Merge the 14 copies of the player placement code on river maps.

In r20153:
Create arrays with [], not new Array in random map scripts

In r20185:
Reverse engineer and unify the 14 copies of the river drawing random mapgen code.

Splits unrecognizably blended magic numbers and makes them available for map editors.

Removes copied terms of copies of conditions of copied of functions,
unused variables, overwritten values, checks that are always true, dead code from checks that are always false and
things like 9 pairs of unneeded parentheses in a single line that was copied several times.

Remove the useless RectPlacer terrain painting call on Guadalquivir River.
Make danubius river borders parallel and drop thetha/seed differences.
Use planar instead of slopy water ground level for Phoenician Levant.

Copied and obscured by
r11137 Aegan Sea, Guadalquivir River, Hyrcanian Shores, Phoenician Levan, Nile,
r12545 Unknown,
r12654 Unknown Land,
r12747 Unknown Nomad,
r12786 English Channel,
r13134 Kerala,
r19434 Danubius.

In r20186:
Unify 34 copies of the civic center tile class setting of random map scripts.

In r20189:
Remove some more recursive duplication in the rmgen library (painting terrain based on height).

In r20222:
Fix division by zero in r12545 and r12654 for the gulf map layout with only one player.

In r20233:
Northern lights river cleanup following r20185.

Note: See TracTickets for help on using tickets.