Skip to content

fix: address code quality issues across networking and workers#1309

Merged
braedonsaunders merged 3 commits into
mainfrom
claude/fix-stun-servers-errors-QlQHU
Feb 3, 2026
Merged

fix: address code quality issues across networking and workers#1309
braedonsaunders merged 3 commits into
mainfrom
claude/fix-stun-servers-errors-QlQHU

Conversation

@braedonsaunders

Copy link
Copy Markdown
Owner
  • Make STUN servers configurable via NEXT_PUBLIC_ICE_SERVERS env var with additional fallback servers (stun2, stun3.l.google.com)
  • Fix silent error swallowing in usePublicLobbies.ts by logging non-rate-limit errors before returning empty array
  • Replace console statements in workers with debug logger:
    • pathfinding.worker.ts: Use debugPathfinding
    • vision.worker.ts: Use debugVision (via terrain category)
    • ai-decisions.worker.ts: Use debugAI
    • All workers now support setDebugSettings message for live toggle
  • Remove legacy faction names (terran/protoss/zerg) from FACTION_COLORS in rendering.config.ts - only dominion and neutral are used
  • Fix stim_pack cooldown mismatch: TS now matches JSON (cooldown: 10)
  • Add documentation noting JSON is authoritative for ability data

https://claude.ai/code/session_01EpqD3m816v9JtwW5DcVbNt

- Make STUN servers configurable via NEXT_PUBLIC_ICE_SERVERS env var
  with additional fallback servers (stun2, stun3.l.google.com)
- Fix silent error swallowing in usePublicLobbies.ts by logging
  non-rate-limit errors before returning empty array
- Replace console statements in workers with debug logger:
  - pathfinding.worker.ts: Use debugPathfinding
  - vision.worker.ts: Use debugVision (via terrain category)
  - ai-decisions.worker.ts: Use debugAI
  - All workers now support setDebugSettings message for live toggle
- Remove legacy faction names (terran/protoss/zerg) from FACTION_COLORS
  in rendering.config.ts - only dominion and neutral are used
- Fix stim_pack cooldown mismatch: TS now matches JSON (cooldown: 10)
- Add documentation noting JSON is authoritative for ability data

https://claude.ai/code/session_01EpqD3m816v9JtwW5DcVbNt

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 19808555a9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 341 to 347
export const FACTION_COLORS = {
terran: {
primary: 0xffaa00, // Orange-yellow tracer
secondary: 0xff6600, // Orange trail
glow: 0xffdd44, // Bright yellow glow
},
dominion: {
primary: 0xffaa00, // Orange-yellow tracer (same as terran for now)
primary: 0xffaa00, // Orange-yellow tracer
secondary: 0xff6600, // Orange trail
glow: 0xffdd44, // Bright yellow glow
},
protoss: {
primary: 0x4488ff, // Blue energy
secondary: 0x8844ff, // Purple trail
glow: 0x66aaff, // Cyan glow
},
zerg: {
primary: 0x88ff44, // Acid green
secondary: 0x44aa00, // Dark green trail
glow: 0xaaff66, // Bright green glow
},
neutral: {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Restore a valid fallback faction color

By removing the legacy keys here, any code that still falls back to 'terran' now resolves FACTION_COLORS[faction] to undefined and will throw when accessing .primary/.glow. BattleEffectsRenderer still defaults to 'terran' for attackerFaction/faction (see lines ~721 and ~761 in src/rendering/effects/BattleEffectsRenderer.ts), so missing/unknown factions will now crash projectile effects at runtime. Either update those fallbacks to an existing key (e.g. dominion/neutral) or add a defensive fallback when indexing FACTION_COLORS.

Useful? React with 👍 / 👎.

- Move STUN/ICE server config from env vars to public/data/networking.json
- Remove ABILITY_DEFINITIONS constant from abilities.ts (JSON is authority)
- Remove AbilityRegistry class and related getAbility/getUnitAbilities
- Remove unused getAbilityDefinitions() from DefinitionRegistry
- Keep type definitions (AbilityDataDefinition, etc.) for type checking
- Keep utility functions (canUseAbility, calculateAbilityDamage)

https://claude.ai/code/session_01EpqD3m816v9JtwW5DcVbNt
- Replace 'terran' fallback with 'dominion' in CombatSystem.ts
- Replace 'terran' fallback with 'dominion' in BattleEffectsRenderer.ts
- Remove SC2 references from comments in AssetManager.ts
- Fix type error in useMultiplayer.ts ICE server loading

https://claude.ai/code/session_01EpqD3m816v9JtwW5DcVbNt
@braedonsaunders braedonsaunders merged commit 8bb5c51 into main Feb 3, 2026
4 checks passed
@braedonsaunders braedonsaunders deleted the claude/fix-stun-servers-errors-QlQHU branch February 3, 2026 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants