#4805 closed defect (fixed)
Remove extreme amounts of rmgen duplication
Reported by: | elexis | Owned by: | elexis |
---|---|---|---|
Priority: | Must Have | Milestone: | Alpha 23 |
Component: | Maps | Keywords: | |
Cc: | Patch: |
Description (last modified by )
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 laterpaintTerrainBasedOnHeight
(every painter inunknown*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 thePasses
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 (56)
comment:1 by , 7 years ago
Description: | modified (diff) |
---|
comment:4 by , 7 years ago
In r20296:
Corsica & Sardinia cleanup.
Unify clCorsica, clSardinia to clIsland and the duplicate resource placement calls using that.
Move and simplify the logic determining which texture to paint depending on height and slope to a function getCosricaSardiniaTerrain.
Inline the createTerrain to greatly increase readability while possibly reducing performance slightly.
Fix whitespace of createObjectGroup calls.
comment:6 by , 7 years ago
In r20301:
Cleanup most createArea and some createObjectGroup calls in random map scripts.
Inline the placer, terrainPainter, elevationPainter, group and sizes helper variables that are used only once.
Thus do not hoist variable declarations from unrelated scopes.
Remove few dozens of painters that are overpainted by the paintBasedOnTerrainHeight call afterwards.
Remove 334 noisy misaligned code comments ( terrains widths blend radius and some variation).
Label the createArea calls so that the reader knows what they do.
Remove unbenefitial Math.PI/8 grass min/max angle limitation.
Use height variables for the SmoothElevationPainter calls so that levels can be modified more easily.
Use Math prototype functions instead of the unfortunate rmgen proxies.
Does not touch starting base code, Unknown maps, Snowflake Searocks and Corsica & Sardinia which need special treatment.
comment:7 by , 7 years ago
In r20306:
Unify duplicate Corsica & Sardinia island generation, refs rP11266.
The code for the other island was copied in a varying order with only the values changed to 1 - x.
Unite the island, subisland, creeks, beaches, main relief, first plateau, second plateau and passages
code under a single loop to display the logical relation
instead of scattering many paragraphs and executing unrelated (starting base) code in between.
Cleanup affected createArea calls as in rP20301.
comment:8 by , 7 years ago
In r20308:
Unify beyond ridiculous Snowflake Searocks duplication gore, refs r11137.
The island generation code was copied once for
the small mapsize and 2 players
the small mapsize and 3 players
the small mapsize and 4 players
the small mapsize and more than 4 players
the medium mapsize and up to 4 players
the medium mapsize and more than 4 players
the normal mapsize and up to 5 players
the normal mapsize and up to 7 players
the normal mapsize and 8 players
the large mapsize and up to 5 players
the large mapsize and more than 5 players
Within each of these copies there were three copies of the island createArea+placer+painters call and
about 10 unreadable island connection setter copies.
Support mapgenerations with only one player.
Cleanup createAreas calls as in rP20301 and broken indentation.
Fix missing forests due to a missing parentheses in the forest count of that commit.
comment:18 by , 7 years ago
In r20420:
Obliterate Unknown (r12545), Unknown Land (r12654) and Unknown Nomad (r12747) triplication of doom, fixes #4317.
Refactor by moving from procedural programming to functional programming.
Fix the horrendous shoreline performance bug on "Continent" of Unknown as in r14162 by dropping the historic differences between the maps.
Remove misleading player coordinate computation when generating the terrain on Unknown Nomad, because the players are spawned elsewhere then.
Remove names like md, mdd1 and mdd2 with different connotations in every landscape type.
Remove six more paintRiver copies in the "Edge Seas" variant as in r20185.
Remove createShoreJaggedness and createExtensionsOrIslands duplication within this triplication.
Remove heightmap initialization octuplication by adding initHeight.
Remove horizontal/vertical duplication by using logical operators.
Remove Painters redundant with the paintTerrainBasedOnHeight calls, pointless comments and cleanup createArea calls as in r20301.
Remove some forest helper duplication as in r20406.
Remove references to Math proxies and use the prototype functions directly.
Remove whitespace issues.
Describe remaining calls with human-readable log messages.
Always spawn enough treasures for a CC (instead of sometimes only giving enough for a dock), refs r16447.
Generate "Mainland" variant with the same probability as the other landscape types.
Generate "Central Sea" variant on Unknown Land too by forcing an Isthmus then.
Don't split players on Unknown Nomad on the "Rivers And Lake" variation, because the resource imbalance is too drastic.
Consistently call markPlayerArea before terrain generation on non-Nomad maps.
Differential Revision: https://code.wildfiregames.com/D252
comment:19 by , 7 years ago
In r20429:
Extend the random map river algorithm (r20185) to allow arbitrary start and end points.
Reverse engineer and cleanup obfuscated Rivers map code, refs r11137. Use vector algebra to replace magic equations and express geometric intend. Fix seed typo by removing river curve duplicate.
On the Rivers map, use areAllies directly instead of copying the diplomacies to a 2D array called isRiver. Use modulo operator instead of an if-statement with duplication to determine the neighbor of the last player, refs r11174.
comment:20 by , 7 years ago
In r20437: Unify rmgen modifyTilesBasedOnHeight (r20189) with the HeightPlacer (r20361).
Create global constants for the elevation modes and make that Placer more versatile by adding the mode argument.
Improves the performance of these functions by not repeating the mode, createTerrain and getTileClass lookups every tile.
Move the functions to library.js, refs #4804.
comment:21 by , 7 years ago
comment:25 by , 7 years ago
Milestone: | Alpha 23 → Backlog |
---|---|
Owner: | removed |
Status: | assigned → new |
comment:36 by , 7 years ago
Milestone: | Backlog → Alpha 23 |
---|
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.