Skip to content

Conversation

@GhostTeen96
Copy link

@GhostTeen96 GhostTeen96 commented Nov 12, 2025

If this PR fixes an issue, link it below. If not, delete these two lines.
Resolves #(issue number)

Description:

  • refactor SAM launcher cost evaluation to treat the first build, subsequent builds, and level upgrades distinctly (1.5M → 3M → 6M flow) in DefaultConfig so economy scaling matches gameplay design
  • align SAM launcher range growth with a linear 70/85/100 progression that matches hydrogen bomb outer radius, replacing the previous asymptotic curve
  • add comprehensive SAMLauncherConfig tests verifying cost tiers, upgrade pricing, and range values across level combinations to guard against regressions
  • drop the deprecated "package.json" dependency from package.json and regenerate package-lock.json so the project no longer installs the unsafe package

Testing

  • npm test -- SAMLauncherConfig

Please complete the following:

  • I have added screenshots for all UI updates
  • I process any text displayed to the user through translateText() and I've added it to the en.json file
  • I have added relevant tests to the test directory
  • I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

Orion Nebula
orion_nebula22

@GhostTeen96 GhostTeen96 requested a review from a team as a code owner November 12, 2025 01:51
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 12, 2025

Walkthrough

Replaces SAMLauncher cost wrapper with a dynamic, tiered cost function that considers player type, infiniteGold, unitsOwned and unitsConstructed; changes samRange(level) to linear values 70/85/100; adds tests validating pricing tiers, unit tracking, and range values.

Changes

Cohort / File(s) Summary
SAMLauncher Configuration Logic
src/core/configuration/DefaultConfig.ts
Replaced wrapper-based cost with a direct (p: Player) => bigint function that bypasses cost for human players with infiniteGold, computes totalLevels from unitsOwned/unitsConstructed, and applies tiered pricing: 1.5M for first SAM, 6M for the specific level-2→3 case, otherwise 3M. Changed samRange(level) to linear mapping: level1→70, level2→85, level3→100.
SAMLauncher Test Suite
tests/core/configuration/SAMLauncherConfig.test.ts
Added tests that initialize game state, spawn a player, perform builds/upgrades, and assert SAM pricing progression (1.5M, 3.0M, 3.0M, 6.0M scenarios), unit counts/levels tracking, and samRange values for levels 1–3.

Sequence Diagram(s)

sequenceDiagram
  participant Player
  participant DefaultConfig
  participant GameState

  Note over Player,DefaultConfig: Request cost for SAMLauncher
  Player->>DefaultConfig: get SAMLauncher cost(player)
  DefaultConfig->>GameState: read unitsOwned / unitsConstructed / units info
  alt player.human && player.infiniteGold
    DefaultConfig-->>Player: cost = 0
  else
    alt totalLevels == 0
      DefaultConfig-->>Player: cost = 1_500_000
    else
      alt upgrading level2 -> 3 and condition met
        DefaultConfig-->>Player: cost = 6_000_000
      else
        DefaultConfig-->>Player: cost = 3_000_000
      end
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Pay attention to the tier condition that detects the level-2→3 upgrade (uses unitsOwned vs unitsConstructed and totalLevels).
  • Verify human/infiniteGold bypass is applied in all relevant code paths.
  • Confirm samRange linear values match intended gameplay expectations and tests.

Possibly related PRs

Suggested labels

Balance Tweak, Feature - Simulation

Suggested reviewers

  • evanpelle
  • jrouillard

Poem

Golden tiers for SAMs now told,
First at low, upgrades at bold,
Ranges march in even line —
Seventy, eighty-five, then one-O-O fine! 🚀

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main changes: removing a deprecated dependency and aligning SAM launcher tests with adjusted costs, matching the core objectives.
Description check ✅ Passed The description clearly relates to the changeset, detailing SAM launcher cost refactoring, range adjustments, test additions, and dependency removal—all present in the raw summary.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0e42292 and d635971.

📒 Files selected for processing (1)
  • tests/core/configuration/SAMLauncherConfig.test.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/core/configuration/SAMLauncherConfig.test.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0200df3 and 0e42292.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • src/core/configuration/DefaultConfig.ts (2 hunks)
  • tests/core/configuration/SAMLauncherConfig.test.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-24T11:13:08.495Z
Learnt from: DevelopingTom
Repo: openfrontio/OpenFrontIO PR: 1900
File: src/core/execution/SAMLauncherExecution.ts:103-111
Timestamp: 2025-08-24T11:13:08.495Z
Learning: In SAMLauncherExecution.ts, the cached target bug can only occur if: 1) SAM is not on cooldown when nuke is in range, 2) SAM goes on cooldown right after computing trajectory, 3) SAM becomes available again before nuke explodes. This is not possible with current cooldown values but the fix is still valuable for robustness.

Applied to files:

  • tests/core/configuration/SAMLauncherConfig.test.ts
  • src/core/configuration/DefaultConfig.ts
📚 Learning: 2025-10-20T20:15:28.858Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.

Applied to files:

  • tests/core/configuration/SAMLauncherConfig.test.ts
  • src/core/configuration/DefaultConfig.ts
📚 Learning: 2025-08-18T03:11:18.617Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 1851
File: src/core/execution/FakeHumanExecution.ts:433-441
Timestamp: 2025-08-18T03:11:18.617Z
Learning: In FakeHumanExecution.maybeSpawnStructure, the perceived cost multiplier (owned + 1, capped at 5) applied to structure costs is intentional. This makes AI nations build fewer structures than players could, saving gold for bombs/nukes rather than maxing out structures.

Applied to files:

  • src/core/configuration/DefaultConfig.ts
🧬 Code graph analysis (2)
tests/core/configuration/SAMLauncherConfig.test.ts (4)
src/core/configuration/DefaultConfig.ts (1)
  • DefaultConfig (227-1013)
tests/util/Setup.ts (1)
  • setup (23-81)
tests/util/TestConfig.ts (1)
  • TestConfig (12-87)
src/core/game/PlayerImpl.ts (2)
  • unitsOwned (251-263)
  • unitsConstructed (227-237)
src/core/configuration/DefaultConfig.ts (3)
src/core/game/Game.ts (1)
  • Player (526-662)
src/core/game/UnitImpl.ts (1)
  • level (415-417)
src/core/game/GameView.ts (1)
  • level (167-169)

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.

1 participant