Refactoring round 7: split BehaviorSystem, fewer copies (#85)
Recurring cleanup round over the code since round 6 (056a622), 105
commits. Behaviour is unchanged. Duplicated code in src went from 29
clones with 229 lines (0.32 %) to 14 clones with 109 lines (0.15 %),
measured with jscpd and the same settings as round 6.
- BehaviorSystem.ts shrinks from 941 to 589 lines. Collecting the
pulses (keys, timers, collisions, animations) moved to
behaviorPulses.ts. The NodeContext the nodes receive moved to
behaviorContext.ts. Imports from outside stay the same, TimeScaled
is still exported from BehaviorSystem.ts.
- PointKindButton and drawPointGrip replace the "Point:" button with
its menu and the point and handle drawing that the curve and the
vector art tab each had on their own.
- lerpVec, cubicAt and splitCubic in src/core/util/vec.ts replace four
copies of the cubic curve and two de Casteljau splits.
- mapNested in Properties.ts is the one walk over lists and structs
that liveObjects and objectRefs did separately. withFieldRenamed
and withoutField replace three copies of renaming a struct field
and two of removing one.
- jsonBody and staleRevision build the JSON body and turn a 409 into
StaleRevisionError for all stores. The auth store turns a 401 into
its error in one place.
- Smaller helpers: structAssetFrom for the two "Make struct type"
commands, shownObject in the inspector, selectedItem for the size
gizmo and the polygon corners, solidTiles in the tilemap collision,
requireVariablesAsset and requireBehaviorVariable.
- Left as is on purpose: the plain forwarding from the collision and
physics systems to Solids, the pointer wiring of the canvases and
the identical last lines of the two make commands.
- Removed unused code: tilemapTileIds and tilemapsUsing were never
called since #128 and #131. EditorState.runtimeProject became unused
with GameRun in #266. The assetId getter of the make commands was
read only by a test.
- 60 values used only in their own file lost their export. The shader
and particle node catalogues stay exported as in round 6.
- Five comments that told history now describe the current state. The
doc comment of BehaviorSystem sits on the class again.
- README lists the struct, clip area and curve assets and mentions the
game link from #266.
Findings:
- Deleting a tile that is painted on a tilemap leaves its cells in
place, and they draw nothing. The delete dialog does not mention the
tilemaps. That was probably the job of the unused tilemapsUsing.
Left as is, because fixing it changes behaviour.
- The solid filter for tile surfaces was untested. A bent filter
stayed green. A new test in tests/core/surface.test.ts now fails
against it.
Tests:
- ./check.sh: 195 files, 2815 tests, exit 0 (one test removed with
tilemapsUsing, one added for tile surfaces).
- Full e2e run: ALL PASSED, 1580 checks.
- Smokes jumprun, tetris, actor: 24, 19 and 21 checks, all passed.
The example project files were restored byte for byte afterwards.
- Counter-checks, 12, all fail as expected: 10 in unit tests (among
them mapNested, splitCubic, cubicAt, staleRevision, the stay pulse,
the late timer and the time scale), 2 in the browser (Point button
always enabled fails 47, selectedItem without a hit fails 29).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
dded62b9f9
commit
bc688f3efc
93 files changed
+914
-902
No files matched your search
@@ -1,5 +1,15 @@
|
||||
import { type AuthStore, roleOf, type SignedIn, WrongCredentialsError } from './AuthStore';
|
||||
import { apiRequest } from './http';
|
||||
import { apiRequest, jsonBody } from './http';
|
||||
|
||||
/**
|
||||
* A wrong password is the expected answer of a login screen, not a breakdown: it gets its own type
|
||||
* so the screen can say one thing about it and another about a server that is down. Changing the
|
||||
* password answers the same way when the current one was wrong; a 422 (too short) carries the
|
||||
* server's wording and stays generic.
|
||||
*/
|
||||
function wrongCredentials(status: number, data: unknown): Error | undefined {
|
||||
return status === 401 ? new WrongCredentialsError((data as { error?: string } | null)?.error) : undefined;
|
||||
}
|
||||
|
||||
export class HttpAuthStore implements AuthStore {
|
||||
constructor(private readonly endpoint: string) {}
|
||||
@@ -18,14 +28,8 @@ export class HttpAuthStore implements AuthStore {
|
||||
this.endpoint,
|
||||
'POST',
|
||||
'auth/login',
|
||||
{
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ username, password }),
|
||||
},
|
||||
// A wrong password is the expected answer of a login screen, not a breakdown: it gets its
|
||||
// own type so the screen can say one thing about it and another about a server that is down.
|
||||
(status, data) =>
|
||||
status === 401 ? new WrongCredentialsError((data as { error?: string } | null)?.error) : undefined,
|
||||
jsonBody({ username, password }),
|
||||
wrongCredentials,
|
||||
);
|
||||
return { user: body.user, role: roleOf(body.role) };
|
||||
}
|
||||
@@ -39,14 +43,8 @@ export class HttpAuthStore implements AuthStore {
|
||||
this.endpoint,
|
||||
'POST',
|
||||
'auth/password',
|
||||
{
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ currentPassword, newPassword }),
|
||||
},
|
||||
// Same split as `logIn`: a 401 here means the current password was wrong, which the dialog
|
||||
// says in its own words. A 422 (too short) carries the server's wording and stays generic.
|
||||
(status, data) =>
|
||||
status === 401 ? new WrongCredentialsError((data as { error?: string } | null)?.error) : undefined,
|
||||
jsonBody({ currentPassword, newPassword }),
|
||||
wrongCredentials,
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -1,11 +1,6 @@
|
||||
import type { Project } from '../model/Project';
|
||||
import { apiRequest } from './http';
|
||||
import {
|
||||
type LoadedProject,
|
||||
type ProjectStore,
|
||||
type ProjectSummary,
|
||||
StaleRevisionError,
|
||||
} from './ProjectStore';
|
||||
import { apiRequest, jsonBody } from './http';
|
||||
import { type LoadedProject, type ProjectStore, type ProjectSummary, staleRevision } from './ProjectStore';
|
||||
import { deserializeProject, serializeProject } from './serialize';
|
||||
|
||||
export class HttpProjectStore implements ProjectStore {
|
||||
@@ -47,18 +42,6 @@ export class HttpProjectStore implements ProjectStore {
|
||||
}
|
||||
|
||||
private async request<T>(method: string, route: string, payload?: unknown): Promise<T> {
|
||||
return apiRequest<T>(
|
||||
this.endpoint,
|
||||
method,
|
||||
route,
|
||||
{
|
||||
headers: payload !== undefined ? { 'Content-Type': 'application/json' } : {},
|
||||
body: payload !== undefined ? JSON.stringify(payload) : undefined,
|
||||
},
|
||||
(status, data) =>
|
||||
status === 409
|
||||
? new StaleRevisionError((data as { revision?: number } | null)?.revision ?? -1)
|
||||
: undefined,
|
||||
);
|
||||
return apiRequest<T>(this.endpoint, method, route, jsonBody(payload), staleRevision);
|
||||
}
|
||||
}
|
||||
@@ -1,5 +1,5 @@
|
||||
import { apiRequest } from './http';
|
||||
import { StaleRevisionError } from './ProjectStore';
|
||||
import { apiRequest, jsonBody } from './http';
|
||||
import { staleRevision } from './ProjectStore';
|
||||
import type { SnapshotStore, SnapshotSummary } from './SnapshotStore';
|
||||
|
||||
export class HttpSnapshotStore implements SnapshotStore {
|
||||
@@ -26,19 +26,7 @@ export class HttpSnapshotStore implements SnapshotStore {
|
||||
}
|
||||
|
||||
private request<T>(method: string, path: string, payload?: unknown): Promise<T> {
|
||||
return apiRequest<T>(
|
||||
this.endpoint,
|
||||
method,
|
||||
path,
|
||||
{
|
||||
headers: payload !== undefined ? { 'Content-Type': 'application/json' } : {},
|
||||
body: payload !== undefined ? JSON.stringify(payload) : undefined,
|
||||
},
|
||||
(status, data) =>
|
||||
status === 409
|
||||
? new StaleRevisionError((data as { revision?: number } | null)?.revision ?? -1)
|
||||
: undefined,
|
||||
);
|
||||
return apiRequest<T>(this.endpoint, method, path, jsonBody(payload), staleRevision);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { type Role, roleOf } from './AuthStore';
|
||||
import { apiRequest } from './http';
|
||||
import { apiRequest, jsonBody } from './http';
|
||||
import type { Account, AccountChange, UsersStore } from './UsersStore';
|
||||
|
||||
/** A refusal of the API (4xx with an `{error}`) as its bare sentence, without route and status around it. */
|
||||
@@ -17,18 +17,14 @@ export class HttpUsersStore implements UsersStore {
|
||||
}
|
||||
|
||||
async create(username: string, password: string, role: Role): Promise<void> {
|
||||
await apiRequest(this.endpoint, 'POST', 'users', this.json({ username, password, role }), refusal);
|
||||
await apiRequest(this.endpoint, 'POST', 'users', jsonBody({ username, password, role }), refusal);
|
||||
}
|
||||
|
||||
async update(username: string, change: AccountChange): Promise<void> {
|
||||
await apiRequest(this.endpoint, 'PATCH', `users/${username}`, this.json(change), refusal);
|
||||
await apiRequest(this.endpoint, 'PATCH', `users/${username}`, jsonBody(change), refusal);
|
||||
}
|
||||
|
||||
async remove(username: string): Promise<void> {
|
||||
await apiRequest<void>(this.endpoint, 'DELETE', `users/${username}`, {}, refusal);
|
||||
}
|
||||
|
||||
private json(body: object): { headers: Record<string, string>; body: string } {
|
||||
return { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body) };
|
||||
}
|
||||
}
|
||||
@@ -20,6 +20,13 @@ export class StaleRevisionError extends Error {
|
||||
}
|
||||
}
|
||||
|
||||
/** The API's 409, with the revision the server holds, as a `StaleRevisionError` (see `apiRequest`). */
|
||||
export function staleRevision(status: number, data: unknown): Error | undefined {
|
||||
return status === 409
|
||||
? new StaleRevisionError((data as { revision?: number } | null)?.revision ?? -1)
|
||||
: undefined;
|
||||
}
|
||||
|
||||
export interface ProjectStore {
|
||||
list(): Promise<ProjectSummary[]>;
|
||||
/**
|
||||
|
||||
@@ -27,7 +27,7 @@ export interface ReleaseStore {
|
||||
}
|
||||
|
||||
/** The page a released game is played on, next to the editor's own. */
|
||||
export const PLAY_PAGE = '/game-maker/play.html';
|
||||
const PLAY_PAGE = '/game-maker/play.html';
|
||||
|
||||
export function releaseUrl(origin: string, token: string): string {
|
||||
return `${origin}${PLAY_PAGE}?game=${token}`;
|
||||
|
||||
@@ -14,10 +14,17 @@ const STATUS_MESSAGES: Record<number, string> = {
|
||||
504: 'Server timed out',
|
||||
};
|
||||
|
||||
export function statusMessage(status: number, statusText: string): string {
|
||||
function statusMessage(status: number, statusText: string): string {
|
||||
return STATUS_MESSAGES[status] ?? (statusText || `HTTP ${status}`);
|
||||
}
|
||||
|
||||
/** The request part of a JSON payload; nothing at all without one. */
|
||||
export function jsonBody(payload?: unknown): Pick<RequestInit, 'headers' | 'body'> {
|
||||
return payload === undefined
|
||||
? {}
|
||||
: { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(payload) };
|
||||
}
|
||||
|
||||
/**
|
||||
* One call against the PHP API. Routing goes through the `r` query parameter because the site's
|
||||
* nginx config does not pass PATH_INFO to PHP; `query` adds further parameters beside it, which is
|
||||
|
||||
Reference in new issue
Block a user