Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#4831 closed defect (fixed)

rmgen JSdoc

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

Description

For many functions and classes of rmgen library it is not clear what they do and how they differ from similar classes.

On the other side there are comments that don't really inform the reader and some noisy formatting comments.

While at it, all comments should be written in the same format, i.e. JSdoc.

Refs #4804 #4805

Change History (16)

comment:1 by elexis, 6 years ago

In 20336:

Define rmgen Constraints, refs #4831.
Use an array function instead of a loop.

comment:2 by elexis, 6 years ago

In 20337:

Define rmgen Entity class, refs #4831.
Make playerID non-optional.
Move default orientation code to the function header.
Add missing quotes.

comment:3 by elexis, 6 years ago

In 20338:

Remove abstract rmgen Terrain class that was inherited by SimpleTerrain and RandomTerrain.
It didn't add a benefit, hid a bug (see rP20335), required the inheriting classes to use terrainObjects, added confusing indirection and none of the other classes (painters, placers, constraints) use that inheritance.
Define what the Terrain classes do, refs #4831.
Rename treeType to templateName.
Remove unneeded round in the validT condition.

comment:4 by elexis, 6 years ago

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 by elexis, 6 years ago

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 by elexis, 6 years ago

In 20350:

Unify duplicate Breadth-First-Search traversing of the LayeredPainter and SmoothElevationPainter in a new helper function, refs #4805, making it much more comprehensible what each part does.
Document the two classes, the new function and every step of their execution, refs #4831.
Unify SmoothElevationPainter height computation duplication, remove pointless comments, use array functions and the prefix increment operator.

comment:7 by elexis, 6 years ago

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:8 by elexis, 6 years ago

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:9 by elexis, 6 years ago

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:10 by elexis, 6 years ago

In 20371:

Document the rmgen Map class, refs #4831
(excluding the height handling which needs revision and functions that should be relocated).

comment:11 by elexis, 6 years ago

In 20400:

Use @file JSDoc tag for the rmgen library, so that these comments are distinguished from block tags about globals, refs #4831.
Cleanup Area class.

comment:12 by elexis, 6 years ago

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:13 by elexis, 6 years ago

Milestone: BacklogAlpha 23
Resolution: fixed
Status: newclosed

Some files don't have good comments yet, but calling this fixed in a23 as most of the work was done in this release.

comment:14 by elexis, 6 years ago

In 21080:

Implement getBoundingBox and getPointsInBoundingBox rmgen helper.

Removes the according duplication in placers and makes the loops over these points onedimensional, refs #4805.
JSdoc for centric placers, refs #4831.

comment:15 by elexis, 6 years ago

Summary: rmgen JS docrmgen JSdoc

comment:16 by elexis, 6 years ago

In 21081:

Rename some variables of the pathplacer and use JSdoc, refs #892, #4831.

Note: See TracTickets for help on using tickets.