Skip to content

feat(config): add validation for out-of-range config values#43

Merged
phlx0 merged 1 commit intophlx0:mainfrom
LuisMIguelFurlanettoSousa:feat/config-validation
Mar 30, 2026
Merged

feat(config): add validation for out-of-range config values#43
phlx0 merged 1 commit intophlx0:mainfrom
LuisMIguelFurlanettoSousa:feat/config-validation

Conversation

@LuisMIguelFurlanettoSousa
Copy link
Copy Markdown

Summary

Closes #42

Adds a Validate() method to Config that checks value ranges after TOML decoding:

  • engine.fps: 1–120
  • engine.cycle_seconds: >= 0
  • scene.rain.density: 0.0–1.0
  • scene.life.density: 0.0–1.0
  • scene.waveform.amplitude: 0.0–1.0
  • scene.pipes.turn_chance: 0.0–1.0

Returns a clear multi-line error listing all invalid fields at once (not just the first one).

Includes table-driven tests for every validated field plus a test confirming defaults pass.

Test plan

  • go vet ./... passes
  • go test ./... passes — including 7 out-of-range subtests and defaults validation

@github-actions github-actions bot added enhancement New feature or request scene New or modified scene config Config or CLI changes labels Mar 29, 2026
Copy link
Copy Markdown
Owner

@phlx0 phlx0 left a comment

Choose a reason for hiding this comment

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

The core idea is good. Load() is the right place for this and collecting all errors instead of failing on the first one is the right approach.

Three things to fix before this can merge:

Portuguese comments again (Cenário, Ação, Validação) — please use English, same note as on #41.

Missing CHANGELOG.md entry under ## [Unreleased].

Inconsistent coverage is the main concern. The PR validates rain.density, life.density, waveform.amplitude and pipes.turn_chance but leaves out starfield.count, orrery.bodies, pipes.heads, particles.count and all the speed and reset_seconds fields. A user setting starfield.count = -5 or orrery.bodies = 0 would still get silent unexpected behavior. Either cover everything with meaningful bounds or add a comment explaining why certain fields are intentionally excluded. As written it gives a false sense of safety.

@phlx0 phlx0 added the Reviewed: Action Needed PR was reviewed, action is needed label Mar 29, 2026
@github-actions github-actions bot added the documentation Documentation changes label Mar 30, 2026
@phlx0 phlx0 force-pushed the feat/config-validation branch from 147030c to b83d7ba Compare March 30, 2026 22:39
Closes phlx0#42

Adds Validate() to Config, called by Load() after TOML decoding.
Collects all invalid fields into a single multi-line error rather than
failing on the first. Covers every numeric field with meaningful bounds
across all scenes including starfield.
@phlx0 phlx0 force-pushed the feat/config-validation branch from b83d7ba to 5ccc7b1 Compare March 30, 2026 22:41
@phlx0 phlx0 merged commit 9cd3c41 into phlx0:main Mar 30, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

config Config or CLI changes documentation Documentation changes enhancement New feature or request Reviewed: Action Needed PR was reviewed, action is needed scene New or modified scene

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid config values are silently accepted

2 participants