-
-
Notifications
You must be signed in to change notification settings - Fork 909
alternative grid generators #832
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
3827a50
aaacd2d
b640f97
db49f1c
4291777
f4b3662
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,3 +2,4 @@ | |
| .vscode | ||
| .idea | ||
| .idea/Fantasy-Map-Generator.iml | ||
| node_modules | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1500,6 +1500,20 @@ | |
| </td> | ||
| </tr> | ||
|
|
||
| <tr data-tip="Select grid generation algorithm"> | ||
| <td></td> | ||
| <td>Grid algorithm</td> | ||
| <td> | ||
| <select id="gridAlgorithm"> | ||
| <option value="voronoiPoints" selected>Default</option> | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is the opposite, we should use inner names as they are, so it should be like 'jittered'
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't consider it a huge impact change, but if you think it's better... so be it. |
||
| <option value="hexPointsF">Hex flat</option> | ||
| <option value="hexPointsP">Hex pointy</option> | ||
| <option value="squarePoints">Squares</option> | ||
| </select> | ||
| </td> | ||
| <td></td> | ||
| </tr> | ||
|
|
||
| <tr data-tip="Define map name (will be used to name downloaded files)"> | ||
| <td> | ||
| <i data-locked="0" id="lock_mapName" class="icon-lock-open"></i> | ||
|
|
@@ -7781,7 +7795,7 @@ | |
| <script src="utils/commonUtils.js"></script> | ||
| <script src="utils/arrayUtils.js?v=02062022"></script> | ||
| <script src="utils/colorUtils.js"></script> | ||
| <script src="utils/graphUtils.js?v=02062022"></script> | ||
| <script src="utils/graphUtils.js?v=06192022"></script> | ||
| <script src="utils/nodeUtils.js"></script> | ||
| <script src="utils/numberUtils.js"></script> | ||
| <script src="utils/polyfills.js"></script> | ||
|
|
@@ -7805,7 +7819,7 @@ | |
| <script src="modules/military-generator.js"></script> | ||
| <script src="modules/markers-generator.js"></script> | ||
| <script src="modules/coa-generator.js"></script> | ||
| <script src="modules/submap.js?v=02062022"></script> | ||
| <script src="modules/submap.js?v=06192022"></script> | ||
| <script src="libs/polylabel.min.js"></script> | ||
| <script src="libs/lineclip.min.js"></script> | ||
| <script src="libs/alea.min.js"></script> | ||
|
|
@@ -7852,14 +7866,14 @@ | |
| <script defer src="modules/ui/emblems-editor.js"></script> | ||
| <script defer src="modules/ui/markers-editor.js"></script> | ||
| <script defer src="modules/ui/3d.js"></script> | ||
| <script defer src="modules/ui/submap.js?v=29052022"></script> | ||
| <script defer src="modules/ui/submap.js?v=06192022"></script> | ||
| <script defer src="modules/ui/hotkeys.js?v=17062022"></script> | ||
| <script defer src="modules/coa-renderer.js"></script> | ||
| <script defer src="libs/rgbquant.min.js"></script> | ||
| <script defer src="libs/jquery.ui.touch-punch.min.js"></script> | ||
|
|
||
| <script defer src="modules/io/save.js?v=29052022"></script> | ||
| <script defer src="modules/io/load.js?v=12062022"></script> | ||
| <script defer src="modules/io/save.js?v=06122022"></script> | ||
| <script defer src="modules/io/load.js?v=06122022"></script> | ||
| <script defer src="modules/io/cloud.js?v=04062022"></script> | ||
| <script defer src="modules/io/export.js?v=04062022"></script> | ||
| <script defer src="modules/io/formats.js"></script> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -631,6 +631,7 @@ void (function addDragToUpload() { | |
| }); | ||
| })(); | ||
|
|
||
| const gridOptimizationRequired = () => window[document.getElementById('gridAlgorithm').value] == voronoiPoints; | ||
| async function generate(options) { | ||
| try { | ||
| const timeStart = performance.now(); | ||
|
|
@@ -642,8 +643,10 @@ async function generate(options) { | |
|
|
||
| applyMapSize(); | ||
| randomizeOptions(); | ||
| const method = window[document.getElementById('gridAlgorithm').value]; | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why it's in window?
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see that function is linked, that's pretty hard to read
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it must be window. Because we are using global functions here. (Yes, we shouldn't, but if we start to refactor everything, we'd better off with a completely new project :-P, I've drawn a line here). The key can be be swapped to byId() if you like. I don't think it's hard to read :-o pretty straightforward.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe we just don't need this reference at all. And even if we need, we can use map object (dict)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why to add a map if we already have it? (the object).
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we need a reference to a function, it's better to create a map for it than reference window
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmmm... why would it be so? window is just an object. And functions are just objects (and methods are actually maps from String to Function). I agree on that we could use globalThis instead of window if we'd like to be super compatible, but it's irrelevant . Of course in an ideal case, these generators shouldn't be globals in the first place, and we could index the module object. It feels pretty clear and natural (to me at least), introducing a map for what we already have is less readable and redundant. The only thing which probably a bit ugly is that we can access any functions not just the generators, but this is the problem of the monolith architecture (globals). The proper way would be to namespace the generators under some module and use references from there. However I didn't wanted to rewrite everything, just do a small patch :) ok, I change it a bit...
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have functions and string options, but not the connection between them. So we will need something like map in any case, so it's better to create a map. Also it will work when functions are no longer be exposed to global scope
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. err... functions has a name. (see Function prototype: name). It's quite a connection. :) It would work in case of modules too. Let's see I'll do it, if it's not working we'll go for maps :-D |
||
| console.log('gird generation method', method); | ||
|
|
||
| if (shouldRegenerateGrid(grid)) grid = precreatedGraph || generateGrid(); | ||
| if (shouldRegenerateGrid(grid, method)) grid = precreatedGraph || generateGrid(method); | ||
| else delete grid.cells.h; | ||
| grid.cells.h = await HeightmapGenerator.generate(grid); | ||
|
|
||
|
|
@@ -1164,23 +1167,27 @@ function generatePrecipitation() { | |
| } | ||
|
|
||
| // recalculate Voronoi Graph to pack cells | ||
| // pars: optimize -> optimize cells structure, copy original graph otherwise. | ||
| function reGraph() { | ||
| TIME && console.time("reGraph"); | ||
| const {cells: gridCells, points, features} = grid; | ||
| const newCells = {p: [], g: [], h: []}; // store new data | ||
| const spacing2 = grid.spacing ** 2; | ||
| const optimize = gridOptimizationRequired() | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here it can be a simple check: const gridType = document.getElementById('gridType')?.value;
const optimizeWaterCells = gridType === "jiterred";
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's not exactly the same, and not a good design. String constants of the module (which is non-existent here of course :) shouldn't be used outside the module. One always should export an accessor (like gridOptimizationRequired) or define constants. Using functions instead of those "magic strings" has an additional advantage: your IDE can identify misspelling.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here I agree, but in this case
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Of course it shouldn't.There are quite a lot of stuff what shouldn't be there. :) It started as a small patch not a full blown refactoring project. That's the other branch. ^-^ |
||
|
|
||
| for (const i of gridCells.i) { | ||
| const height = gridCells.h[i]; | ||
| const type = gridCells.t[i]; | ||
| if (height < 20 && type !== -1 && type !== -2) continue; // exclude all deep ocean points | ||
| if (type === -2 && (i % 4 === 0 || features[gridCells.f[i]].type === "lake")) continue; // exclude non-coastal lake points | ||
| if (optimize) { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If optimization is not required, probably the whole rePack is not required as well
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure. you are absolutely right. But that pack object is super coupled with er... everything. I didn't dare to eliminate it. This is the safe bet. When we rewrite this part, this can be changed. If you are absolutely sure there will be no problem (you know the code much better than me) let's change it.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree, but we already can do a minor refactoring, leaving the resulting pack object as is. We can extract the logic for points repacking into a separate function and call it conditionally. If optimization is not required, then just return existing
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, that was my first intention, but I was lazy to do it. It would certainly save some bytes but memory is cheap even in case of 100k maps (SVG eats several orders of magnitude more ram).
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's more of a refactoring issue. Adding more options we make the code harder to understand, so if anything new is added, we need to make the old part simpler to not increase the entropy too much |
||
| if (height < 20 && type !== -1 && type !== -2) continue; // exclude all deep ocean points | ||
| if (type === -2 && (i % 4 === 0 || features[gridCells.f[i]].type === "lake")) continue; // exclude non-coastal lake points | ||
| } | ||
| const [x, y] = points[i]; | ||
|
|
||
| addNewPoint(i, x, y, height); | ||
|
|
||
| // add additional points for cells along coast | ||
| if (type === 1 || type === -1) { | ||
| if (optimize && (type === 1 || type === -1)) { | ||
| if (gridCells.b[i]) continue; // not for near-border cells | ||
| gridCells.c[i].forEach(function (e) { | ||
| if (i > e) return; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,7 +34,8 @@ function getMapData() { | |
| +hideLabels.checked, | ||
| stylePreset.value, | ||
| +rescaleLabels.checked, | ||
| urbanDensity | ||
| urbanDensity, | ||
| byId('gridAlgorithm').value, | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The value will be incorrect if user change gridAlgorithm, but doesn't generate new map. So
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct! I'll fix. |
||
| ].join("|"); | ||
| const coords = JSON.stringify(mapCoordinates); | ||
| const biomes = [biomesData.color, biomesData.habitability, biomesData.name].join("|"); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,29 +2,30 @@ | |
| // FMG utils related to graph | ||
|
|
||
| // check if new grid graph should be generated or we can use the existing one | ||
| function shouldRegenerateGrid(grid) { | ||
| function shouldRegenerateGrid(grid, method) { | ||
| const cellsDesired = +byId("pointsInput").dataset.cells; | ||
| if (cellsDesired !== grid.cellsDesired) return true; | ||
|
|
||
| const newSpacing = rn(Math.sqrt((graphWidth * graphHeight) / cellsDesired), 2); | ||
| const newCellsX = Math.floor((graphWidth + 0.5 * newSpacing - 1e-10) / newSpacing); | ||
| const newCellsY = Math.floor((graphHeight + 0.5 * newSpacing - 1e-10) / newSpacing); | ||
|
|
||
| return grid.spacing !== newSpacing || grid.cellsX !== newCellsX || grid.cellsY !== newCellsY; | ||
| return grid.generator !== method || grid.spacing !== newSpacing || grid.cellsX !== newCellsX || grid.cellsY !== newCellsY; | ||
| } | ||
|
|
||
| function generateGrid() { | ||
| function generateGrid(generator = voronoiPoints) { | ||
| Math.random = aleaPRNG(seed); // reset PRNG | ||
| const {spacing, cellsDesired, boundary, points, cellsX, cellsY} = placePoints(); | ||
| //const {spacing, cellsDesired, boundary, points, cellsX, cellsY} = placePoints(); | ||
| const {spacing, cellsDesired, boundary, points, cellsX, cellsY} = generator(); | ||
| const {cells, vertices} = calculateVoronoi(points, boundary); | ||
| return {spacing, cellsDesired, boundary, points, cellsX, cellsY, cells, vertices}; | ||
| return {spacing, cellsDesired, boundary, points, cellsX, cellsY, cells, vertices, generator}; | ||
| } | ||
|
|
||
| // place random points to calculate Voronoi diagram | ||
| function placePoints() { | ||
| function voronoiPoints() { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can reuse this function, mostly it's the same for all 3 sampling methods, can just pass different points generators |
||
| TIME && console.time("placePoints"); | ||
| const cellsDesired = +byId("pointsInput").dataset.cells; | ||
| const spacing = rn(Math.sqrt((graphWidth * graphHeight) / cellsDesired), 2); // spacing between points before jirrering | ||
| const spacing = rn(Math.sqrt((graphWidth * graphHeight) / cellsDesired), 2); // spacing between points before jittering | ||
|
|
||
| const boundary = getBoundaryPoints(graphWidth, graphHeight, spacing); | ||
| const points = getJitteredGrid(graphWidth, graphHeight, spacing); // points of jittered square grid | ||
|
|
@@ -35,6 +36,58 @@ function placePoints() { | |
| return {spacing, cellsDesired, boundary, points, cellsX, cellsY}; | ||
| } | ||
|
|
||
| // alternatively generate hex-grid | ||
| function hexPointsP() { return hexPoints(true) } | ||
| function hexPointsF() { return hexPoints(false) } | ||
| const hexRatio = Math.sqrt(3)/2; | ||
| function hexPoints(pointy) { // pointy must be 0 or 1 | ||
| TIME && console.time("hexPoints"); | ||
| const cellsDesired = +byId("pointsInput").dataset.cells; | ||
| let spacing = rn(Math.sqrt(graphWidth * graphHeight / hexRatio / cellsDesired), 2); // spacing between points | ||
| let xSpacing, ySpacing; | ||
| if (pointy) { | ||
| xSpacing = spacing; | ||
| ySpacing = spacing * hexRatio; | ||
| } else { | ||
| xSpacing = spacing * hexRatio * 2; | ||
| ySpacing = spacing / 2 ; | ||
| } | ||
|
|
||
| const boundary = getBoundaryPoints(graphWidth, graphHeight, spacing); | ||
| let points = []; | ||
|
|
||
| let rc, lc, x, y; | ||
| for (y = ySpacing / 2, lc = 0 ; y < graphHeight; y += ySpacing, lc++) { | ||
| for (x = lc % 2 ? 0 : xSpacing / 2, rc=0; x < graphWidth; x += xSpacing, rc++) { | ||
| points.push([x, y]); | ||
| } | ||
| } | ||
|
|
||
| TIME && console.timeEnd("hexPoints"); | ||
| return {spacing, cellsDesired, boundary, points, cellsX: rc, cellsY: lc}; | ||
| } | ||
|
|
||
| // square grid | ||
| function squarePoints() { | ||
| TIME && console.time("squarePoints"); | ||
| const cellsDesired = +byId("pointsInput").dataset.cells; | ||
| const spacing = rn(Math.sqrt((graphWidth * graphHeight) / cellsDesired), 2); | ||
|
|
||
| const boundary = getBoundaryPoints(graphWidth, graphHeight, spacing); | ||
| const cellsX = Math.floor((graphWidth + 0.5 * spacing - 1e-10) / spacing); | ||
| const cellsY = Math.floor((graphHeight + 0.5 * spacing - 1e-10) / spacing); | ||
|
|
||
| const radius = spacing / 2; | ||
|
|
||
| let points = []; | ||
| for (let y = radius; y < graphHeight; y += spacing) | ||
| for (let x = radius; x < graphWidth; x += spacing) | ||
| points.push([x, y]); | ||
|
|
||
| TIME && console.timeEnd("squarePoints"); | ||
| return {spacing, cellsDesired, boundary, points, cellsX, cellsY}; | ||
| } | ||
|
|
||
| // calculate Delaunay and then Voronoi diagram | ||
| function calculateVoronoi(points, boundary) { | ||
| TIME && console.time("calculateDelaunay"); | ||
|
|
@@ -96,7 +149,19 @@ function getJitteredGrid(width, height, spacing) { | |
|
|
||
| // return cell index on a regular square grid | ||
| function findGridCell(x, y, grid) { | ||
| return Math.floor(Math.min(y / grid.spacing, grid.cellsY - 1)) * grid.cellsX + Math.floor(Math.min(x / grid.spacing, grid.cellsX - 1)); | ||
| let xSpacing = grid.spacing; | ||
| let ySpacing = grid.spacing * Math.sqrt(3) / 2; | ||
| const maxindex = grid.cells.i.length; // safety belt | ||
| switch (grid.generator) { | ||
| case voronoiPoints: | ||
| case squarePoints: | ||
| return Math.floor(Math.min(y / grid.spacing, grid.cellsY - 1)) * grid.cellsX + Math.floor(Math.min(x / grid.spacing, grid.cellsX - 1)); | ||
| case hexPointsF: | ||
| xSpacing = grid.spacing * hexRatio; | ||
| ySpacing = grid.spacing / 2; | ||
| case hexPointsP: | ||
| return Math.min(Math.floor(Math.min(y / ySpacing + 1e-10, grid.cellsY - 1)) * grid.cellsX + Math.floor(Math.min(x / xSpacing + 1e-10, grid.cellsX - 1)), maxindex); | ||
| } | ||
| } | ||
|
|
||
| // return array of cell indexes in radius on a regular square grid | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Algorithm is too technical, it can be just 'Grid type'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok...