Skip to content

Commit 66aa865

Browse files
author
DavidQ
committed
Continue Phase 3 shared utility duplicate cleanup - PR_26140_034-shared-utils-phase-3
1 parent 5328c36 commit 66aa865

59 files changed

Lines changed: 197 additions & 297 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
# PR_26140_034-shared-utils-phase-3 Report
2+
3+
## Scope
4+
- Continued Phase 3 shared utility cleanup from PR_26140_033.
5+
- Extracted only behavior-identical helpers and switched local duplicates to shared imports.
6+
- Did not change gameplay logic, schema behavior, manifest contracts, generated/vendor/archive files, or report snapshots.
7+
8+
## Implementation Summary
9+
- Added shared `normalizePathSeparators` to `src/shared/string/stringHelpers.js` and exported it through shared string/utils indexes.
10+
- Replaced behavior-identical local text helpers with shared `normalizeText` and `sanitizeText` imports across active game/tool/shared modules.
11+
- Replaced array-rejecting object helpers with shared `asObject`; replaced array-allowing pipeline helpers with shared `toObject` where semantics matched.
12+
- Replaced duplicated finite number helpers with shared `toFiniteNumber` or `asFiniteNumber` in runtime, games, and Collision Inspector V2.
13+
- Kept local helpers where semantics differ, including path helpers with slash-collapse/fallback differences and geometry point helpers with invalid-point/rounding differences.
14+
15+
## Duplicate Scanner Results
16+
Source: `tools/shared/powerShell/find_dupes_called.ps1`, refreshed to `tmp/dupes_called.txt`.
17+
18+
| Candidate | Before Phase 3 | After Phase 3 | Notes |
19+
| --- | ---: | ---: | --- |
20+
| `sanitizeText` | 38 | 8 | Remaining hits are samples and an old report snapshot, out of PR scope. |
21+
| `normalizeText` | 11 | 4 | Remaining active hits differ by lowercase semantics or deprecated tool scope; samples are out of scope. |
22+
| `normalizePath` | 4 | 2 | Remaining active helpers differ by fallback, trimming, or slash-collapse behavior. |
23+
| `asObject` | 8 | 2 | Remaining duplicate includes sample runtime plus shared utility. |
24+
| `toFiniteNumber` | 4 | 0 | Consolidated. |
25+
| `numberValue` | 2 | 0 | Consolidated active engine/tool numeric helpers; unrelated DOM-specific helper remains outside matched duplicate line. |
26+
| `normalizePoint` | 2 | 2 | Kept local because invalid point handling differs. |
27+
| `normalizePoints` | 2 | 2 | Kept local because filtering/rounding semantics differ. |
28+
| `toObject` | 2 | 2 | Remaining duplicate includes old report snapshot plus shared utility. |
29+
| `node_modules` paths | not found | not found | Scanner output stayed focused away from dependency tree. |
30+
31+
## Remaining Repo-Owned Candidates
32+
- `src/engine/runtime/fullscreenBezel.js` and `tools/preview-generator-v2/PreviewGeneratorV2RepoAccess.js` both contain `normalizePath`, but they do not have identical behavior.
33+
- `tools/shared/assetPipelineConverters.js` has a path normalizer built around `normalizeProjectRelativePath` and fallback behavior, so it was left local.
34+
- `tools/asset-manager-v2/js/assetPreviewHelpers.js` has `normalizeText` with lowercase semantics, so it was not merged into the trim-only helper.
35+
- `src/engine/collision/objectVector.js` and `tools/shared/vector/vectorAssetContract.js` keep separate `normalizePoint(s)` helpers because invalid point and precision behavior differ.
36+
- Sample-phase helpers and old report snapshots are intentionally out of scope for this PR.
37+
38+
## Validation
39+
- PASS: `npm run build:manifest`
40+
- PASS: `node tests/games/AsteroidsValidation.test.mjs`
41+
- PASS: `node tests/games/AsteroidsManifestScreenDimensions.test.mjs`
42+
- PASS: `node tests/games/AsteroidsPresentation.test.mjs`
43+
- PASS: `node tests/tools/ObjectVectorFinalRuntimeCleanup.test.mjs`
44+
- PASS: `node tests/tools/ObjectVectorStudioV2DeleteCleanup.test.mjs`
45+
- PASS: `npm run test:workspace-v2` (58 passed)
46+
- PASS: `npx playwright test tests/playwright/tools/ObjectVectorStudioV2FirstClassToolStarter.spec.mjs --project=playwright --workers=1 --reporter=list` (4 passed)
47+
- PASS: `npx playwright test tests/playwright/tools/CollisionInspectorV2.spec.mjs --project=playwright --workers=1 --reporter=list` (4 passed)
48+
- PASS: `npx playwright test tests/playwright/tools/AsteroidsGameSceneCleanup.spec.mjs --project=playwright --workers=1 --reporter=list` (1 passed)
49+
- PASS: `tools/shared/powerShell/find_dupes_called.ps1` rerun and summarized above.
50+
- PASS: `git diff --check` (only existing CRLF normalization warnings reported; no whitespace errors).
51+
52+
## Playwright Impact
53+
Playwright impacted: Yes. Shared runtime/tool JavaScript imports changed, so Workspace Manager V2, Object Vector Studio V2, Collision Inspector V2, and Asteroids gameplay smoke were validated.
54+
55+
## Coverage
56+
- Produced advisory Playwright V8 coverage artifacts at `docs/dev/reports/playwright_v8_coverage_report.txt` and `docs/dev/reports/coverage_changed_js_guardrail.txt`.
57+
- Missing changed runtime JS coverage is reported as WARN per project rules, not FAIL.
58+
59+
## Full Samples Smoke
60+
Skipped. This PR is a scoped utility extraction with targeted Workspace, Asteroids, Object Vector Studio V2, and Collision Inspector V2 validation; it does not broadly change the sample loader/framework.
61+
62+
## Manual Validation Notes
63+
1. Open Workspace Manager V2 and confirm repo selection still populates games/tools.
64+
2. Launch Object Vector Studio V2 and Collision Inspector V2 from Workspace Manager V2.
65+
3. Launch Asteroids and confirm normal gameplay render/collision behavior.
66+
4. Review `tmp/dupes_called.txt` for remaining out-of-scope duplicate candidates listed above.

games/SolarSystem/game/SolarSystemWorld.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ David Quesenberry
55
SolarSystemWorld.js
66
*/
77
import { toObject } from '/src/shared/utils/objectUtils.js';
8+
import { toFiniteNumber } from '/src/shared/number/index.js';
89

910
const MAX_STEP_SECONDS = 1 / 60;
1011

@@ -86,11 +87,6 @@ function clampIndex(value, max) {
8687
return Math.max(0, Math.min(max, value));
8788
}
8889

89-
function toFiniteNumber(value, fallback) {
90-
const numeric = Number(value);
91-
return Number.isFinite(numeric) ? numeric : fallback;
92-
}
93-
9490
function sanitizeSolarWorldSkin(skin) {
9591
const entities = toObject(skin?.entities);
9692
const sun = toObject(entities.sun);

games/bouncing-ball/game/BouncingBallWorld.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ BouncingBallWorld.js
66
*/
77
import { clamp } from '/src/shared/utils/mathUtils.js';
88
import { toObject } from '/src/shared/utils/objectUtils.js';
9+
import { toFiniteNumber } from '/src/shared/number/index.js';
910

1011
const MAX_STEP_SECONDS = 1 / 120;
1112
const DEFAULT_BOUNCING_BALL_WORLD_SKIN = Object.freeze({
@@ -18,11 +19,6 @@ const DEFAULT_BOUNCING_BALL_WORLD_SKIN = Object.freeze({
1819
}
1920
});
2021

21-
function toFiniteNumber(value, fallback) {
22-
const numeric = Number(value);
23-
return Number.isFinite(numeric) ? numeric : fallback;
24-
}
25-
2622
function sanitizeBouncingBallWorldSkin(skin) {
2723
const colors = toObject(skin?.colors);
2824
const sizing = toObject(skin?.sizing);

games/breakout/game/BreakoutWorld.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ BreakoutWorld.js
66
*/
77
import { clamp } from '/src/shared/utils/mathUtils.js';
88
import { toObject } from '/src/shared/utils/objectUtils.js';
9+
import { toFiniteNumber } from '/src/shared/number/index.js';
910

1011
const MAX_STEP_SECONDS = 1 / 120;
1112

@@ -34,11 +35,6 @@ const DEFAULT_BREAKOUT_SKIN = Object.freeze({
3435
}
3536
});
3637

37-
function toFiniteNumber(value, fallback) {
38-
const numeric = Number(value);
39-
return Number.isFinite(numeric) ? numeric : fallback;
40-
}
41-
4238
function sanitizeBreakoutWorldSkin(value) {
4339
const source = toObject(value);
4440
const colors = toObject(source.colors);

games/pong/game/PongScene.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import PongAudio from './PongAudio.js';
1010
import { getPongModes } from './PongModeConfig.js';
1111
import { wrapTextByCharacterCount } from '/src/shared/utils/index.js';
1212
import { toObject } from '/src/shared/utils/objectUtils.js';
13+
import { toFiniteNumber } from '/src/shared/number/index.js';
1314

1415
const COURT = {
1516
width: 960,
@@ -37,11 +38,6 @@ const DEFAULT_SIZING = {
3738
ballRadius: 8
3839
};
3940

40-
function toFiniteNumber(value, fallback) {
41-
const numeric = Number(value);
42-
return Number.isFinite(numeric) ? numeric : fallback;
43-
}
44-
4541
function sanitizePongSkin(skin) {
4642
const colors = toObject(skin?.colors);
4743
const sizing = toObject(skin?.sizing);

games/shared/gameManifestPreviewResolver.js

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,13 @@ import {
22
resolveManifestChromeAssetPaths,
33
resolveRuntimeAssetUrl
44
} from "/src/engine/runtime/gameImageConvention.js";
5-
6-
function normalizeText(value) {
7-
return typeof value === "string" ? value.trim() : "";
8-
}
9-
10-
function normalizePath(value) {
11-
return normalizeText(value).replace(/\\/g, "/");
12-
}
5+
import { normalizePathSeparators, normalizeText } from "../../src/shared/string/index.js";
6+
import { asArray } from "../../src/shared/utils/arrayUtils.js";
137

148
function hasProtocol(value) {
159
return /^[a-z][a-z0-9+.-]*:/i.test(value);
1610
}
1711

18-
function asArray(value) {
19-
return Array.isArray(value) ? value : [];
20-
}
21-
2212
function documentRefOrNull(documentRef) {
2313
return documentRef || globalThis.document || null;
2414
}
@@ -33,7 +23,7 @@ async function waitForDocumentReady(documentRef) {
3323
}
3424

3525
export function manifestPathFromGameHref(href) {
36-
const normalized = normalizePath(href);
26+
const normalized = normalizePathSeparators(href);
3727
if (!normalized || hasProtocol(normalized) || normalized.includes("..")) {
3828
return "";
3929
}
@@ -42,7 +32,7 @@ export function manifestPathFromGameHref(href) {
4232
}
4333

4434
export function manifestPathForGame(game) {
45-
const explicitPath = normalizePath(game?.manifestPath || game?.gameManifestPath);
35+
const explicitPath = normalizePathSeparators(game?.manifestPath || game?.gameManifestPath);
4636
if (explicitPath && !hasProtocol(explicitPath) && !explicitPath.startsWith("//") && !explicitPath.includes("..")) {
4737
return explicitPath.startsWith("/") ? explicitPath : `/${explicitPath}`;
4838
}
@@ -51,7 +41,7 @@ export function manifestPathForGame(game) {
5141

5242
export async function resolveGameManifestPreview(options = {}) {
5343
const documentRef = documentRefOrNull(options.documentRef);
54-
const manifestPath = normalizePath(options.manifestPath);
44+
const manifestPath = normalizePathSeparators(options.manifestPath);
5545
if (!manifestPath) {
5646
return {
5747
manifestPath: "",
@@ -65,7 +55,7 @@ export async function resolveGameManifestPreview(options = {}) {
6555
manifestPayload: options.manifestPayload,
6656
documentRef
6757
});
68-
const previewPath = normalizePath(resolved.previewPath);
58+
const previewPath = normalizePathSeparators(resolved.previewPath);
6959
return {
7060
manifestPath: resolved.manifestPath || manifestPath,
7161
previewPath,
@@ -112,7 +102,7 @@ export async function hydrateGameManifestPreviewImage(options = {}) {
112102
const gameId = normalizeText(options.gameId);
113103
const resolved = await resolveGameManifestPreview({
114104
gameId,
115-
manifestPath: normalizePath(options.manifestPath) || (gameId ? `/games/${gameId}/game.manifest.json` : ""),
105+
manifestPath: normalizePathSeparators(options.manifestPath) || (gameId ? `/games/${gameId}/game.manifest.json` : ""),
116106
documentRef
117107
});
118108
if (resolved.previewUrl) {

src/engine/collision/objectVector.js

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
} from '../rendering/OrientationTransform.js';
1616
import { isRecord } from '../../shared/types/typeGuards.js';
1717
import { deepClone as clone } from '../../shared/utils/jsonUtils.js';
18+
import { asFiniteNumber } from '../../shared/number/index.js';
1819

1920
export const OBJECT_VECTOR_COLLISION_ENGINE_PATH = 'src/engine/collision/objectVector.js';
2021
export const OBJECT_VECTOR_COLLISION_MODES = Object.freeze(['bounds', 'vector', 'pixel-sprite', 'hybrid']);
@@ -32,15 +33,10 @@ const MODE_ALIASES = Object.freeze({
3233
const POLYGON_SAMPLE_COUNT = 28;
3334
const DEFAULT_MASK_CELL_SIZE = 4;
3435

35-
function numberValue(value, fallback = 0) {
36-
const parsed = Number(value);
37-
return Number.isNaN(parsed) || Math.abs(parsed) === Infinity ? fallback : parsed;
38-
}
39-
4036
function normalizePoint(point) {
4137
return {
42-
x: numberValue(point?.x),
43-
y: numberValue(point?.y),
38+
x: asFiniteNumber(point?.x),
39+
y: asFiniteNumber(point?.y),
4440
};
4541
}
4642

@@ -63,12 +59,12 @@ function shapeTool(shape) {
6359

6460
function sortedShapes(object) {
6561
return [...(Array.isArray(object?.shapes) ? object.shapes : [])]
66-
.sort((left, right) => numberValue(left?.order) - numberValue(right?.order));
62+
.sort((left, right) => asFiniteNumber(left?.order) - asFiniteNumber(right?.order));
6763
}
6864

6965
function sortedFrames(state) {
7066
return [...(Array.isArray(state?.frames) ? state.frames : [])]
71-
.sort((left, right) => numberValue(left?.order) - numberValue(right?.order));
67+
.sort((left, right) => asFiniteNumber(left?.order) - asFiniteNumber(right?.order));
7268
}
7369

7470
function firstObjectFrame(object, preferredStateIds = ['active', 'idle']) {
@@ -154,31 +150,31 @@ function shapeLocalPolygons(shape) {
154150
const geometry = isRecord(shape?.geometry) ? shape.geometry : {};
155151
const tool = shapeTool(shape);
156152
if (tool === 'rectangle') {
157-
return [rectanglePoints(numberValue(geometry.x), numberValue(geometry.y), numberValue(geometry.width, 1), numberValue(geometry.height, 1))];
153+
return [rectanglePoints(asFiniteNumber(geometry.x), asFiniteNumber(geometry.y), asFiniteNumber(geometry.width, 1), asFiniteNumber(geometry.height, 1))];
158154
}
159155
if (tool === 'polygon') {
160156
const points = normalizePoints(geometry.points);
161157
return points.length >= 3 ? [points] : [];
162158
}
163159
if (tool === 'polyline') {
164160
const points = normalizePoints(geometry.points);
165-
const strokeWidth = Math.max(2, numberValue(shape?.style?.strokeWidth, 2));
161+
const strokeWidth = Math.max(2, asFiniteNumber(shape?.style?.strokeWidth, 2));
166162
return points.slice(1).map((point, index) => segmentPolygon(points[index], point, strokeWidth));
167163
}
168164
if (tool === 'line') {
169-
const strokeWidth = Math.max(2, numberValue(shape?.style?.strokeWidth, 2));
165+
const strokeWidth = Math.max(2, asFiniteNumber(shape?.style?.strokeWidth, 2));
170166
return [segmentPolygon(
171167
normalizePoint(geometry.point1),
172168
normalizePoint(geometry.point2),
173169
strokeWidth
174170
)];
175171
}
176172
if (tool === 'circle') {
177-
const radius = Math.max(1, numberValue(geometry.r, 1));
178-
return [ellipsePoints(numberValue(geometry.cx), numberValue(geometry.cy), radius, radius)];
173+
const radius = Math.max(1, asFiniteNumber(geometry.r, 1));
174+
return [ellipsePoints(asFiniteNumber(geometry.cx), asFiniteNumber(geometry.cy), radius, radius)];
179175
}
180176
if (tool === 'ellipse') {
181-
return [ellipsePoints(numberValue(geometry.cx), numberValue(geometry.cy), Math.max(1, numberValue(geometry.rx, 1)), Math.max(1, numberValue(geometry.ry, 1)))];
177+
return [ellipsePoints(asFiniteNumber(geometry.cx), asFiniteNumber(geometry.cy), Math.max(1, asFiniteNumber(geometry.rx, 1)), Math.max(1, asFiniteNumber(geometry.ry, 1)))];
182178
}
183179
return [];
184180
}
@@ -285,7 +281,7 @@ export function createObjectVectorCollisionGeometry(object, instance = {}, optio
285281
.filter((polygon) => polygon.length >= 3);
286282
});
287283
const bounds = boundsFromPolygons(polygons);
288-
const maskCellSize = Math.max(1, numberValue(options.maskCellSize, DEFAULT_MASK_CELL_SIZE));
284+
const maskCellSize = Math.max(1, asFiniteNumber(options.maskCellSize, DEFAULT_MASK_CELL_SIZE));
289285
return {
290286
bounds,
291287
hasGeometry: polygons.length > 0,

src/engine/debug/network/shared/hostReadUtils.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
function asObject(value) {
2-
return value !== null && typeof value === "object" && !Array.isArray(value)
3-
? value
4-
: {};
5-
}
1+
import { asObject } from "../../../../shared/utils/objectUtils.js";
62

73
export function readHostStatus(host) {
84
return host && typeof host.getStatus === "function"

src/engine/debug/panels/PromotionGatePanel.js

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,7 @@ PromotionGatePanel.js
66
*/
77

88
import { drawPanel } from '../DebugPanel.js';
9-
10-
function asObject(value) {
11-
return value && typeof value === 'object' && !Array.isArray(value)
12-
? value
13-
: {};
14-
}
9+
import { asObject } from '../../../shared/utils/objectUtils.js';
1510

1611
function resolvePromotionStatus(source) {
1712
const snapshot = asObject(source);

src/engine/debug/standard/threeD/shared/threeDDebugUtils.js

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,9 @@ threeDDebugUtils.js
66
*/
77

88
import { sanitizeText } from "../../../../../shared/string/index.js";
9+
import { asObject } from "../../../../../shared/utils/objectUtils.js";
910

10-
export { sanitizeText };
11-
12-
export function asObject(value) {
13-
return value !== null && typeof value === "object" && !Array.isArray(value)
14-
? value
15-
: {};
16-
}
11+
export { asObject, sanitizeText };
1712

1813
export function asArray(value) {
1914
return Array.isArray(value) ? value : [];

0 commit comments

Comments
 (0)