Skip to content

Commit ef677f3

Browse files
author
DavidQ
committed
Extract high-confidence duplicate helpers into shared utility modules - PR_26139_030-shared-utils-duplicate-method-extraction
1 parent 57d6f43 commit ef677f3

16 files changed

Lines changed: 113 additions & 104 deletions

File tree

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# PR_26139_030 Shared Utils Duplicate Method Extraction Report
2+
3+
## Scope
4+
5+
- Read `docs/dev/PROJECT_INSTRUCTIONS.md` and `tmp/dups.txt`.
6+
- Treated `tmp/dups.txt` as broad duplicate-input signal only; ignored vendor, bundled, Playwright/test-runner, generated, and unrelated tool noise.
7+
- Limited extraction to high-confidence helpers whose behavior matched current callers.
8+
9+
## Changes
10+
11+
- Added shared geometry helpers in `src/shared/utils/geometryUtils.js`:
12+
- `normalizePoints`
13+
- `centerPoints`
14+
- `maxRadius`
15+
- Added exact JSON-clone helper `deepClone` in `src/shared/utils/jsonUtils.js`.
16+
- Preserved existing `cloneJson` nullish behavior by delegating non-nullish values to `deepClone`.
17+
- Reused existing `asArray` from `src/shared/utils/arrayUtils.js`.
18+
- Reused existing `isRecord` / `isPlainObject` shared guards where local semantics matched.
19+
- Replaced duplicate local helpers in:
20+
- Asteroids ship, bullet, UFO, ship debris, and asteroid profile geometry modules.
21+
- Asteroids manifest object geometry loader.
22+
- Shared Object Vector collision path.
23+
- Collision Inspector V2 constants.
24+
- Object Vector Studio V2 app/schema service.
25+
26+
## Deliberate Non-Changes
27+
28+
- Did not merge `src/engine/collision/objectVector.js` local `normalizePoints`; it preserves non-finite point entries via fallback conversion, while the extracted Asteroids helper filters non-finite points.
29+
- Did not normalize broad `sanitizeText`, `normalizeText`, `clamp`, or `toFiniteNumber` duplicates because `tmp/dups.txt` includes many unrelated or semantically different active/tool/vendor definitions.
30+
- Did not touch deprecated Vector Map Editor, Playwright internals, CodeMirror/bundled code, generated artifacts, or test-runner code.
31+
32+
## Validation
33+
34+
- PASS `npm run build:manifest`
35+
- PASS `node tests/games/AsteroidsValidation.test.mjs`
36+
- PASS `node tests/games/AsteroidsPresentation.test.mjs`
37+
- PASS `node tests/tools/ObjectVectorStudioV2DeleteCleanup.test.mjs`
38+
- PASS `npx playwright test tests/playwright/tools/AsteroidsGameSceneCleanup.spec.mjs --project=playwright --workers=1 --reporter=list`
39+
- PASS `npx playwright test tests/playwright/tools/ObjectVectorStudioV2FirstClassToolStarter.spec.mjs --project=playwright --workers=1 --reporter=list`
40+
- PASS `npx playwright test tests/playwright/tools/CollisionInspectorV2.spec.mjs --project=playwright --workers=1 --reporter=list`
41+
- PASS `npx playwright test tests/playwright/tools/AsteroidsGameSceneCleanup.spec.mjs tests/playwright/tools/ObjectVectorStudioV2FirstClassToolStarter.spec.mjs tests/playwright/tools/CollisionInspectorV2.spec.mjs --project=playwright --workers=1 --reporter=list`
42+
- PASS vendor/generated modification check: no `vendor`, bundled, minified, generated, Playwright-report, or test-results paths were modified.
43+
- Advisory Playwright V8 coverage refreshed in `docs/dev/reports/playwright_v8_coverage_report.txt` and `docs/dev/reports/coverage_changed_js_guardrail.txt`.
44+
45+
## Notes
46+
47+
- `npm run build:manifest` generated `docs/build/sample-manifest.json`; it was removed after validation because it is a generated build artifact outside this PR scope.
48+
- Full samples smoke test was skipped; this PR is a scoped helper extraction and requested targeted Asteroids/Object Vector/Collision Inspector validation instead.

dupes_called.txt

Whitespace-only changes.

games/Asteroids/entities/Bullet.js

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,7 @@ Bullet.js
66
*/
77
import { wrap } from '../utils/math.js';
88
import { transformCollisionPoints } from '../../../src/engine/collision/index.js';
9-
10-
function normalizePoints(points) {
11-
return Array.isArray(points)
12-
? points.map((point) => ({
13-
x: Number(point?.x ?? 0),
14-
y: Number(point?.y ?? 0),
15-
})).filter((point) => Number.isFinite(point.x) && Number.isFinite(point.y))
16-
: [];
17-
}
9+
import { normalizePoints } from '../../../src/shared/utils/geometryUtils.js';
1810

1911
export default class Bullet {
2012
constructor(x, y, vx, vy, life = 1.1, { angle = 0, collisionPoints = [] } = {}) {

games/Asteroids/entities/Ship.js

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,7 @@ Ship.js
66
*/
77
import { wrap } from '../utils/math.js';
88
import { transformCollisionPoints } from '../../../src/engine/collision/index.js';
9-
10-
function normalizePoints(points) {
11-
return Array.isArray(points)
12-
? points.map((point) => ({
13-
x: Number(point?.x ?? 0),
14-
y: Number(point?.y ?? 0),
15-
})).filter((point) => Number.isFinite(point.x) && Number.isFinite(point.y))
16-
: [];
17-
}
9+
import { normalizePoints } from '../../../src/shared/utils/geometryUtils.js';
1810

1911
export default class Ship {
2012
constructor(x, y, { collisionPoints = [] } = {}) {

games/Asteroids/entities/Ufo.js

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ Ufo.js
66
*/
77
import Bullet from './Bullet.js';
88
import { distance } from '../../../src/shared/utils/mathUtils.js';
9+
import { normalizePoints } from '../../../src/shared/utils/geometryUtils.js';
910
import { transformCollisionPoints } from '../../../src/engine/collision/index.js';
1011
import { randomRange } from '../utils/math.js';
1112

@@ -26,15 +27,6 @@ const UFO_PROFILES = {
2627
},
2728
};
2829

29-
function normalizePoints(points) {
30-
return Array.isArray(points)
31-
? points.map((point) => ({
32-
x: Number(point?.x ?? 0),
33-
y: Number(point?.y ?? 0),
34-
})).filter((point) => Number.isFinite(point.x) && Number.isFinite(point.y))
35-
: [];
36-
}
37-
3830
export default class Ufo {
3931
constructor(bounds, type = 'large', level = 1, rng = Math.random, { bulletCollisionPoints = [], collisionPoints = [] } = {}) {
4032
this.bounds = bounds;

games/Asteroids/game/asteroidObjectGeometry.js

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,38 +2,15 @@ import {
22
ASTEROIDS_ASTEROID_SIZE_OBJECT_IDS,
33
} from './asteroidsObjectGeometryManifest.js';
44
import { getObjectVectorCollisionOutlinePoints } from '../../../src/engine/collision/index.js';
5+
import { asArray } from '../../../src/shared/utils/arrayUtils.js';
6+
import { centerPoints, maxRadius } from '../../../src/shared/utils/geometryUtils.js';
57

68
const ASTEROID_SIZE_LABELS = Object.freeze({
79
1: 'SML',
810
2: 'MED',
911
3: 'LRG',
1012
});
1113

12-
function asArray(value) {
13-
return Array.isArray(value) ? value : [];
14-
}
15-
16-
function centerPoints(points) {
17-
if (!points.length) {
18-
return [];
19-
}
20-
const xs = points.map(({ x }) => x);
21-
const ys = points.map(({ y }) => y);
22-
const centerX = (Math.min(...xs) + Math.max(...xs)) / 2;
23-
const centerY = (Math.min(...ys) + Math.max(...ys)) / 2;
24-
return points.map(({ x, y }) => ({
25-
x: x - centerX,
26-
y: y - centerY,
27-
}));
28-
}
29-
30-
function maxRadius(points) {
31-
if (!points.length) {
32-
return 0;
33-
}
34-
return Math.max(...points.map(({ x, y }) => Math.sqrt(x * x + y * y)));
35-
}
36-
3714
function logProfileFailure(logger, message, details = {}) {
3815
if (!logger) {
3916
return;

games/Asteroids/game/asteroidsObjectGeometryManifest.js

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { getObjectVectorCollisionOutlinePoints } from '../../../src/engine/collision/index.js';
2+
import { isRecord } from '../../../src/shared/types/typeGuards.js';
3+
import { deepClone as clone } from '../../../src/shared/utils/jsonUtils.js';
24

35
const ASTEROIDS_OBJECT_VECTOR_TOOL_KEY = 'object-vector-studio-v2';
46
const OBJECT_VECTOR_PAYLOAD_KEYS = new Set(['version', 'toolId', 'name', 'objects']);
@@ -34,14 +36,6 @@ export const ASTEROIDS_UFO_TYPE_OBJECT_IDS = Object.freeze({
3436
small: ASTEROIDS_OBJECT_GEOMETRY_IDS.ufoSmall,
3537
});
3638

37-
function isRecord(value) {
38-
return Boolean(value) && typeof value === 'object' && !Array.isArray(value);
39-
}
40-
41-
function clone(value) {
42-
return JSON.parse(JSON.stringify(value));
43-
}
44-
4539
function normalizeString(value) {
4640
return String(value || '').trim();
4741
}

games/Asteroids/systems/ShipDebrisSystem.js

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,7 @@ David Quesenberry
55
ShipDebrisSystem.js
66
*/
77
import { randomRange } from '../utils/math.js';
8-
9-
function normalizePoints(points) {
10-
return Array.isArray(points)
11-
? points.map((point) => ({
12-
x: Number(point?.x ?? 0),
13-
y: Number(point?.y ?? 0),
14-
})).filter((point) => Number.isFinite(point.x) && Number.isFinite(point.y))
15-
: [];
16-
}
8+
import { normalizePoints } from '../../../src/shared/utils/geometryUtils.js';
179

1810
function createShipSegments(points) {
1911
const normalized = normalizePoints(points);

scripts/PS/find-duplicate-methods/dupes_called.ps1

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,22 @@
1010
# Get the root path (3 levels up)
1111
$rootPath = Resolve-Path "$PSScriptRoot\..\..\..\"
1212

13-
Get-ChildItem -Path $rootPath -Recurse -Filter *.js |
14-
Select-String -Pattern "function\s+[a-zA-Z0-9_]*\(", "[a-zA-Z0-9_]*\s*=\s*function", "[a-zA-Z0-9_]*:\s*function" |
15-
Group-Object Line |
16-
Where-Object { $_.Count -gt 1 } |
13+
Get-ChildItem -Path $rootPath -Recurse -Filter *.js -File |
14+
Where-Object {
15+
$_.FullName -notmatch '\\node_modules\\'
16+
} |
17+
Select-String `
18+
-Pattern "function\s+[a-zA-Z0-9_]*\(", "[a-zA-Z0-9_]*\s*=\s*function", "[a-zA-Z0-9_]*:\s*function" |
19+
Group-Object Line |
20+
Where-Object { $_.Count -gt 1 } |
1721
ForEach-Object {
1822
"($($_.Count)) Duplicate line: $($_.Name.Trim())"
19-
23+
2024
$_.Group | ForEach-Object {
2125
# Replace the absolute root path with a single backslash
2226
$relativePath = $_.Path -replace [regex]::Escape($rootPath), ""
2327
" -> Line $($_.LineNumber): \$relativePath"
2428
}
25-
""
26-
}
2729

30+
""
31+
}

src/engine/collision/objectVector.js

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import {
1313
normalizeObjectVectorTransform,
1414
transformRuntimeOrientedPoints,
1515
} from '../rendering/OrientationTransform.js';
16+
import { isRecord } from '../../shared/types/typeGuards.js';
17+
import { deepClone as clone } from '../../shared/utils/jsonUtils.js';
1618

1719
export const OBJECT_VECTOR_COLLISION_ENGINE_PATH = 'src/engine/collision/objectVector.js';
1820
export const OBJECT_VECTOR_COLLISION_MODES = Object.freeze(['bounds', 'vector', 'pixel-sprite', 'hybrid']);
@@ -30,14 +32,6 @@ const MODE_ALIASES = Object.freeze({
3032
const POLYGON_SAMPLE_COUNT = 28;
3133
const DEFAULT_MASK_CELL_SIZE = 4;
3234

33-
function isRecord(value) {
34-
return Boolean(value) && typeof value === 'object' && !Array.isArray(value);
35-
}
36-
37-
function clone(value) {
38-
return JSON.parse(JSON.stringify(value));
39-
}
40-
4135
function numberValue(value, fallback = 0) {
4236
const parsed = Number(value);
4337
return Number.isNaN(parsed) || Math.abs(parsed) === Infinity ? fallback : parsed;

0 commit comments

Comments
 (0)