Skip to content

Add comprehensive null/undefined safety checks #7

@hannyan

Description

@hannyan

The codebase has multiple locations where null or undefined values can cause runtime errors. TypeScript's strict null checking is partially disabled (strictPropertyInitialization: false), and many methods don't validate their inputs.

Risk Areas

1. Player Class

// player.playerData may be null
const hp = player.playerData.currentHP  // Potential null reference

// Player methods don't validate state
player.attack(null)  // No target validation

2. Folder References

// sceneFolder/targetFolder may be null
fx.sceneFolder.addChild(node)  // Crashes if sceneFolder is null

3. Formula Evaluation

// Variables may not exist
fx.evaluate('ATK + DEF')  // What if ATK is undefined?

4. Event System

// Listeners may be null/undefined
Eve.dispatch(eventType, null)  // Payload validation

Proposed Solutions

1. TypeScript Configuration

Enable strict null checking in tsconfig.json:

{
  "compilerOptions": {
    "strictNullChecks": true,
    "strictPropertyInitialization": true,
    "noUncheckedIndexedAccess": true
  }
}

2. Assertion Functions

// src/utils/assertions.ts
export function assertDefined<T>(
  value: T | null | undefined,
  message?: string
): asserts value is T {
  if (value === null || value === undefined) {
    throw new FxError(
      'FX0001',
      message ?? 'Expected value to be defined'
    )
  }
}

export function assertNumber(
  value: unknown,
  message?: string
): asserts value is number {
  if (typeof value !== 'number' || isNaN(value)) {
    throw new FxError(
      'FX0002',
      message ?? 'Expected value to be a valid number'
    )
  }
}

export function assertPlayer(
  player: unknown
): asserts player is Player {
  if (!(player instanceof Player)) {
    throw new CombatError(
      'FX5002',
      'Expected a valid Player instance'
    )
  }
}

3. Safe Access Utilities

// src/utils/safeAccess.ts
export function safeGet<T, K extends keyof T>(
  obj: T | null | undefined,
  key: K
): T[K] | undefined {
  return obj?.[key]
}

export function safeGetOrThrow<T, K extends keyof T>(
  obj: T | null | undefined,
  key: K,
  errorMessage?: string
): T[K] {
  if (obj == null) {
    throw new FxError('FX0001', errorMessage ?? `Cannot access ${String(key)} on null`)
  }
  return obj[key]
}

export function defaultIfNull<T>(
  value: T | null | undefined,
  defaultValue: T
): T {
  return value ?? defaultValue
}

4. Updated Method Signatures

// Before
class Player {
  attack(target: Player) {
    // target might be null
  }
}

// After
class Player {
  attack(target: Player): void {
    assertPlayer(target)
    assertDefined(this.playerData, 'Player not initialized')
    // Safe to proceed
  }
}

5. Optional Chaining Migration

// Before (unsafe)
const value = data.nested.property

// After (safe)
const value = data?.nested?.property ?? defaultValue

6. Null Object Pattern

For commonly accessed nullable objects:

// NullPlayer for safe fallback
class NullPlayer implements IPlayer {
  readonly id = 'null'
  readonly name = 'Unknown'

  attack() { /* no-op */ }
  takeDamage() { return 0 }
  getHP() { return 0 }
}

// Usage
const player = fx.getPlayer() ?? new NullPlayer()
player.attack(target)  // Safe even if player was null

Migration Plan

Phase 1: Add Assertion Utilities

  • Create assertion functions
  • Add safe access utilities
  • No breaking changes

Phase 2: Enable Strict Null Checks

  • Enable strictNullChecks: true
  • Fix compiler errors (estimated 50-100 locations)
  • Update method signatures

Phase 3: Enable Property Initialization

  • Enable strictPropertyInitialization: true
  • Add definite assignment assertions where needed
  • Initialize all class properties

Phase 4: Enable Unchecked Index Access

  • Enable noUncheckedIndexedAccess: true
  • Update array/object access patterns
  • Add null checks for dynamic access

Example Refactoring

Player.ts

// Before
class Player {
  playerData: PlayerData

  getCurrentHP() {
    return this.playerData.currentHP
  }
}

// After
class Player {
  private playerData: PlayerData | null = null

  getCurrentHP(): number {
    if (!this.playerData) {
      throw new ConfigurationError(
        ErrorCodes.CONFIG_MISSING_REQUIRED,
        'Player data not initialized'
      )
    }
    return this.playerData.currentHP
  }

  // Or with safe fallback
  getCurrentHPSafe(): number {
    return this.playerData?.currentHP ?? 0
  }
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions