Changeset View
Standalone View
binaries/data/mods/public/maps/random/rmgen/placer/centered/DiskPlacer.js
| /** | /** | ||||
| * Returns all points on a disk at the given location that meet the constraint. | * Returns all points on a disk at the given location that meet the constraint. | ||||
| */ | */ | ||||
| function DiskPlacer(radius, centerPosition = undefined) | function DiskPlacer(radius, centerPosition = undefined) | ||||
| { | { | ||||
| this.radiusSquared = Math.square(radius); | this.radiusSquared = Math.square(radius); | ||||
| this.radius = radius; | |||||
elexis: (perhaps its cleaner to make radiusSquared a local, so that one couldnt externally modify only… | |||||
Done Inline ActionsI'd prefer to keep radiusSquared as it is to keep this part of original implementation at the moment even though I'm sure the performance benefits are negligible. I have no strong feelings either way however. badosu: I'd prefer to keep `radiusSquared` as it is to keep this part of original implementation at the… | |||||
| this.centerPosition = undefined; | this.centerPosition = undefined; | ||||
| if (centerPosition) | if (centerPosition) | ||||
| this.setCenterPosition(centerPosition); | this.setCenterPosition(centerPosition); | ||||
| } | } | ||||
| DiskPlacer.prototype.setCenterPosition = function(position) | DiskPlacer.prototype.setCenterPosition = function(position) | ||||
| { | { | ||||
| this.centerPosition = deepfreeze(position.clone().round()); | this.centerPosition = deepfreeze(position.clone().round()); | ||||
| }; | }; | ||||
| DiskPlacer.prototype.place = function(constraint) | DiskPlacer.prototype.place = function(constraint) | ||||
| { | { | ||||
| let points = []; | let points = []; | ||||
| for (let x = 0; x < g_Map.getSize(); ++x) | const xMin = Math.max(0, this.centerPosition.x - this.radius); | ||||
| for (let y = 0; y < g_Map.getSize(); ++y) | const yMin = Math.max(0, this.centerPosition.y - this.radius); | ||||
| const xMax = Math.min(g_Map.getSize(), this.centerPosition.x + this.radius); | |||||
Done Inline ActionsMap size edge is not present on Area due to 0-indexing: g_Map.getSize() - 1 avoids that badosu: Map size edge is not present on Area due to 0-indexing: `g_Map.getSize() - 1` avoids that | |||||
| const yMax = Math.min(g_Map.getSize(), this.centerPosition.y + this.radius); | |||||
| let it = new Vector2D(); | |||||
| for (it.x = xMin; it.x <= xMax; ++it.x) | |||||
| for (it.y = yMin; it.y <= yMax; ++it.y) | |||||
Done Inline ActionsExtend rectangle containing disk by 1 tile on each dimension when radius is floating point badosu: Extend rectangle containing disk by 1 tile on each dimension when radius is floating point | |||||
| { | { | ||||
| let point = new Vector2D(x, y); | if (g_Map.validTile(it) && this.centerPosition.distanceToSquared(it) <= this.radiusSquared && constraint.allows(it)) | ||||
| if (this.centerPosition.distanceToSquared(point) <= this.radiusSquared && constraint.allows(point)) | points.push(it.clone()); | ||||
Done Inline ActionsStan: For Information https://stackoverflow.com/questions/28725693/how-clone-has-more-performance… | |||||
Done Inline Actions(Wrong programming language, 0ads clone() is in ScriptInterface.cpp, and Vector2D.clone in Vector2D) elexis: (Wrong programming language, 0ads `clone()` is in ScriptInterface.cpp, and Vector2D.clone in… | |||||
| points.push(point); | |||||
| } | } | ||||
Done Inline ActionsI'm using validTile where in other places inMapBounds is used instead. The reason I made this choice is that inMapBounds does not check for circular maps, this might be an issue with that function or a misunderstanding on my part. badosu: I'm using `validTile` where in other places `inMapBounds` is used instead. The reason I made… | |||||
Done Inline Actions@elexis Pertaining your comment on checking map boundaries, it seems it's necessary due to Area cache requiring the point to be present on the map, see: https://github.com/0ad/0ad/blob/master/binaries/data/mods/public/maps/random/rmgen/Area.js#L11-L13 badosu: @elexis Pertaining your comment on checking map boundaries, it seems it's necessary due to… | |||||
Done Inline ActionsA actually allow now points outside the map disk range, the boundary checking is irrelevant for square maps (due to the iterator) and for circular maps we can expect the map designer to be explicit on the constraints, especially since we are not changing the expected behavior compared to the current implementation. badosu: A actually allow now points outside the map disk range, the boundary checking is irrelevant for… | |||||
| return points; | return points; | ||||
| }; | }; | ||||
(perhaps its cleaner to make radiusSquared a local, so that one couldnt externally modify only one of the two members. Then again it saves performance in case place was called multiple times on a different center, which thus seems preferable (performance benefit over fault protection, so ok)