Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#4804 closed defect (fixed)

Sort rmgen functions by logic

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

Description (last modified by elexis)

The old school rmgen directory is a bit messy, because three of the files don't have a clear separation of functions according to logic:

  • utilityfunctions.js contains some frequently used elevation and entity placement helpers.
    • The filename is arbitrary and should be renamed before random unrelated code is added there.
    • It could be split to terrain_helpers.js and placer_helpers.js or similar.
    • Its tileClass hardcodings can lead to reference errors and have to be removed.
  • misc.js
    • contains some frequently helper functions for the starting base that should be moved to a new file startingentities.js. That file will be extended with new helper functions that will replace the duplication of the starting base code in every rmgen file.
    • contains some frequently used terrain and placer helper functions too.
    • contains a lot of linear algebra unrelated to any rmgen implementation and should be moved to a new file
    • contains some rmgen1 brand functions which are deeply connected to the code in constraint.js, tileclass.js and painter.js and hence should be moved there
    • contains the four modifyTilesBasedOnHeight functions. They could be either moved to the three files above or the other short global helpers which don't fit anywhere in library.js.
  • library.js:
    • contains some starting base functions which should be moved to the new file.
    • contains some engine hardcodings, which should be replaced with an engine getter sometime (FeXoR ticket somewhere)
    • contains some useful short globals like fractionToTiles
    • contains a lot of silly proxies like sin = Math.sin and PI = Math.PI which should be deleted
    • contains some unrelated math helpers like getAngle or getDistance which can be moved to the new math file

After that, library.js will be about 100 to 200 lines of code only, misc.js and utilityfunctions.js deleted. So it will be easier for new readers to understand the rmgen library.

placers.js and pathplacer.js will become placer_centered.js (ClumpPlacer, ChainPlacer, can be used at random places meeting a given constraint) and placers_noncentered.js (RectPlacer, PathPlacer, can only be used to place at hardcoded coordinates).

Change History (34)

comment:1 Changed 2 years ago by elexis

In 20295:

Replace hardcoded trigonometry magic numbers on Corinthian Isthmus with a rotateCoordinates computation, allowing to change the river angle arbitrarily.

The new math library file will contain five more math helpers used by random maps, refs #4804.
These functions are agnostic of random map vocabulary, so could possibly be moved to or unified with globalscripts.

Cleanup and label obscure createArea calls in this map, unify duplicate magic numbers, refs #4805.
Remove three useless LayeredPainter? calls that are overwritten by the paintTerrainBasedOnHeight tShore and tWater calls afterwards.

comment:2 Changed 2 years ago by elexis

Description: modified (diff)

comment:3 Changed 2 years ago by elexis

After the functions were moved to separate files as mentioned above,
the files can be grouped by logic in new rmgen libraries, allowing us to
build a clean software stack afterwards.

Layer 0 (Pyrogenesis C++): C++ random map engine

Layer 1 (Pyrogenesis Interface): JS library providing a blank interface to the engine

Layer 2 (Map Generation): functions handling entities, terrain and height changes (free of 0ad vocabulary):
2a. rmgen1-core: placers, painters (area shaping and texturing, entity assembly and placement, used by every map, still could be exchanged by a mod)
2b. rmgen1-collision (tileclasses to provide resource collisions. opt-in, as FeXoR prefers to avoid that)
2c. heightmap library (diamond square algorithm)

Layer 3 (Mod): functions often used by 0ad and non-fictional RTS (containing 0ad vocabulary, some alien mod might want to delete)
3a. terrain helpers (mountains, hills, rivers, ...)
3b. placer helpers (starting bases, wallbuilder, forests, mines, ...)
3c. rmgen2 (rmgen1 dialect)

Layer 4 (Application instance): random maps scripts
4a. Actual maps. They should ideally only provide some artistic numbers, reducing logic that can bug intrinsically
4b. Random mapgen libraries that are only relevant for very few maps. For instance the three Unknown maps (Phab:D252)

The effort to unify truckloads of rmgen duplication tracked in #4805 can be understood as part of this stack building process where logic is moved to the lower layers if it is not application or mod logic.

comment:4 Changed 2 years ago by elexis

In 20347:

Add rmgen Painter definitions, refs #4831.
Move the least complex rmgen painters (TileClassPainter?, TileClassPainter?, MultiPainter?, TerrainPainter?) to the top of the painters file, refs #4804.
Group the TerrainPainter? with the LayeredPainter? which both draw a texture on the terrain contrary to the others.
Use for-of loops and remove unneeded variables.
(Diff split for auditability.)

comment:5 Changed 2 years ago by elexis

In 20349:

Move ElevationPainter? and SmoothElevationPainter? constants above the SmoothElevationPainter?, refs #4804 and define them, refs #4831.
Remove two unneeded variables and reduce the performance by some milliseconds by using for...of loops, the destructing assignment and inlining the unified DX/DZ array.
Fix a wrong comment in the BorderTileClassConstraint? of rP20336.

Differential Revision: https://code.wildfiregames.com/D984
Not accepted by anyone

comment:6 Changed 2 years ago by elexis

In 20355:

Move SimpleGroup? and RandomGroup? from placers.js to a new file, because they aren't Placers, refs #4804.
Document what a Group is, refs #4831.
Remove duplicate place function by calling the SimpleGroup? from the RandomGroup?, refs #4805.
Move the default parameters from the function body to the function header, remove duplicate CELL_SIZE division, pointless comment and use let instead of var.

comment:7 Changed 2 years ago by elexis

In 20357:

Move SimpleObject? and RandomObject? from placers.js to a new file, because they aren't Placers, refs #4804.
Define what an Object is, refs #4831.
Remove RandomObject? duplication from rP9388 by just calling the SimpleObject? with a random template, refs #4805.

Remove the tautologous (!fail) check and replace needless multilevel fail-check nesting with two && operators.
Move default parameters from the function body to the function header and rename resultObjs to entities.
Throw errors instead of printing something if min > max.

comment:8 Changed 2 years ago by elexis

In 20360:

Rename placer.js to placer_centered.js and pathplacer.js to placer_noncentered.js, refs #4804.
Document the difference between the two types of classes, refs #4831.
Move RectPlacer? (as of r20185 unreferenced but possibly useful)to placer_noncentered.js and remove three redundant checks there.

comment:9 Changed 2 years ago by elexis

In 20361:

Move the HeightPlacer? from the Schwarzwald map (rP15327) to the placer library, refs #4804.
Remove the unneeded NullConstraint? check.

comment:10 Changed 2 years ago by elexis

In 20386:

rmgen cleanup.

Remove initTerrain from Map, because that class should only have getters and setters, refs #4804.
Unify it with the global initTerrain function.
Remove some redundant calls to that.

comment:11 Changed 2 years ago by elexis

In 20387:

rmgen cleanup.

Remove placeTerrain, and createObjectGroup from Map, because that class should only have getters and setters, refs #4804.
Unify it with the global functions of the same name.
Let Map.createArea only construct and register the Area object with the Map, but do the place and painter logic in the global function.

comment:12 Changed 2 years ago by elexis

In 20388:

rmgen cleanup.

Remove PointXZ class which is essentially a subset of Vector2D, refs #4834, #4804.
Remove Point3D which is unused and the same as Vector3D without the math features, refs #4805.

comment:13 Changed 2 years ago by elexis

In 20395:

Move checkIfIntersect and distanceOfPointFromLine from misc.js to math.js (because the latter should only contain terrain and entity placement utilities), refs #4804.
Delete unused getGradient and move getAngle from library.js to math.js.

comment:14 Changed 2 years ago by elexis

In 20406:

Delete the remains of rmgen/utilityfunctions.js, refs #4804 by
cleaning up the createForests function.

Remove g_numStragglerTrees and clForest references from the rmgen library.
Compute the number of forest- and straggler trees in a new function instead of hiding it and obstructing the placement code.
Remove unused numMultiplier argument.

Replace duplication of these three functions with calls to them, refs #4805.
Remove duplicate random biome JSON constants of the Islands, Migration and Snowflake Searocks map.
There is significant partial createForests duplication that should be unified eventually.

comment:15 Changed 2 years ago by elexis

In 20410:

Move landscape generation functions from misc.js to gaia_terrain.js, refs #4804.

Rename passageMaker to createShallowsPassage to avoid confusion with straightPassageMaker aka PassMaker? from Corsica & Sardinia and Pyrenean Sierra.
Remove TILE_CENTERED_HEIGHT_MAP operations in that function from rP12641 that don't do anything.
Add infinite loop protection to getTIPIADBON.
Remove unneeded parentheses, braces, redundancy, whitespace issues, use early return and merge consecutive if-statements with a logical or in these functions.

comment:16 Changed 2 years ago by elexis

In 20415:

Move starting player entity rmgen functions from misc.js to a new player.js, refs #4804.
Remove two duplicates of the starting entity placement code in the Fortress map of rP11361 and placeCivDefaultEntities of rP11754 by just making the distance an argument, refs #4805.

Equally to the starting cavalry, don't spawn special units like dogs or worker elephants on Fortress twice, but only the women and Infantry.
Consider the weird kwargs iberWalls argument of placeCivDefaultEntities from rP11754 deprecated and just make the orientation a regular optional argument.
Rename createStartingPlayerEntities to placeStartingEntities.

comment:17 Changed 2 years ago by elexis

In 20416:

Move rmgen player location functions from library.js to player.js, refs #4804.
Doesn't change the code besides abstracting distributePointsOnCircle and attempting to improve the documentation, refs #4831.

comment:18 Changed 2 years ago by elexis

In 20417:

Unify Caledonian Meadows and Wild Lake player location duplication from rP19704 / rP18781, refs #4805.
Move getOrderOfPointsForShortestClosePath from library.js to math.js, refs #4804 and name it sortPointsShortestCycle.

comment:19 Changed 2 years ago by elexis

In 20437:

Unify rmgen modifyTilesBasedOnHeight (rP20189) with the HeightPlacer? (rP20361).

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:20 Changed 2 years ago by elexis

In 20468:

Add rotateAround Vector2D function.
Remove the rmgen rotateCoordinates helper function from rP20295 and clean the implementation of that commit, refs #4845, #4804.
Deepfreeze mapCenter vector from rP20463 used here to prevent accidental overwrites with the mutating Vector2D functions, refs #4854.

comment:21 Changed 2 years ago by elexis

In 20528:

Delete GetCivData? from MapGeneratorWorker?.cpp which is redundant with the ReadJSONFile from rP20129.
Unify civ file loading from gui/common/functions_civinfo.js and rmgen/library.js in globalscripts/Templates.js.
Delete the two forgotton headers in rP20507.

Refs #4868, #4804, D900.
Differential Revision: https://code.wildfiregames.com/D1062
Discussed with: leper

comment:22 Changed 2 years ago by elexis

Milestone: Alpha 23Backlog
Owner: elexis deleted
Status: assignednew

comment:23 Changed 2 years ago by elexis

Milestone: BacklogAlpha 23

The plan in comment:3 is not finished yet, but calling it finished otherwise as the files themselves are cleaned for the most part.

comment:24 Changed 2 years ago by elexis

Owner: set to elexis
Resolution: fixed
Status: newclosed

In 20815:

Implement random map script playerbase function.
Unifies 54 variants of the rmgen playerbase code, fixes #4805.

Add retry loops to prevent collisions of starting resources, fixes #4600 and
resources placed outside of the map area, fixes #4796.

Lays the foundation to test for collisions with Iberian walls, refs #2192 and
allows to rearrange the starting resources on all random maps without being confronted with code.

Delete remains of misc.js, leaving the rmgen files sorted by logic, fixes #4804.
Concludes what was started in rP18816, rP19282, specifically the 82 rmgen commits starting rP20115, rP20301.
Uses vector algebra, refs #4845.
Removes many Math proxy calls, refs #4933.
Removes 35 unused elevation and 24 unused cliffRadius variables, demonstrating the copy&paste antipattern.
Reduce iberian-wall hardcoding to one line, refs #4940.

comment:25 Changed 2 years ago by elexis

In 20879:

Extract a RandomPathPlacer? prototype from Deep Forest (rP11444) and Schwarzwald (rP15327) duplication, refs #4805, #4804.

This allows creation of paths that are not linear nor sine-shaped like the PathPlacer?, refs #892.
To mimic the per-tile path elevation randomization on Deep Forest, use a SmoothElevationPainter? and it's randomization argument from rP20354.
Use vector algebra and the mapCenter getter, refs #4845, #4854.

comment:26 Changed 2 years ago by elexis

In 20932:

Move ExportMap? code from global helper to RandomMap? object, refs #4804.
Delete WATER_LEVEL_CHANGED global, refs #4964.

comment:27 Changed 2 years ago by elexis

In 20995:

Delete InitMap? and ExportMap? proxies and let the random map define the RandomMap? object, refs #4804.
Eventually there should be no direct reference to g_Map in the library, refs #4964.

comment:28 Changed 2 years ago by elexis

In 20999:

Move rmgen playerdata getters to the other player-related code, refs #4804.

comment:29 Changed 2 years ago by elexis

In 21025:

Stop identifying TileClasses? by a customly defined TileClassID, but just identify the prototype instance directly, refs #4804, rP9096.
Delete hence unneeded validClass, getTileClass, addToClass and removeFromClass.

Use vectors for the TileClass? add and remove function, refs #4992.
Use createArea call for addCivicCenterAreaToClass.

comment:30 Changed 2 years ago by elexis

In 21069:

rmgen random placement and Entity instantiation refactoring, fixes #4992.

ChainPlacer?, ClumpPlacer?, SimpleObject? receive the vectors that are in place everywhere already, refs #4845.
Add public setCenterPosition to CenteredPlacer? and Group rather than writing private properties of the prototypes.
ChainPlacer? and ClumpPlacer? simplifications, deduplication and renames, refs #4805.

Replace placeObject global with RandomMap? placeEntity members, refs #4804.
Split to placeEntityPassable / placeEntityAnywhere, as well as validTilePassable / validTileAnywhere to more cleanly distinguish actor and casual entity placement.
No more does SimpleObject? create Entity instances and register entityIDs if they are never placed.
Removes the map global reference from the Entity constructor, refs #4964.
By definition of what is passed to the engine, an Entity has an ID and position, so keep it impossible to create Entities without IDs.

Implement randomPositionOnTile so that there aren't different implementations thereof, including unintented ones as in ardennes_forest.js in rP21021.
On Caledonian Meadows, remove unused pathplacing code, to be superseded by #4368.
On Schwarzwald, delete unused startLocations following rP20864.
On Latium, replace complicated duplicated hardcoded fish location computation with a simple HeightConstraint?, refs #4960.

comment:31 Changed 2 years ago by elexis

In 21287:

Split the random map generation Placer prototypes into one file per prototype, refs #5042, #4804.
The flags should preserve revision history.

comment:32 Changed 2 years ago by elexis

In 21288:

Split random map generation Painter prototypes into one file per prototype, refs #5042, #4804.

comment:33 Changed 2 years ago by elexis

In r21289:

Separate object-oriented random map generation core library "rmgen" from miscellaneous helper procedures which are moved to a new library "rmgen-common", refs #4804, #5042.
This way mods can use the placement and painting prototypes while excluding any common 0ad code.

comment:34 Changed 2 years ago by elexis

In 21290:

Use CamelCase for random map files containing the prototype of the same name, refs #5042, #4804.

Note: See TracTickets for help on using tickets.