diff --git a/.fallowrc.json b/.fallowrc.json new file mode 100644 index 00000000..0d478b56 --- /dev/null +++ b/.fallowrc.json @@ -0,0 +1,11 @@ +{ + "entry": [ + "source/cloudflare/worker.mjs", + "source/engine/main-browser.ts", + "source/game/**/main.ts" + ], + "ignorePatterns": [ + ".fallow/**", + "dist/**" + ] +} diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index b95c08a7..842322bc 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -63,6 +63,10 @@ eventBus.subscribe("registry.frozen", () => { - **No `any`, `unknown`, or `*`**. Use specific types (e.g. `ArrayBuffer`). - **No `@returns {void}`**. +### Helper Functions +- **Prefer function declarations** for helper functions when lexical `this`, expression semantics, or very local inline usage are not needed. +- **Use `const foo = (...) => ...`** only when arrow-function behavior is actually relevant. + ## Specific Patterns to Observe - **Module System**: ES Modules exclusively (`type: "module"` in package.json). diff --git a/.github/instructions/build-and-deploy.instructions.md b/.github/instructions/build-and-deploy.instructions.md index 7b1e59ee..f10f5259 100644 --- a/.github/instructions/build-and-deploy.instructions.md +++ b/.github/instructions/build-and-deploy.instructions.md @@ -14,8 +14,12 @@ ### Three different kinds of builds -We need to make sure the correct build is used at all times. +All code is always compiled through esbuild (via Vite) before execution. This ensures full TypeScript feature support (decorators, const enum, etc.) across every environment. -1. Dedicated development server: The code is executed in node.js directly, no build step required. -2. Dedicated production server: The code is built using `npm run build:production` and the output files in `dist/` are executed in node.js. This is required to strip out all console.assert statements and other development-only code for optimal performance. -3. Client code: The code is built using `npm run build:production` and the output files in `dist/` are executed in the browser. +1. Dedicated development server: Build with `npm run dedicated:dev` (Vite watch mode), run with `npm run dedicated:start`. The build step compiles TypeScript to JavaScript in `dist/dedicated/` with source maps enabled. +2. Dedicated production server: Build with `npm run dedicated:build:production` and run with `npm run dedicated:start`. Strips console.assert and other development-only code for optimal performance. +3. Client code: Build with `npm run build:production` and serve the output in `dist/browser/` to the browser. + +### Testing + +Tests use `tsx` (esbuild-based) as the Node.js loader, ensuring the same TypeScript compilation behavior as the Vite builds. Run with `npm test`. diff --git a/.github/instructions/code-style-guide.instructions.md b/.github/instructions/code-style-guide.instructions.md index f49f1642..28fa10f4 100644 --- a/.github/instructions/code-style-guide.instructions.md +++ b/.github/instructions/code-style-guide.instructions.md @@ -53,10 +53,10 @@ Use `eventBus` for **business logic events and lifecycle hooks**. ## File Organization -### No index.mjs Files +### No index.ts Files - **Avoid barrel exports**. Use direct imports instead. -- Example: Import `BrushModelRenderer` from `./renderer/BrushModelRenderer.mjs`, not `./renderer`. +- Example: Import `BrushModelRenderer` from `./renderer/BrushModelRenderer.ts`, not `./renderer`. ## General Style Guidelines @@ -64,6 +64,7 @@ Use `eventBus` for **business logic events and lifecycle hooks**. - **Use camelCase** for variables and functions, PascalCase for classes. - **Use descriptive names** for variables and functions. - **Keep functions small** and focused on a single task or a single responsibility. +- **Prefer function declarations** for helper functions when arrow-function semantics are not needed. - **Use early returns** to reduce nesting and improve readability. - **Avoid deep nesting**; refactor into helper functions if necessary. - **Never mutate function parameters**; create new variables instead. @@ -153,13 +154,15 @@ class GL { ### `null` initializations -- **Explicitly initialize variables to `null`** when they will later hold an object reference and provide JSDoc type annotations either as cast or an inline comment. - - Example: `let model = /** @type {BaseModel} */ (null);` +- **Explicitly initialize variables to `null`** when they will later hold an object reference. + - In `.ts` files: `let model: BaseModel | null = null;` + - In `.mjs` files: `let model = /** @type {BaseModel} */ (null);` ### Empty Arrays -- **Initialize empty arrays with `[]`** instead of `new Array()` and provide JSDoc type annotations. - - Example: `let vertices = /** @type {number[]} */ ([]);` +- **Initialize empty arrays with `[]`** instead of `new Array()`. + - In `.ts` files: `const vertices: number[] = [];` + - In `.mjs` files: `let vertices = /** @type {number[]} */ ([]);` ## Class and Interface Design @@ -172,6 +175,7 @@ class GL { - **Use `_` prefix** for protected methods. Add `@protected` JSDoc tag. - **Use `#` prefix** for private methods. +- In `.ts` files, prefer native `protected` / `private` keywords. See `typescript-port.instructions.md`. ### Respect boundaries of abstraction @@ -184,7 +188,7 @@ class GL { - `model` (or `clmodel`) instead of `m`. - `entity` instead of `ent` or `e`. - **Constants:** UPPER_CASE. -- **Files:** PascalCase for classes (`BrushModelRenderer.mjs`), camelCase for utils (`modelUtils.mjs`). Always `.mjs`. +- **Files:** PascalCase for classes (`BrushModelRenderer.ts`), camelCase for utils (`modelUtils.ts`). Always `.ts` for TypeScript source files. ## Comments diff --git a/.github/instructions/game-logic-port.instructions.md b/.github/instructions/game-logic-port.instructions.md new file mode 100644 index 00000000..dd8512c3 --- /dev/null +++ b/.github/instructions/game-logic-port.instructions.md @@ -0,0 +1,640 @@ +## Game Logic TypeScript Port Guide + +This document covers porting the `source/game/` entity system from `.mjs` to `.ts`. It builds on the general [typescript-port.instructions.md](typescript-port.instructions.md) and addresses game-specific patterns: field serialization, the state machine, entity components, and Defs enums. + +All rules from the general TS porting guide still apply. This document only adds game-logic-specific guidance. + +--- + +### 1. Serializable Fields — Use `@entity` / `@serializable` Decorators + +The current JS pattern snapshots `Object.keys()` before and after field assignment to discover which properties to serialize: + +```javascript +// ❌ Current JS pattern +_declareFields() { + super._declareFields(); + this._serializer.startFields(); + this.health = 100; + this.enemy = null; + this._serializer.endFields(); + this._damageHandler = new DamageHandler(this); +} +``` + +In TypeScript, use the `@entity` class decorator and `@serializable` field decorator from `MiscHelpers.ts`. The `@serializable` decorator marks individual fields, and `@entity` on the class flushes them into a frozen `static serializableFields` array at class definition time — fully compatible with the existing `collectSerializableFields()` prototype-chain walk. + +```typescript +// ✅ TypeScript pattern — decorators +import { entity, serializable, Serializer } from '../helper/MiscHelpers.ts'; + +@entity +class BaseMonster extends BaseEntity { + @serializable health = 100; + @serializable enemy: BaseEntity | null = null; + @serializable pausetime = 0; + + // Components are regular class fields — not decorated, not serialized. + protected readonly _damageHandler = new DamageHandler(this); +} +``` + +The legacy `static readonly serializableFields` array pattern still works but is no longer recommended for new TypeScript classes. Legacy `.mjs` callers using `startFields()`/`endFields()` continue to work during the migration. + +#### How it works + +1. `@serializable` field decorators accumulate field names in a module-level array during class definition. +2. `@entity` class decorator freezes those names into a `static serializableFields` property. +3. `collectSerializableFields()` in `MiscHelpers.ts` walks the constructor chain bottom-up and merges every class's `serializableFields` array. Parent fields are included automatically. + +```typescript +// BaseEntity uses @entity + @serializable for its fields: +@entity +class BaseEntity { + @serializable ltime = 0.0; + @serializable origin = new Vector(); + // ... +} + +// BaseMonster adds its own: +@entity +class BaseMonster extends BaseEntity { + @serializable health = 100; + @serializable enemy: BaseEntity | null = null; +} + +// ArmySoldierMonster adds its own: +@entity +class ArmySoldierMonster extends BaseMonster { + @serializable _aiState = 'idle'; +} + +// At runtime the Serializer sees all three merged: ['ltime', 'origin', ..., 'health', ..., '_aiState'] +``` + +#### `_declareFields()` removal checklist + +| Before (JS) | After (TS) | +|---|---| +| Override `_declareFields()`, call `super._declareFields()` | Delete the method entirely | +| `this._serializer.startFields()` / `endFields()` | Delete both calls | +| Field assignments inside the boundary | Move to typed class field declarations with `@serializable` | +| Component creation after `endFields()` | Regular class fields (no `@serializable`) | +| Manual `static readonly serializableFields = [...]` | Delete — `@entity` + `@serializable` handles this | +| `this._serializer = new Serializer(this, this.engine)` in constructor | Keep as-is; Serializer reads `serializableFields` from the chain | + +The legacy `startFields()`/`endFields()` API remains in `Serializer` for `.mjs` callers during the migration. + +--- + +### 2. `Object.seal(this)` — Replace with TypeScript Strictness + +`BaseEntity` currently calls `Object.seal(this)` after `_declareFields()` to prevent accidental property additions at runtime. In TypeScript this is no longer necessary: + +- `noUncheckedIndexedAccess` and strict property checking catch typos at compile time. +- TS class fields define the shape exhaustively. + +**Remove `Object.seal(this)` from base entity constructors.** If runtime sealing is still wanted for defense-in-depth against map data injection in `assignInitialData`, do it inside `assignInitialData` after construction — not in the constructor (where it forces the `_declareFields` ceremony). + +--- + +### 3. State Machine — Typed Animation Sequences + +The current system defines ~200 states per monster as individual `_defineState()` calls with string keys, string frame names, string next-state references, and untyped `function()` callbacks. Problems: + +- Typos in state names or frame names are silent. +- `function() { this._ai.stand(); }` callbacks lose `this` typing. +- Verbose: 8 identical calls for an 8-frame stand loop. + +#### 3a. Typed state key union + +Each entity class should declare a string literal union of its valid state names: + +```typescript +type SoldierState = + | 'army_stand1' | 'army_stand2' | 'army_stand3' | 'army_stand4' + | 'army_stand5' | 'army_stand6' | 'army_stand7' | 'army_stand8' + | 'army_run1' | 'army_run2' | 'army_run3' | 'army_run4' + // ... + | 'army_die1' | 'army_die2' | 'army_die3'; +``` + +The `_defineState` and `_runState` signatures become generic on the concrete state key: + +```typescript +// On BaseEntity: +protected static _defineState( + state: S, keyframe: string | number | null, nextState: S | null, handler?: (this: BaseEntity) => void, +): void; + +protected _runState(state?: S | null): boolean; +``` + +This catches typos at compile time. Generating the union type is straightforward — it is the set of first-argument strings across all `_defineState` calls. + +#### 3b. Animation sequence helper — `_defineSequence` + +`BaseEntity._defineSequence()` is already implemented and generates a numbered sequence of states from a prefix and frame array: + +```typescript +static _defineSequence( + prefix: string, + frames: readonly (string | number)[], + handler: ((this: T, frameIndex: number) => void) | null = null, + loop = true, +): void; +``` + +The generic `T` is inferred from the callback's `this` annotation, so subclass callbacks get full type safety without casts. + +States are named `${prefix}1`, `${prefix}2`, ... with the last frame looping back to `${prefix}1` by default. This collapses 8 stand-state calls into one: + +```typescript +// ❌ Before: 8 calls +this._defineState('army_stand1', 'stand1', 'army_stand2', function () { this._ai.stand(); }); +this._defineState('army_stand2', 'stand2', 'army_stand3', function () { this._ai.stand(); }); +// ... 6 more + +// ✅ After: 1 call +this._defineSequence('army_stand', + ['stand1','stand2','stand3','stand4','stand5','stand6','stand7','stand8'], + function () { this._ai.stand(); }); +``` + +For sequences where individual frames need different behavior (walk speeds, firing on a specific frame), use the `frameIndex` parameter or fall back to individual `_defineState` calls: + +```typescript +const walkSpeeds = [1,1,1,1,2,3,4,4,2,2,2,1,0,1,1,1,3,3,3,3,2,1,1,1]; +this._defineSequence('army_walk', + Array.from({length: 24}, (_, i) => `prowl_${i + 1}`), + function (frameIndex) { + if (frameIndex === 0) { this.idleSound(); } + this._ai.walk(walkSpeeds[frameIndex]); + }); +``` + +#### 3c. Model QC — Static parsing only + +The `_modelQC` string and `_parseModelData` pattern can stay mostly as-is, but: + +- Make `_modelData` typed: `static readonly _modelData: Readonly | null`. +- Consider moving the raw QC string into a separate `.qc` asset file loaded at build time (Vite raw import) so it doesn't bloat the TS source. Not mandatory, but cleaner for large QC definitions. + +#### 3d. Callbacks use the concrete entity `this` type + +`_defineState` and `_defineSequence` are generic on `T extends BaseEntity` — the `T` is inferred from the callback's `this` annotation. Annotate callbacks with the concrete entity type to get full type safety without casts: + +```typescript +this._defineSequence('f_stand', swimFrames, + function (this: FishMonsterEntity) { this._ai.stand(); }); + +this._defineState('f_death21', 'death21', null, + function (this: FishMonsterEntity) { this.solid = solid.SOLID_NOT; }); +``` + +The generic is erased at storage time (`handler as ScheduledThinkCallback`) so the untyped `_states` record remains compatible, while `_runState` dispatches via `.call(this)` on the real entity instance. + +--- + +### 4. Defs Enums — Port to Native TypeScript Enums + +Current `Defs.mjs` re-exports engine-side frozen objects and defines game-specific ones: + +```javascript +// ❌ Current JS +export const dead = Object.freeze({ DEAD_NO: 0, DEAD_DYING: 1, DEAD_DEAD: 2, DEAD_RESPAWNABLE: 3 }); +export const damage = Object.freeze({ DAMAGE_NO: 0, DAMAGE_YES: 1, DAMAGE_AIM: 2 }); +``` + +Port to TS enums per the general TS guide: + +```typescript +// ✅ TS enum +export enum Dead { + NO = 0, + DYING = 1, + DEAD = 2, + RESPAWNABLE = 3, +} + +export enum Damage { + NO = 0, + YES = 1, + AIM = 2, +} +``` + +#### Naming conventions + +| JS name | TS name | Members | +|---|---|---| +| `dead.DEAD_NO` | `Dead.NO` | Drop the redundant prefix | +| `damage.DAMAGE_YES` | `Damage.YES` | | +| `moveType.MOVETYPE_PUSH` | `MoveType.PUSH` | | +| `solid.SOLID_TRIGGER` | `Solid.TRIGGER` | | +| `items.IT_SHOTGUN` | `Item.SHOTGUN` | | +| `effect.EF_MUZZLEFLASH` | `Effect.MUZZLEFLASH` | | +| `state.STATE_TOP` | `PropState.TOP` | Rename `state` export to avoid conflict with `_stateCurrent` | + +#### Bit-flag enums + +For `items`, `flags`, `effects`, and other bitfields, use `const enum` only if no runtime iteration is needed. Otherwise use a regular `enum`. Continue using bitwise operators (`|`, `&`, `~`) — TS enums support this. + +```typescript +export enum Item { + AXE = 4096, + SHOTGUN = 1, + SUPER_SHOTGUN = 2, + // ... +} + +// Usage stays the same: +this.items |= Item.QUAD; +this.items &= ~(Item.ARMOR1 | Item.ARMOR2 | Item.ARMOR3); +``` + +#### Engine re-exports + +Values re-exported from `engine/Defs.ts` (`solid`, `moveType`, `flags`, etc.) should be imported directly from the engine module rather than re-exporting through `game/id1/Defs.ts`. If the game-side module needs to add game-specific values, extend via a new enum or union — don't shadow the engine enum. + +--- + +### 5. Entity Components — Typed Composition + +#### `EntityWrapper` base + +Port `EntityWrapper` to a generic TypeScript class: + +```typescript +abstract class EntityWrapper { + readonly #entity: WeakRef; + + constructor(entity: T) { + this.#entity = new WeakRef(entity); + } + + protected get _entity(): T { + return this.#entity.deref()!; + } + + protected get _game(): ServerGameAPI { + return this._entity.game; + } + + protected get _engine(): ServerEngineAPI { + return this._entity.engine; + } +} +``` + +#### `DamageHandler` and `DamageInflictor` + +These already extend `EntityWrapper`. In TS, parameterize on the entity type so that `this._entity` resolves without casts: + +```typescript +class DamageHandler extends EntityWrapper { + // this._entity is typed — no casts needed for health, thinkPain, etc. +} +``` + +#### `Sub` (mover helper) + +Port similarly. The nested `_moveData` / `_useData` plain objects should become typed interfaces: + +```typescript +interface MoveData { + finalOrigin: Vector | null; + finalAngle: Vector | null; + callback: (() => void) | null; + active: boolean; +} + +interface UseData { + callback: ((activator: BaseEntity) => void) | null; +} +``` + +If `Sub` needs serialization, its fields should use `@serializable` like everything else. + +#### `AI` component + +Same pattern — `EntityWrapper`. The AI methods (`stand`, `walk`, `run`, `face`, etc.) get proper signatures. + +--- + +### 6. Entity Base Class — Structural Cleanup + +#### Constructor simplification + +With `@serializable` decorators and TS class fields, the `BaseEntity` constructor shrinks significantly. Fields move to class-body declarations: + +```typescript +export default class BaseEntity { + static readonly classname: string | null = null; + static readonly clientEdictHandler: typeof BaseClientEdictHandler | null = null; + static readonly clientEntityFields: string[] = []; + + // Serialized core fields + @serializable ltime = 0.0; + @serializable origin = new Vector(); + @serializable oldorigin = new Vector(); + @serializable angles = new Vector(); + @serializable mins = new Vector(); + @serializable maxs = new Vector(); + @serializable absmin = new Vector(); + @serializable absmax = new Vector(); + @serializable size = new Vector(); + @serializable velocity = new Vector(); + @serializable avelocity = new Vector(); + @serializable movetype: MoveType = MoveType.NONE; + @serializable solid: Solid = Solid.NOT; + @serializable flags: number = 0; + // ... remaining fields + + // Non-serialized + readonly engine: ServerEngineAPI; + readonly game: ServerGameAPI; + + protected _sub: Sub | null = null; + protected _damageHandler: DamageHandler | null = null; + + constructor(edict: ServerEdict, gameAPI: ServerGameAPI) { + this.#edict = edict ? new WeakRef(edict) : null; + this.engine = gameAPI.engine; + this.game = gameAPI; + this._precache(); + } +} +``` + +This eliminates the ~120-line constructor body that currently assigns every field. + +#### `clientEntityFields` — Type-safe with `keyof` + +```typescript +static readonly clientEntityFields: readonly (keyof PlayerEntity)[] = [ + 'items', 'armortype', 'armorvalue', 'health', +]; +``` + +This catches typos and renames at compile time. + +#### `assignInitialData` — Input validation + +The current method uses `switch(true)` with `instanceof Vector` / `typeof === 'number'` to cast string values from map data. In TS, define: + +```typescript +interface EdictInitialData { + readonly [key: string]: string; +} +``` + +And use a type-guard or explicit parse map instead of the generic `instanceof` chain. Consider extracting a `parseFieldValue(currentValue: unknown, rawString: string): unknown` helper. + +--- + +### 7. `ScheduledThink` — Lightweight Typed Class + +```typescript +interface ScheduledThink { + nextThink: number; + callback: (this: BaseEntity) => void; + identifier: string | null; + isRequired: boolean; +} +``` + +There is no need for `ScheduledThink` to have its own `Serializer`. It should be a plain interface or a lightweight `class` that does not have its own serialization infrastructure. The parent entity's serializer already handles the `_scheduledThinks` array through `TYPE_ARRAY` → `TYPE_SERIALIZABLE` recursion. + +However — **reconsider whether `ScheduledThink` needs serialization at all**. The callbacks are `function() {}` closures that get serialized via `toString()` and deserialized via `new Function()`. This is: + +- A security concern (`new Function` is eval-adjacent). +- Fragile (arrow functions, closures over locals, minified code all break it). +- Unnecessary if schedule reconstruction happens on load (entities re-schedule their thinks in `spawn()` or state restoration). + +If thinks can be reconstructed from entity state after deserialization, drop function serialization entirely and make `ScheduledThink` a simple runtime-only structure. + +--- + +### 8. Entity Registration & Static Members + +#### `static classname` + +Use `static readonly override` in subclasses: + +```typescript +class ArmySoldierMonster extends WalkMonster { + static readonly classname = 'monster_army' as const; + // ... +} +``` + +The `as const` ensures the type is the literal `'monster_army'`, not `string`. + +#### Model statics + +```typescript +class ArmySoldierMonster extends WalkMonster { + static readonly classname = 'monster_army' as const; + protected static readonly _health = 30; + protected static readonly _size = [new Vector(-16, -16, -24), new Vector(16, 16, 40)] as const; + protected static readonly _modelDefault = 'progs/soldier.mdl'; + protected static readonly _modelHead = 'progs/h_guard.mdl'; +} +``` + +#### `static _states` typing + +```typescript +interface StateDefinition { + readonly keyframe: string | number | null; + readonly nextState: string | null; + readonly handler: ((this: BaseEntity) => void) | null; +} + +// On the class: +protected static _states: Readonly> | null = null; +``` + +--- + +### 9. Port Order + +Port in dependency order, bottom-up: + +1. **`Defs.mjs` → `Defs.ts`** — Enums first; everything depends on them. +2. **`helper/MiscHelpers.mjs` → `helper/MiscHelpers.ts`** — `Serializer`, `EntityWrapper`, decorator infrastructure. +3. **`BaseEntity.mjs` → `BaseEntity.ts`** — Core entity, decorators, state machine. +4. **`Subs.mjs` → `Subs.ts`** — `Sub`, `TriggerFieldEntity`, `DelayedThinkEntity`. +5. **`entity/Weapons.mjs` → `entity/Weapons.ts`** — `DamageHandler`, `DamageInflictor`, projectiles. +6. **`entity/Items.mjs` → `entity/Items.ts`** — Item hierarchy. +7. **`entity/props/` → `.ts`** — `BasePropEntity`, Doors, Platforms, Buttons. +8. **`entity/monster/BaseMonster.mjs` → `BaseMonster.ts`** — Monster base + Walk/Fly/Swim. +9. **Concrete monsters** — Soldier, Demon, Ogre, etc. (one at a time). +10. **`entity/Player.mjs` → `Player.ts`** — Largest entity, do last. +11. **`entity/Triggers.mjs`, `entity/Misc.mjs`, `entity/Worldspawn.mjs`**. +12. **`GameAPI.mjs` → `GameAPI.ts`**, `main.mjs → main.ts`. +13. **`hellwave/` game mod** — After `id1` is fully ported. + +At each step: port, add/update unit tests, run `eslint --fix`, verify `tsc --noEmit`. + +--- + +### 10. Unit Test Considerations + +- Serialization behavior must be regression-tested: create an entity, serialize, deserialize, assert field equality. The decorator-based approach changes the collection mechanism, so existing serialization round-trip tests need to keep passing. +- State machine tests: verify that `_runState` advances through the expected sequence and invokes handlers. Test with a small synthetic entity, not a full monster. +- `assignInitialData`: test that string-to-type coercion works for Vectors, numbers, and strings. Test that private fields and functions are rejected. +- Mock pattern: use `withMockRegistry` as described in the unit test instructions. Entity construction needs a mock `ServerEngineAPI` and `ServerGameAPI`. + +--- + +### 11. Migration Checklist Per File + +For each `.mjs` → `.ts` entity file: + +- [ ] Rename to `.ts`. +- [ ] Replace all JSDoc `@type`/`@param` type annotations with TS syntax. Keep description text. +- [ ] Replace `_declareFields()` + `startFields`/`endFields` with `@serializable` field decorators. +- [ ] Remove `Object.seal(this)` from constructors. +- [ ] Convert frozen-object enums to TS `enum`. +- [ ] Add access modifiers (`protected`, `readonly`, `#private`). +- [ ] Add `override` to all overridden methods. +- [ ] Type `_states` and state names. +- [ ] Collapse repetitive `_defineState` sequences with `_defineSequence` where appropriate. +- [ ] Replace `/** @typedef */` imports with `import type`. +- [ ] Verify all components (`DamageHandler`, `Sub`, `AI`) are typed via generic `EntityWrapper`. +- [ ] Run `eslint --fix` and `tsc --noEmit`. +- [ ] Add or update unit tests for serialization round-trip and key behaviors. +- [ ] Convert the old `.mjs` file to a one-line re-export shim: `export { default } from './File.ts';` +- [ ] Update internal TS imports to point to `.ts` directly (e.g. `GameAPI.ts`). + +--- + +### 12. Common Pitfalls When Porting Monster Entities + +These issues were encountered and resolved during the Fish.mjs → Fish.ts port. They apply to all monster/entity ports. + +#### 12a. Callback `this` parameter in `_defineSequence` / `_defineState` + +`_defineSequence` and `_defineState` are generic — `T` is inferred from the callback's `this` annotation. Always annotate callbacks with the concrete entity type for full type safety: + +```typescript +// ✅ Correct — T is inferred as FishMonsterEntity, full autocomplete on this +this._defineSequence('f_stand', swimFrames, + function (this: FishMonsterEntity) { this._ai.stand(); }); +``` + +The generic is erased when stored in `_states` (`handler as ScheduledThinkCallback`), which is safe because `_runState` always dispatches via `.call(this)` on the concrete entity instance. + +#### 12b. Engine edict interface properties cannot use `override` + +Properties like `netname`, `classname`, and other edict fields exist on the engine's `BaseEntity` **interface** (defined in `Edict.ts`), not on the game-side `BaseEntity` class. TypeScript `override` only applies to members declared in a parent class. Implementing an interface property is not an override. + +```typescript +// ❌ Compile error — netname is not in the class hierarchy +override get netname(): string { return 'a fish'; } + +// ✅ Correct — plain getter (implements the interface property) +get netname(): string { return 'a fish'; } +``` + +#### 12c. Vector uses indexed access, not named properties + +The `Vector` class uses `[0]`, `[1]`, `[2]` for component access — **not** `.x`, `.y`, `.z`. There is no `Vector.of()` static factory either. + +```typescript +// ❌ Wrong +Vector.of(-16, -16, -24) +vector.x + +// ✅ Correct +new Vector(-16, -16, -24) +vector[0] +``` + +#### 12d. Static `_size` must not use `as const` + +The parent class declares `_size` as `[Vector | null, Vector | null]` (a mutable tuple). Using `as const` on the child declaration creates a `readonly` tuple that is not assignable to the mutable parent type. + +```typescript +// ❌ Compile error — readonly tuple not assignable to mutable +static _size = [new Vector(-16, -16, -24), new Vector(16, 16, 24)] as const; + +// ✅ Correct — explicit mutable tuple type +static _size: [Vector, Vector] = [new Vector(-16, -16, -24), new Vector(16, 16, 24)]; +``` + +#### 12e. ESM circular dependency in tests + +Entity `.ts` files import from each other and from `GameAPI.ts`, creating circular module dependencies. In production the entry point (`GameAPI.ts`) evaluates first, establishing all bindings. In tests, importing a specific entity file directly may invert the evaluation order, causing TDZ errors. + +**Always `await import('GameAPI.ts')` before importing any entity class in tests:** + +```javascript +// In test file — must be top-level await +await import('../../source/game/id1/GameAPI.ts'); +const { default: FishMonsterEntity } = await import('../../source/game/id1/entity/monster/Fish.ts'); +``` + +#### 12f. `_initStates` must be called explicitly in tests + +The game registry calls `_initStates()` on each entity class during bootstrap. In unit tests without the full registry, call it manually before testing state machine properties: + +```javascript +FishMonsterEntity._initStates(); +``` + +#### 12g. The `.mjs` shim pattern + +After porting, convert the old `.mjs` file to a one-line re-export shim so that any remaining `.mjs` importers continue to work without modification: + +```javascript +// Fish.mjs — re-export shim +export { default } from './Fish.ts'; +``` + +Then update all **internal TypeScript imports** to point directly to `.ts`. The shim is only for external/legacy `.mjs` consumers. + +#### 12h. Reference port: Fish.ts + +`source/game/id1/entity/monster/Fish.ts` serves as the canonical example of a fully ported monster entity. It demonstrates: + +- `@entity` class decorator (no `@serializable` fields needed — Fish has none beyond inherited) +- `_defineSequence` to collapse repetitive state definitions +- Callback `this: FishMonsterEntity` with generic inference (no casts needed) +- Non-looping sequences (`loop = false` for death) +- Override of `_defineState` for terminal frames (attack→run, pain→run, death→null) +- `override` on methods, `protected` on `_newEntityAI` and `hasMeleeAttack` +- `private` on helper methods (`_fishMelee`) + +#### 12i. `SpawnEntity` — Generic Entity Spawning + +`ServerEngineAPI.SpawnEntity` accepts an optional generic type parameter `T` that narrows the returned edict's `.entity` to the expected entity class. Since TypeScript generics are erased at runtime, the generic is a caller-side assertion — always pair it with a `console.assert(… instanceof …)` to validate at runtime. + +```typescript +// ❌ Old pattern — manual `as` cast, no runtime check +const backpack = this.engine.SpawnEntity(BackpackEntity.classname, { + origin: this.origin.copy(), + regeneration_time: 0, +})?.entity as BackpackEntity | undefined; + +backpack?.toss(); + +// ✅ New pattern — generic + instanceof assert +const backpack = this.engine.SpawnEntity(BackpackEntity.classname, { + origin: this.origin.copy(), + regeneration_time: 0, +})?.entity!; + +console.assert(backpack instanceof BackpackEntity); + +backpack.toss(); +``` + +Key rules: + +- **Always pass the concrete entity class** as the generic argument: `SpawnEntity(…)`. +- **Always follow with `console.assert(result instanceof EntityClass)`** — the generic cannot be checked at runtime by the engine, so the call site must verify. +- **Use `!` (non-null assertion) on `.entity`** when the spawn is expected to succeed. If failure is a possibility, use `?.` and guard accordingly. +- **Never use `as EntityClass | undefined`** to narrow the result — prefer the generic parameter instead. diff --git a/.github/instructions/typescript-port.instructions.md b/.github/instructions/typescript-port.instructions.md new file mode 100644 index 00000000..eea202ee --- /dev/null +++ b/.github/instructions/typescript-port.instructions.md @@ -0,0 +1,485 @@ + +## TypeScript Porting Guide + +When porting `.mjs` files to `.ts` (or polishing an earlier verbatim JS→TS port), apply every applicable rule below. The goal is idiomatic, type-safe TypeScript that relies on the compiler rather than JSDoc for type information. + +Files have to end with an empty line. + +### Interfaces over Type Aliases + +- **Prefer `interface`** for object shapes. Only use `type` for unions, intersections, tuples, or mapped types. +- **Mark every field `readonly`** unless the field is genuinely mutated after construction. + +```typescript +// ✅ Good +interface Hull { + readonly clipnodes: Clipnode[]; + readonly planes: Plane[]; + readonly clip_mins: Vector; + readonly clip_maxs: Vector; +} + +// ❌ Bad — type alias for a plain object shape +type Hull = { + clipnodes: Clipnode[]; + planes: Plane[]; +}; +``` + +### Typed Declarations — No JSDoc Type Casts + +Replace every `/** @type {X} */` cast with a native TS annotation or `as` cast. + +```typescript +// ❌ JSDoc cast +const materials = /** @type {Record} */ ({}); +loadmodel.version = /** @type {29|844124994} */ (dv.getUint32(0, true)); +const node = /** @type {Node} */ (stack.pop()); + +// ✅ TS annotation or `as` cast +const materials: Record = {}; +loadmodel.version = dv.getUint32(0, true) as 29 | 844124994; +const node = stack.pop()!; // when non-null is guaranteed +const node = stack.pop() as Node; // when a type narrowing is needed +``` + +Apply the same rule to local variables that used a JSDoc `@type` on the preceding line: + +```typescript +// ❌ JSDoc-typed local +/** @type {Map} */ +const leafsByType = new Map(); + +// ✅ TS generic +const leafsByType = new Map(); +``` + +### Method and Function Signatures + +- **Always provide explicit parameter types and return types** on every class method (public, protected, and private). +- For inner arrow / local functions, add parameter types; the return type may be inferred unless it improves clarity. + +```typescript +// ❌ Untyped method — leftover from JS +_loadVertexes(loadmodel, buf) { + +// ✅ Fully typed +_loadVertexes(loadmodel: BrushModel, buf: ArrayBuffer): void { +``` + +### JSDoc Cleanup + +When a method has TS parameter/return types, remove the redundant JSDoc `@param {Type}` and `@returns {Type}` annotations. **Keep the description text.** + +```typescript +// ❌ Redundant JSDoc types +/** + * Load vertices from BSP lump. + * @param {BrushModel} loadmodel - The model being loaded + * @param {ArrayBuffer} buf - The BSP file buffer + * @returns {void} + */ +_loadVertexes(loadmodel: BrushModel, buf: ArrayBuffer): void { + +// ✅ Description only +/** + * Load vertices from BSP lump. + * @protected + */ +_loadVertexes(loadmodel: BrushModel, buf: ArrayBuffer): void { +``` + +- **Never leave an empty JSDoc block** — always add a description sentence. +- **End JSDoc description sentences with a period.** +- **Keep `@protected` and `@private` tags** only during the transition period while the codebase still has `.mjs` callers that rely on them. Once all callers are `.ts`, prefer TS native access modifiers (see below). + +### Access Modifiers — `protected` / `private` + +Convert JSDoc access annotations to native TS modifiers: + +| JSDoc pattern | TS replacement | +|---|---| +| `@protected` + `_` prefix | `protected _methodName(…)` | +| `@private` + `_` prefix | `#methodName(…)` | +| `#methodName` (already private) | keep `#methodName(…)` | + +- **Use `protected`** for methods overridden by subclasses (e.g., `_loadFaces` in BSP29Loader → BSP2Loader). +- **Use `#` (hard private)** for methods that are truly internal and never accessed outside the class. +- **Keep the `_` prefix** on protected members for visual consistency during the migration. Once the full codebase is TS, the prefix may be dropped. + +### `override` Keyword + +Add `override` to every method that overrides a base class method. This catches accidental signature mismatches at compile time. + +```typescript +// ✅ Signals this overrides ModelLoader.getMagicNumbers() +override getMagicNumbers(): number[] { + return [29]; +} +``` + +### `readonly` and `static readonly` + +- Mark class fields and static fields `readonly` when they are assigned once (at declaration or in the constructor) and never reassigned. +- Applies to `static` lookup tables, frozen objects, configuration sets, etc. + +```typescript +static readonly #lump = Object.freeze({ entities: 0, planes: 1, /* … */ }); +static readonly doorClassnames = new Set(['func_door', 'func_door_secret']); +``` + +### Enums + +Port obvious enumeration-like patterns to native TS `enum` (or `const enum` for zero-runtime overhead when all consumers are TS): + +```typescript +// ❌ JS-era frozen object enum +const materialFlags = Object.freeze({ + MF_SKY: 1, + MF_TURBULENT: 2, + MF_TRANSPARENT: 4, +}); + +// ✅ TS enum +export enum MaterialFlags { + MF_SKY = 1, + MF_TURBULENT = 2, + MF_TRANSPARENT = 4, +} +``` + +**When to use `const enum`:** only when every consumer is TypeScript and no runtime object is needed (e.g., internal flags bitfields). Prefer a regular `enum` when the values may be iterated at runtime or exposed to `.mjs` callers. + +### Redundant Constructor Removal + +Remove empty constructors that only call `super()` with no additional logic — TypeScript (and JavaScript) does this implicitly. + +```typescript +// ❌ Redundant +constructor() { + super(); +} + +// ✅ Just omit it +``` + +### Avoid typeof to check existence of functions + +```typescript + +// ❌ Avoid this pattern +… +if (typeof someModule.someFunction === 'function') { +… +const attachedClient = typeof ent.getClient === 'function' ? ent.getClient() : null; +… + +// ✅ Instead, use optional chaining and nullish coalescing +if (sometype instanceof SomeClass) { +… +``` + +### Hot-Path Narrowing and API Contracts + +When TypeScript complains in render loops, BSP recursion, movement code, input dispatch, or other hot paths, do **not** introduce tiny helper functions merely to placate the type checker. + +- **Do not create helper functions whose only purpose is syntactic narrowing** such as `isFoo(...)`, `requireBar(...)`, `resolveBaz(...)`, or `getNodeChild(...)` when the call site already knows the invariant and is in a hot path. +- **Prefer local invariant checks** at the use site: + - `const worldmodel = CL.state.worldmodel!;` + - `console.assert(worldmodel !== null, 'worldmodel required');` + - then use the narrowed local directly. +- **Use small local `as` casts only after an adjacent `console.assert(...)` or branch that already proves the invariant.** Keep the cast at the use site instead of hiding it in another function. +- **Preserve existing runtime contracts.** Do not make required parameters optional just to quiet the compiler; fix every caller instead. +- **Avoid `Reflect.get`, `Reflect.set`, and other dynamic property access in hot paths.** If a property is part of the runtime contract, teach the type system about it with an interface or class member and use direct property access. +- **Avoid structural capability probes based on repeated reflective checks** like `typeof Reflect.get(entity, 'serialize') === 'function'` in gameplay code. Prefer a real runtime capability marker when possible. + +Example: + +```typescript +// ❌ Avoid helper indirection for a known hot-path invariant. +function getNodeChild(node: Node, childIndex: 0 | 1): Node { + const child = node.children[childIndex]; + console.assert(child instanceof Node, 'linked child required'); + return child as Node; +} + +BrushTrace._recursiveHullCheck(ctx, getNodeChild(node, 0), p1f, p2f, p1, p2, depth + 1); + +// ✅ Narrow locally where the value is consumed. +const frontChild = node.children[0] as Node; +console.assert(frontChild instanceof Node, 'linked child required'); +BrushTrace._recursiveHullCheck(ctx, frontChild, p1f, p2f, p1, p2, depth + 1); +``` + +For runtime capabilities shared with still-JS game code, prefer a dedicated runtime marker over repeated structural probes. A good pattern is a small abstract class with `Symbol.hasInstance` so engine code can use `instanceof` while JS implementations remain compatible. + +```typescript +abstract class SerializableEntity { + static [Symbol.hasInstance](value: unknown): boolean { + if (value === null || typeof value !== 'object') { + return false; + } + + const candidate = value as { + readonly classname?: unknown; + readonly serialize?: unknown; + readonly deserialize?: unknown; + }; + + return typeof candidate.classname === 'string' + && typeof candidate.serialize === 'function' + && typeof candidate.deserialize === 'function'; + } +} +``` + +### Template Literals + +Replace string concatenation with template literals for readability. + +```typescript +// ❌ Concatenation +throw new Error('Bad lump size in ' + loadmodel.name); + +// ✅ Template literal +throw new Error(`Bad lump size in ${loadmodel.name}`); +``` + +### Null Initialization and Empty Arrays + +When porting `null`-initialized or empty-array variables, use TS annotations directly instead of JSDoc casts: + +```typescript +// ❌ JSDoc +let model = /** @type {BaseModel} */ (null); +let vertices = /** @type {number[]} */ ([]); + +// ✅ TS +let model: BaseModel | null = null; +const vertices: number[] = []; +``` + +### Checklist (per file) + +Use this checklist when polishing a ported `.ts` file: + +1. [ ] All `/** @type {X} */` casts → TS annotations or `as` casts. +2. [ ] All method parameters and return types explicitly typed. +3. [ ] JSDoc `@param {Type}` / `@returns {Type}` removed (descriptions kept). +4. [ ] `interface` + `readonly` for all object shape types. +5. [ ] `override` on every overriding method. +6. [ ] `static readonly` on immutable class fields. +7. [ ] `protected` / `private` / `#` replacing JSDoc `@protected` / `@private`. +8. [ ] Obvious enums ported to TS `enum`. +9. [ ] Redundant empty constructors removed. +10. [ ] String concatenation → template literals. +11. [ ] No empty JSDoc blocks — every block has a description ending with a period. +12. [ ] ESLint clean (`npx eslint `). +13. [ ] All tests pass (`npm run test`). +14. [ ] All original comments preserved, especially TODOs and complex logic explanations. +15. [ ] File ends with an empty line. +16. [ ] If there is some important logic that is not covered by tests yet, add tests for it. +17. [ ] No helper functions were introduced solely for TypeScript narrowing in hot paths. +18. [ ] No hot-path reflective property access was introduced where a typed field or runtime capability marker would do. +19. [ ] Existing method signatures were preserved unless there was an intentional API change. + +### Avoid inline import type annotations + +When importing types, prefer file-level imports over inline `import('…').Type` annotations for better readability and maintainability. + +```typescript + +// ❌ Inline import type +static _brushMayAffectTrace(ctx: BrushTraceContext, brush: import('./model/BSP.ts').Brush): boolean { +… + +// ✅ File-level import +import { Brush } from './model/BSP.ts'; +… +static _brushMayAffectTrace(ctx: BrushTraceContext, brush: Brush): boolean { +… +``` + +Exception: When used in dynamically to load modules like so + +```typescript +const comModule = await import(/* @vite-ignore */ serverComId); +const COM = comModule.COM as typeof import('../common/COM.ts'); +``` + +But this is a special case and should not be used as a general pattern for type imports! + +### Porting over comments + +Make sure to **always** carry over comments from the original `.mjs` file, especially those that explain complex logic or important context. + +However, **do not carry over comments that only describe types** (e.g., "Bounding radius for culling") since the TS types should be self-explanatory. Instead, add a JSDoc comment with a description if needed. + +For example, consider this original code snippet with helpful comments: + +```javascript +… + + // Calculate bounding box + this._calculateBounds(loadmodel); + + // Generate tangents and bitangents for normal mapping + if (loadmodel.normals && loadmodel.texcoords) { + this._generateTangentSpace(loadmodel); + } + + // Set texture name (convention: same as model name without .obj) + const baseName = name.replace(/\.obj$/i, '.png').replace(/^models\//i, 'textures/'); + loadmodel.textureName = baseName; + +… +``` + +❌ Omitting comments: + +```typescript + + this.#calculateBounds(loadmodel); + + if (loadmodel.normals !== null && loadmodel.texcoords !== null) { + this.#generateTangentSpace(loadmodel); + } + + const baseName = name.replace(/\.obj$/i, '.png').replace(/^models\//i, 'textures/'); + loadmodel.textureName = baseName; + +``` + +✅ With comments preserved and updated: + +```typescript + + // Calculate bounding box for frustum culling + this.#calculateBounds(loadmodel); + + // Generate tangents/bitangents needed for normal mapping + if (loadmodel.normals !== null && loadmodel.texcoords !== null) { + this.#generateTangentSpace(loadmodel); + } + + // Derive texture name from model name (e.g. models/foo.obj → textures/foo.png) + const baseName = name.replace(/\.obj$/i, '.png').replace(/^models\//i, 'textures/'); + loadmodel.textureName = baseName; + +``` + +**Never ever delete TODO or FIXME unless the underlying issue has been fully resolved.** If the comment is no longer relevant, update it instead of deleting. + +### Adding missing tests + +If you encounter important logic that is not covered by tests, add new tests to cover it. This is especially critical for complex algorithms, edge cases, or any code that has caused bugs in the past. + +If code looks risky or has had bugs before, but there are no tests for it, that's a strong signal that tests should be added. Don't skip this step just to get the TS port done faster — the goal is not just to convert to TypeScript, but to improve code quality and maintainability overall. + +### Initialize the registry properly + +```typescript + +// ❌ This will cause static analysis regarding e.g. Con being undefined: + +let { Con } = registry; + +eventBus.subscribe('registry.frozen', () => { + ({ Con } = registry); +}); + +// ✅ Instead, initialize with the helper function that has the correct typing and will be updated when the registry is frozen: + +let { Con } = getCommonRegistry(); + +eventBus.subscribe('registry.frozen', () => { + ({ Con } = getCommonRegistry()); +}); + +``` + +### Potential null and undefined values + +When porting, if you encounter a variable that is initialized to `null` or `undefined` and later assigned an object, make sure to update the type annotation to reflect this. For example: + +```typescript + +// ❌ Original JS with JSDoc cast +let model = /** @type {BaseModel} */ (null); + +// ✅ TS with explicit nullability +let model: BaseModel | null = null; + +``` + +There are cases where the variable is initialized to `null` but is guaranteed to be assigned a non-null value before it is used. In such cases, you can use the non-null assertion operator (`!`) when accessing the variable, or you can refactor the code to ensure that the variable is properly initialized before use. + +```typescript +// Example of using non-null assertion +let model: BaseModel | null = null; + +function initializeModel() { + model = new BaseModel(); +} + +function useModel() { + console.assert(model !== null, 'Model must be initialized before use'); + + model!.doSomething(); // Using non-null assertion +} +``` + +**Note**: When in doubt, always combine a console.assert() check with the non-null assertion to ensure that the assumption holds true at runtime. This way, if there is a case where the variable is accessed before being initialized, it will throw an error with a clear message. Prefer console.assert() over if-checks that throw errors, as it is more concise and clearly indicates that this is an invariant assumption rather than normal control flow. It will also be stripped out in production builds, so it won't have any performance impact. + +#### Avoiding unnecessary null checks together with indirections + +Another important optimization while porting over code and in regards to nullability is to **avoid unnecessary null checks**. If you have a variable that is initialized to `null` but is guaranteed to be assigned a non-null value before it is used, you can safely use the non-null assertion operator (`!`) without adding redundant null checks throughout the code. + +```typescript + +// ❌ ent.entity is guaranteed to be non-null, so the null check would be redundant and add unnecessary complexity + +for (const ent of SV.area.tree.queryAABB(mins, maxs)) { + if (ent.num === 0 || ent.isFree()) { + continue; + } + + const eorg = origin.copy().subtract(ent.entity.origin.copy().add(ent.entity.mins.copy().add(ent.entity.maxs).multiply(0.5))); + + if (eorg.len() > radius) { + continue; + } + + if (!filterFn || filterFn(ent)) { + edicts.push(ent); + } +} + +// ✅ Instead, use non-null assertion and add a console.assert to ensure the assumption holds + +for (const ent of SV.area.tree.queryAABB(mins, maxs)) { + if (ent.num === 0 || ent.isFree()) { + continue; + } + + const entity = ent.entity!; // Non-null assertion + console.assert(entity !== null, 'Entity must be initialized before use'); + + const eorg = origin.copy().subtract(entity.origin.copy().add(entity.mins.copy().add(entity.maxs).multiply(0.5))); + + if (eorg.len() > radius) { + continue; + } + + if (!filterFn || filterFn(ent)) { + edicts.push(ent); + } +} + +``` + +In absolutely guaranteed non-null cases, avoid the console.assert as well. + +### Do not touch game code unless necessary + +When porting the engine code, try to avoid making changes to the core game logic or mechanics unless it is necessary for the TypeScript conversion. Before making any changes, raise a question or discussion to clarify whether the change is necessary and beneficial for the overall codebase. If a change is needed, make sure to add tests to cover the new behavior and ensure that all existing tests still pass. diff --git a/.github/instructions/unit-tests.instructions.md b/.github/instructions/unit-tests.instructions.md index 72148fd0..22cc72f4 100644 --- a/.github/instructions/unit-tests.instructions.md +++ b/.github/instructions/unit-tests.instructions.md @@ -3,8 +3,11 @@ ### Runner and File Layout - **Test runner**: Node.js built-in (`node:test`). No third-party frameworks. -- **Primary glob**: `node --test test/**/*.test.mjs` (see `package.json` scripts). -- **Category globs**: Keep tests grouped by top-level area such as `test/common/`, `test/physics/`, and `test/renderer/`. +- **Primary engine glob**: `node --test test/**/*.test.mjs` (see `package.json` scripts). +- **Repo-local game globs**: `node --test source/game/**/test/*.test.mjs source/game/**/test/**/*.test.mjs`. +- **Folder ownership**: Keep engine-owned tests under `test/`. Keep game/mod-owned tests in repo-local folders under `source/game//test/`. +- **Repo-local organization**: Within game repos, prefer mirroring the source layout with folders such as `client/`, `entity/`, `helper/`, `monster/`, `props/`, and `core/` when that keeps related tests easier to find. +- **Category globs**: Keep engine tests grouped by top-level area such as `test/common/`, `test/physics/`, and `test/renderer/`. - **File naming**: `.test.mjs`. One file per production class/module. - **Shared helpers**: `test/physics/fixtures.mjs` (no `.test.` — never auto-run). - **All files are ESM** (`.mjs`). Use `import`/`export` exclusively. @@ -61,25 +64,27 @@ withMockRegistry({ ### Writing New Tests 1. **One file per production module**: `ServerPhysics` → `test/physics/server-physics.test.mjs`, `Mod` → `test/common/model-cache.test.mjs`. -2. **Regression tests go in the relevant subsystem file**, not a catch-all file. -3. **Document magic numbers** with a comment explaining the derivation. Example: - ```javascript - // checkStuck tries: 1 (current pos) + 1 (oldorigin) + 18 z × 3 x × 3 y = 164 - assert.equal(testCallCount, 164); - ``` -4. **Prefer precise assertions** (`assert.deepEqual`, `assert.equal`) over loose checks. -5. **Use `assertNear`** for any floating-point comparison. -6. **Avoid `Math.random` in production paths** — if production uses it, save and restore in tests: - ```javascript - const originalRandom = Math.random; - Math.random = () => 0.0; - try { ... } finally { Math.random = originalRandom; } - ``` +2. **Game/mod tests live with their repo**: `source/game/id1/test/entity/items.test.mjs`, `source/game/id1/test/monster/ogre.test.mjs`, `source/game/hellwave/test/hellwave-game-api.test.mjs`. +3. **Regression tests go in the relevant subsystem file**, not a catch-all file. +4. **Document magic numbers** with a comment explaining the derivation. Example: + ```javascript + // checkStuck tries: 1 (current pos) + 1 (oldorigin) + 18 z × 3 x × 3 y = 164 + assert.equal(testCallCount, 164); + ``` +5. **Prefer precise assertions** (`assert.deepEqual`, `assert.equal`) over loose checks. +6. **Use `assertNear`** for any floating-point comparison. +7. **Avoid `Math.random` in production paths** — if production uses it, save and restore in tests: + ```javascript + const originalRandom = Math.random; + Math.random = () => 0.0; + try { ... } finally { Math.random = originalRandom; } + ``` ### Running Tests ```bash npm test # all tests +npm run test:game # repo-local game/mod tests npm run test:common # common engine tests npm run test:physics # all physics tests npm run test:renderer # renderer tests diff --git a/.github/instructions/workers.instructions.md b/.github/instructions/workers.instructions.md index 2bf424b6..24a0ba19 100644 --- a/.github/instructions/workers.instructions.md +++ b/.github/instructions/workers.instructions.md @@ -8,28 +8,28 @@ sends back responses. ### Key files -- `source/engine/common/PlatformWorker.mjs` — Single worker wrapper that detects the +- `source/engine/common/PlatformWorker.ts` — Single worker wrapper that detects the platform at module load and normalises message, error, and shutdown APIs. -- `source/engine/common/WorkerFactories.mjs` — Maps worker names to factory functions +- `source/engine/common/WorkerFactories.ts` — Maps worker names to factory functions using the `new Worker(new URL(...))` pattern for Vite static analysis. New workers **must** be registered here. -- `source/engine/common/WorkerManager.mjs` — Orchestrator that spawns workers and +- `source/engine/common/WorkerManager.ts` — Orchestrator that spawns workers and bridges the eventBus between the main thread and worker threads. -- `source/engine/common/WorkerFramework.mjs` — Bootstrap code that runs **inside** a +- `source/engine/common/WorkerFramework.ts` — Bootstrap code that runs **inside** a worker thread; sets up a lean registry, Con proxy, and message bridge. ### Adding a new worker -1. Create the worker script in `source/engine/server/` (see `DummyWorker.mjs` for an example). - The filename **must** end in `Worker.mjs`. -2. Register it in `WorkerFactories.mjs` with a `new Worker(new URL(...))` factory. +1. Create the worker script in `source/engine/server/` (see `DummyWorker.ts` for an example). + The filename **must** end in `Worker.ts`. +2. Register it in `WorkerFactories.ts` with a `new Worker(new URL(...))` factory. -The dedicated build automatically discovers `*Worker.mjs` files and bundles them +The dedicated build automatically discovers `*Worker.ts` files and bundles them into `dist/dedicated/workers/` via a secondary Rollup pass (`dedicatedWorkerBundlePlugin` in `vite.config.dedicated.mjs`). ### How the Worker global works in Node.js -The dedicated server entry (`main-dedicated.mjs`) polyfills `globalThis.Worker` with -`worker_threads.Worker` so that `WorkerFactories.mjs` can use the same +The dedicated server entry (`main-dedicated.ts`) polyfills `globalThis.Worker` with +`worker_threads.Worker` so that `WorkerFactories.ts` can use the same `new Worker(url, opts)` constructor on every platform. diff --git a/.gitignore b/.gitignore index 37757113..86861dae 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ data/id1/*.pak data/ dist/ .wrangler/ +.fallow/ diff --git a/.plan b/.plan index 7c95cf36..4b5d54d8 100644 --- a/.plan +++ b/.plan @@ -34,7 +34,7 @@ Grouped by sections and then sorted by priority (more or less). - [_] CL: go over the client API again, too much confusing lingo here and there after me hacking in lots of features - [_] SV: cleanup the `SV.WriteEntitiesToClient` mess a bit, make it more flexible - [_] cleanup `charCodeAt` everywhere -- [_] completely get rid of `Host.client` +- [X] completely get rid of `Host.client` - [X] refactor `MSG`/`SZ` especially in regards to `NET.message` and `client[…].message` - [_] `CL.state.intermission`/`ExitIntermission`: unsure what happens next on DS - [_] remove C style code that is almost verbatim from Quake @@ -157,6 +157,13 @@ Grouped by sections and then sorted by priority (more or less). ## Log +### 2026-04-05 + +- The Grand TypeScript Port™ + - due to the use of annotations, dedicated must be transpiled before run + - decided to ditch QuakeC VM and legacy code paths completely + - decided to ditch old SBar code as well + ### 2026-04-01 - removed nipplejs diff --git a/Dockerfile b/Dockerfile index 0f9abf46..45115295 100644 --- a/Dockerfile +++ b/Dockerfile @@ -12,10 +12,10 @@ COPY package.json package-lock.json ./ RUN npm ci COPY index.html ./index.html -COPY dedicated.mjs ./dedicated.mjs +COPY dedicated.ts ./dedicated.ts COPY vite.config.mjs ./vite.config.mjs COPY vite.config.dedicated.mjs ./vite.config.dedicated.mjs -COPY jsconfig.json ./jsconfig.json +COPY tsconfig.json ./tsconfig.json COPY source ./source COPY public ./public COPY test ./test @@ -60,4 +60,4 @@ EXPOSE 3000 HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \ CMD wget -qO /dev/null http://localhost:3000/ || exit 1 -CMD ["npm", "run", "dedicated:start:production"] +CMD ["npm", "run", "dedicated:start"] diff --git a/README.md b/README.md index 3f4eab8b..32daa1b8 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # The Quake Shack Engine -This is a modern JavaScript port of Quake 1 with some features sprinkled on top. +This is a modern TypeScript port of Quake 1 with some features sprinkled on top. Added features include, but are not limited to: @@ -83,7 +83,7 @@ Open http://localhost:3000/ and enjoy hacking. The dedicated server can be started using `npm run dedicated:start`, but this will run with many `console.assert(…)` in hot paths and is only really suitable for development work. -That’s why you should compile the dedicated server with `npm run dedicated:build:production` and start it using `npm run dedicated:start:production`. +That’s why you should compile the dedicated server with `npm run dedicated:build:production` and start it using `npm run dedicated:start`. ### Deploy to a CDN @@ -158,8 +158,8 @@ There are two main entrypoints: | File | Description | | - | - | -| source/engine/main-browser.mjs | launcher for a full browser session | -| source/engine/main-dedicated.mjs | launcher for a dedicated server | +| source/engine/main-browser.ts | launcher for a full browser session | +| source/engine/main-dedicated.ts | launcher for a dedicated server | ## Based on the work of diff --git a/dedicated.mjs b/dedicated.ts old mode 100755 new mode 100644 similarity index 69% rename from dedicated.mjs rename to dedicated.ts index 69d0d5f0..7a85c2fe --- a/dedicated.mjs +++ b/dedicated.ts @@ -1,6 +1,6 @@ -#!/usr/bin/env node import process from 'node:process'; -import EngineLauncher from './source/engine/main-dedicated.mjs'; + +import EngineLauncher from './source/engine/main-dedicated.ts'; // make sure working directory is the directory of this script process.chdir(new URL('./', import.meta.url).pathname); diff --git a/docs/code-style-guide.md b/docs/code-style-guide.md index 4b8b51d8..abcee39d 100644 --- a/docs/code-style-guide.md +++ b/docs/code-style-guide.md @@ -61,10 +61,10 @@ This document outlines the coding conventions and style rules for the Quakeshack 5. **Use specific types from imports** ```javascript // ✅ GOOD - /** @type {import('./ClientEntities.mjs').ClientEdict} */ + /** @type {import('./ClientEntities.ts').ClientEdict} */ // ✅ GOOD for model types - /** @type {import('../../common/model/BSP.mjs').BrushModel} */ + /** @type {import('../../common/model/BSP.ts').BrushModel} */ ``` ## Registry and Global Variables @@ -96,11 +96,11 @@ eventBus.subscribe('registry.frozen', () => { ```javascript // ❌ NEVER ACCESS DIRECTLY - This breaks in nested scopes and loses type inference registry.Con.DPrint(...); // WRONG! -registry.Mod.type.brush; // WRONG! +registry.Mod.ClearAll(); // WRONG! // ✅ ALWAYS USE - Destructured variables work everywhere Con.DPrint(...); // CORRECT! -Mod.type.brush; // CORRECT! +Mod.ClearAll(); // CORRECT! ``` **Important:** Even in files that already have registry access, always set up the destructuring prolog at the top of the file. Never use `registry.ModuleName` syntax anywhere in the code. @@ -118,7 +118,7 @@ Mod.type.brush; // CORRECT! ```javascript // ✅ GOOD - GL is not in registry, import directly -import GL from './GL.mjs'; +import GL from './GL.ts'; let gl = null; eventBus.subscribe('gl.ready', () => { @@ -188,16 +188,16 @@ eventBus.subscribe('gl.ready', () => { ## File Organization -### No index.mjs Files +### No index.ts Files Avoid barrel exports - use direct imports instead: ```javascript -// ❌ BAD (using index.mjs) +// ❌ BAD (using index.ts) import { BrushModelRenderer } from './renderer'; // ✅ GOOD (direct import) -import { BrushModelRenderer } from './renderer/BrushModelRenderer.mjs'; +import { BrushModelRenderer } from './renderer/BrushModelRenderer.ts'; ``` **Rationale:** @@ -231,10 +231,10 @@ Always verify import paths are correct: ```javascript // ❌ BAD - Wrong relative path -/** @param {import('../../../common/model/BSP.mjs').BrushModel} model */ +/** @param {import('../../../common/model/BSP.ts').BrushModel} model */ // ✅ GOOD - Correct relative path from current file -/** @param {import('../../common/model/BSP.mjs').BrushModel} model */ +/** @param {import('../../common/model/BSP.ts').BrushModel} model */ ``` ### Return Types from Library Functions @@ -262,17 +262,33 @@ getModelType() { } ``` -### Private Methods +### Protected and Private Methods -Use `_` prefix for private methods and add `@private` JSDoc tag: +Use native TypeScript access modifiers in `.ts` files. Keep `_` prefixes for protected members that subclasses override, and use `#` for methods that are truly private: ```javascript /** - * Render opaque surfaces + * Render opaque surfaces. + */ +protected _renderOpaqueSurfaces(clmodel) { + // Implementation +} + +class SignalingClient { + #connectSignaling() { + // Implementation + } +} +``` + +Keep `@protected` and `@private` tags only when a mixed JS/TS boundary still needs them. + +```javascript +/** + * Render legacy compatibility state. * @private - * @param {BrushModel} clmodel The brush model */ -_renderOpaqueSurfaces(clmodel) { +#renderCompatibilityState() { // Implementation } ``` @@ -286,12 +302,12 @@ _renderOpaqueSurfaces(clmodel) { ### Constants - Use UPPER_CASE for true constants -- Use `Mod.type.brush` pattern for enum-like values +- Use native enums like `ModelType.brush` for enum-like values ### Files -- Use PascalCase for class files: `BrushModelRenderer.mjs` -- Use camelCase for utility files: `modelUtils.mjs` -- Always use `.mjs` extension +- Use PascalCase for class files: `BrushModelRenderer.ts` +- Use camelCase for utility files: `modelUtils.ts` +- Always use `.ts` extension for TypeScript source files ## Comments diff --git a/docs/console.md b/docs/console.md index 73b88a9f..31e8ea87 100644 --- a/docs/console.md +++ b/docs/console.md @@ -2,19 +2,19 @@ Everything related to this is in: -* `Console.mjs` -* `Cmd.mjs` -* `Cvar.mjs` -* `Key.mjs` +* `Console.ts` +* `Cmd.ts` +* `Cvar.ts` +* `Key.ts` ## Console Frontend * `Con.DrawNotify` is responsible for drawing the chat input box. * `Con.DrawInput` draws the input box for the actual console. * `Con.DrawConsole` is responsible for drawing everything. -* `Key.dest.value` decides what is receiving key strokes: - * `Key.dest.console` is for the console - * `Key.dest.message` is for chatting +* `Key.destination` decides what is receiving key strokes: + * `KeyDestination.console` is for the console + * `KeyDestination.message` is for chatting * However, actual handling of the key strokes is happening in `Key.Console`. ### Console Background Customization diff --git a/docs/dedicated.md b/docs/dedicated.md index b1d1cd83..5f954121 100644 --- a/docs/dedicated.md +++ b/docs/dedicated.md @@ -2,10 +2,11 @@ QuakeShack supports running as a dedicated server in a Node.js environment. A dedicated server allows you to host multiplayer games with better performance and without requiring a client GUI to be actively running. -To start a dedicated server for production use, you can use the command: +To build and start a dedicated server for production use, run: ```sh -npm run dedicated:start:production +npm run dedicated:build:production +npm run dedicated:start ``` It will automatically execute `server.cfg` upon start. diff --git a/docs/webrtc.md b/docs/webrtc.md index 417e0976..940f867d 100644 --- a/docs/webrtc.md +++ b/docs/webrtc.md @@ -26,7 +26,7 @@ The WebRTC implementation allows clients to connect directly to each other for m ### Components -1. **WebRTCDriver (`engine/source/engine/network/NetworkDrivers.mjs`)**: +1. **WebRTCDriver (`engine/source/engine/network/NetworkDrivers.ts`)**: * Implements the `BaseDriver` interface. * Handles signaling handshake via WebSocket. * Manages `RTCPeerConnection` and `RTCDataChannel`s. diff --git a/eslint.config.mjs b/eslint.config.mjs index 24b8b520..9152a44f 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -1,4 +1,6 @@ import { defineConfig } from 'eslint/config'; +import { dirname } from 'path'; +import { fileURLToPath } from 'url'; import globals from 'globals'; import pluginJs from '@eslint/js'; import jsdoc from 'eslint-plugin-jsdoc'; @@ -6,16 +8,24 @@ import stylistic from '@stylistic/eslint-plugin'; import tseslint from '@typescript-eslint/eslint-plugin'; import tsparser from '@typescript-eslint/parser'; +const __dirname = dirname(fileURLToPath(import.meta.url)); +const typeAwareParserOptions = { + ecmaVersion: 'latest', + sourceType: 'module', + // Game modules vendored under source/game are responsible for shipping a + // local tsconfig.json when they want typed linting for their own tests. + projectService: true, + tsconfigRootDir: __dirname, +}; + const jsdocPlugin = /** @type {import('eslint').ESLint.Plugin} */ (jsdoc); const stylisticPlugin = /** @type {import('eslint').ESLint.Plugin} */ (stylistic); const typeScriptEslintPlugin = /** @type {import('eslint').ESLint.Plugin} */ ( /** @type {unknown} */ (tseslint) ); -const nodeGlobals = { - ...globals.node, -}; - -delete nodeGlobals.Buffer; +const nodeGlobals = Object.fromEntries( + Object.entries(globals.node).filter(([name]) => name !== 'Buffer'), +); const commonRules = /** @type {import('eslint').Linter.RulesRecord} */ ({ 'max-len': 'off', @@ -70,6 +80,43 @@ const commonRules = /** @type {import('eslint').Linter.RulesRecord} */ ({ '@typescript-eslint/no-redundant-type-constituents': 'warn', }); +/** Extra strict rules applied only to TypeScript files. */ +const tsStrictRules = /** @type {import('eslint').Linter.RulesRecord} */ ({ + // Replace base rules with TS-aware extension equivalents + 'consistent-return': 'off', + '@typescript-eslint/consistent-return': 'error', + 'no-duplicate-imports': 'off', + + // Strict type-checked rules + '@typescript-eslint/no-base-to-string': 'error', + '@typescript-eslint/no-confusing-void-expression': 'error', + '@typescript-eslint/no-duplicate-enum-values': 'error', + '@typescript-eslint/no-duplicate-type-constituents': 'error', + '@typescript-eslint/no-for-in-array': 'error', + '@typescript-eslint/no-implied-eval': 'error', + '@typescript-eslint/no-mixed-enums': 'error', + '@typescript-eslint/no-non-null-asserted-nullish-coalescing': 'error', + '@typescript-eslint/no-non-null-asserted-optional-chain': 'error', + '@typescript-eslint/no-redundant-type-constituents': 'error', + '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'error', + '@typescript-eslint/no-unnecessary-template-expression': 'error', + '@typescript-eslint/no-unnecessary-type-constraint': 'error', + '@typescript-eslint/no-unnecessary-type-conversion': 'error', + '@typescript-eslint/no-unsafe-enum-comparison': 'error', + '@typescript-eslint/no-useless-constructor': 'error', + '@typescript-eslint/only-throw-error': 'error', + '@typescript-eslint/prefer-promise-reject-errors': 'error', + '@typescript-eslint/prefer-reduce-type-parameter': 'error', + '@typescript-eslint/prefer-return-this-type': 'error', + '@typescript-eslint/restrict-plus-operands': ['error', { + allowNumberAndString: true, + }], + '@typescript-eslint/restrict-template-expressions': 'off', + '@typescript-eslint/return-await': ['error', 'error-handling-correctness-only'], + '@typescript-eslint/unbound-method': 'warn', + '@typescript-eslint/unified-signatures': 'error', +}); + export default defineConfig([ { ignores: [ @@ -86,11 +133,7 @@ export default defineConfig([ languageOptions: { globals: globals.browser, parser: tsparser, - parserOptions: { - ecmaVersion: 'latest', - sourceType: 'module', - project: './jsconfig.json', - }, + parserOptions: typeAwareParserOptions, }, plugins: { '@stylistic': stylisticPlugin, @@ -99,29 +142,72 @@ export default defineConfig([ }, rules: commonRules, }, + { + files: ['**/*.{ts,mts,cts}'], + languageOptions: { + globals: globals.browser, + parser: tsparser, + parserOptions: typeAwareParserOptions, + }, + plugins: { + '@stylistic': stylisticPlugin, + '@typescript-eslint': typeScriptEslintPlugin, + jsdoc: jsdocPlugin, + }, + rules: { + ...commonRules, + ...tsStrictRules, + 'jsdoc/check-param-names': 'off', + 'jsdoc/require-param': 'off', + 'jsdoc/require-param-type': 'off', + 'jsdoc/require-property-type': 'off', + 'jsdoc/require-returns-type': 'off', + 'no-undef': 'off', + 'no-unused-vars': 'off', + '@typescript-eslint/no-unused-vars': ['error', { + argsIgnorePattern: '^_', + caughtErrorsIgnorePattern: '^_', + varsIgnorePattern: '^_', + }], + }, + }, { files: [ - 'dedicated.mjs', + 'dedicated.ts', 'eslint.config.mjs', + 'eslint.config.ts', 'vite.config.mjs', + 'vite.config.ts', 'vite.config.dedicated.mjs', - 'source/engine/main-dedicated.mjs', - 'source/engine/server/**/*.mjs', - 'source/engine/common/**/*.mjs', - 'test/**/*.mjs', + 'vite.config.dedicated.ts', + 'source/engine/main-dedicated.ts', + 'source/engine/server/**/*.{mjs,ts,mts,cts}', + 'source/engine/common/**/*.{mjs,ts,mts,cts}', + 'source/game/**/test/**/*.{mjs,ts,mts,cts}', + 'test/**/*.{mjs,ts,mts,cts}', ], languageOptions: { globals: nodeGlobals, }, }, { - files: ['source/cloudflare/**/*.mjs'], + files: [ + 'source/game/**/test/**/*.{mjs,ts,mts,cts}', + 'test/**/*.{mjs,ts,mts,cts}', + ], + rules: { + 'jsdoc/require-param-type': 'off', + 'jsdoc/require-returns-type': 'off', + }, + }, + { + files: ['source/cloudflare/**/*.{mjs,ts,mts,cts}'], languageOptions: { globals: globals.serviceworker, }, }, { - files: ['**/*.cjs'], + files: ['**/*.{cjs,cts}'], languageOptions: { sourceType: 'commonjs', }, diff --git a/index.html b/index.html index 263dae8a..5fdd6bdf 100644 --- a/index.html +++ b/index.html @@ -34,7 +34,7 @@