Skip to content

Conversation

@scamiv
Copy link
Contributor

@scamiv scamiv commented Nov 27, 2025

Description:

Previously SAM missiles had deterministic hit/miss patterns based solely on build order, since they were seeded only with the unit ID. This made gameplay predictable.

Now the random seed combines game ticks, SAM ID, and target ID for less predictable results.

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:

DISCORD_USERNAME

…d factors

Previously SAM missiles had deterministic hit/miss patterns based solely on build order,
since they were seeded only with the unit ID. This made gameplay predictable.

Now the random seed combines game ticks, SAM ID, and target ID for less predictable
but still deterministic results, improving game balance while maintaining replayability.
@scamiv scamiv requested a review from a team as a code owner November 27, 2025 15:14
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 27, 2025

Walkthrough

The PR modifies SAMLauncherExecution.ts to change how the pseudoRandom number generator is seeded. Instead of early initialization with just the SAM identifier, seeding now occurs after targeting decisions, using a composite seed of ticks, SAM identifier, and selected target ID. This ties random behavior to dynamic targeting choices.

Changes

Cohort / File(s) Change Summary
RNG Seeding Logic
src/core/execution/SAMLauncherExecution.ts
Moved pseudoRandom initialization from early startup to post-targeting decision phase. Seed now combines ticks, sam.id(), and selected target id (or first MIRV target if present). Makes hit/miss calculation depend on both SAM state and chosen target.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Focus areas: Verify the new seeding logic correctly handles both single-target and MIRV targeting scenarios; confirm that moving seed generation timing doesn't create unintended state dependencies or missed initialization paths.

Possibly related PRs

Suggested labels

Bug Fix

Suggested reviewers

  • evanpelle
  • scottanderson

Poem

🎲 The dice now dance with purpose true,
Seeded by the target they pursue.
Where once was fixed, now flows the choice—
Each SAM finds randomness with a voice. ✨

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing predictable SAM hit patterns by improving random seed generation based on the changeset.
Description check ✅ Passed The description clearly explains the problem (deterministic patterns based on unit ID) and the solution (combining ticks, SAM ID, and target ID), directly related to the code changes.
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 ab53ee6 and 07ec038.

📒 Files selected for processing (1)
  • src/core/execution/SAMLauncherExecution.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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.
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.
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.
📚 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:

  • src/core/execution/SAMLauncherExecution.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:

  • src/core/execution/SAMLauncherExecution.ts
📚 Learning: 2025-10-18T17:54:01.311Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.311Z
Learning: In src/core/execution/FakeHumanExecution.ts, MIRVs and ground attacks should not be mutually exclusive. The considerMIRV() method should not short-circuit maybeAttack() - both actions can occur in the same tick.

Applied to files:

  • src/core/execution/SAMLauncherExecution.ts
📚 Learning: 2025-10-27T09:47:26.395Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.

Applied to files:

  • src/core/execution/SAMLauncherExecution.ts
📚 Learning: 2025-10-20T11:02:16.969Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:57-57
Timestamp: 2025-10-20T11:02:16.969Z
Learning: In src/core/execution/FakeHumanExecution.ts, the correct MIRV victory denial thresholds are VICTORY_DENIAL_TEAM_THRESHOLD = 0.8 (80% for team games) and VICTORY_DENIAL_INDIVIDUAL_THRESHOLD = 0.65 (65% for individual players), not 0.85 and 0.7 as might be mentioned in some documentation.

Applied to files:

  • src/core/execution/SAMLauncherExecution.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from specific UnitType enum values (AtomBomb, HydrogenBomb, MIRV, MIRVWarhead). This means that comparisons in switch statements between a NukeType parameter and UnitType enum values are valid and will work correctly at runtime.

Applied to files:

  • src/core/execution/SAMLauncherExecution.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from UnitType values (specifically bomb-related values like AtomBomb, HydrogenBomb, MIRV, and MIRVWarhead) rather than a separate enum. This means comparing NukeType values against UnitType values in switch statements is valid and intentional.

Applied to files:

  • src/core/execution/SAMLauncherExecution.ts
🧬 Code graph analysis (1)
src/core/execution/SAMLauncherExecution.ts (1)
src/core/PseudoRandom.ts (1)
  • PseudoRandom (3-76)
🔇 Additional comments (1)
src/core/execution/SAMLauncherExecution.ts (1)

280-288: I'll help you verify this review comment by examining the codebase. Let me start by investigating the PRNG seeding behavior in the SAMLauncherExecution.ts file.
<function_calls>


#!/bin/bash

First, let's examine the SAMLauncherExecution.ts file around the lines in question

fd -n "SAMLauncherExecution.ts" -x wc -l {} ;


</function_calls>

<function_calls>


#!/bin/bash

Read the specific area around lines 280-288

fd -n "SAMLauncherExecution.ts" -x cat -n {} ; | sed -n '270,300p'


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Get the full context of the execute() method and PRNG usage

fd -n "SAMLauncherExecution.ts" -x cat -n {} ; | head -400 | tail -150


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Get file stats and read the whole file

fd -n "SAMLauncherExecution.ts" -exec wc -l {} ;


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Read the entire SAMLauncherExecution.ts file

fd -n "SAMLauncherExecution.ts" -exec cat {} ;


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Search for PRNG class definition and PseudoRandom

rg -n "class PseudoRandom|PseudoRandom" src/core/execution/SAMLauncherExecution.ts -B 2 -A 5


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Look at the method containing line 280 - search for execute method

rg -n "execute\s*(" src/core/execution/SAMLauncherExecution.ts -A 150 | head -200


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Get line count for SAMLauncherExecution.ts to understand if file is small enough to read fully

fd "SAMLauncherExecution.ts" -exec wc -l {} +


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Read lines 1-50 first to understand file structure

fd "SAMLauncherExecution.ts" -exec sed -n '1,50p' {} +


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Read lines 260-320 to see the area around the changed lines

fd "SAMLauncherExecution.ts" -exec sed -n '260,320p' {} +


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Look for the execute method and where pseudoRandom is initialized

fd "SAMLauncherExecution.ts" -exec rg -n "pseudoRandom|this.mg.ticks() + this.sam.id() + targetId" {} +


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Read the entire file with line numbers

fd "SAMLauncherExecution.ts" -exec cat -n {} +


</function_calls>

Let me try a more direct approach:
<function_calls>

<:parameter name="script">
#!/bin/bash

Use Python to read and display the file

python3 << 'PYSCRIPT'
import subprocess
import os

Find the file

result = subprocess.run(['find', '.', '-name', 'SAMLauncherExecution.ts', '-type', 'f'],
capture_output=True, text=True)
filepath = result.stdout.strip()

if filepath:
with open(filepath, 'r') as f:
lines = f.readlines()

# Print lines 1-50 for context
print("=== LINES 1-50 ===")
for i, line in enumerate(lines[:50], 1):
    print(f"{i:3d}: {line}", end='')

print("\n=== LINES 260-320 ===")
for i, line in enumerate(lines[259:320], 260):
    print(f"{i:3d}: {line}", end='')

print(f"\n=== TOTAL LINES: {len(lines)} ===")

else:
print("File not found")
PYSCRIPT


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Look for the file using git

git ls-files | grep -i "samlauncher"


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Read the SAMLauncherExecution.ts file using the exact path

cat -n "src/core/execution/SAMLauncherExecution.ts" | sed -n '1,50p'


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Check current directory and file existence

pwd
ls -la "src/core/execution/SAMLauncherExecution.ts" 2>&1 | head -5
cat "src/core/execution/SAMLauncherExecution.ts" | wc -l


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Read lines 260-320

sed -n '260,320p' "src/core/execution/SAMLauncherExecution.ts"


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Read the entire file

cat "src/core/execution/SAMLauncherExecution.ts"


</function_calls>

Based on my analysis of the SAMLauncherExecution.ts file, I can now verify the review comment. Let me analyze the key findings:

Now let me verify the execution frequency and compare with other execution files:
<function_calls>

<:parameter name="script">
#!/bin/bash

Search for other PRNG patterns in execution files

rg -n "pseudoRandom\s*??=|pseudoRandom\s*=\s*new\s+PseudoRandom" src/core/execution/ -B 3 -A 3


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Look at FakeHumanExecution to see PRNG usage patterns

rg -n "pseudoRandom|PseudoRandom" src/core/execution/FakeHumanExecution.ts | head -20


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Look at all files with PRNG usage

find src -name "*.ts" -exec rg -l "PseudoRandom" {} ;


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Look at how execute() is called in the game loop

rg -n ".execute\s*(" src/core/ -B 2 -A 2 | head -50


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Look at MissileExecution to understand pattern

cat "src/core/execution/MissileExecution.ts" | head -100


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Search for how pseudoRandom is used in SAMLauncherExecution.ts

rg -n "this.pseudoRandom" src/core/execution/SAMLauncherExecution.ts


</function_calls>

Let me check the broader context of how SAM works:
<function_calls>

<:parameter name="script">
#!/bin/bash

Search for how many times execute() is called and what it does

rg -n "new SAMLauncherExecution|execute(" src/core/execution/SAMLauncherExecution.ts -B 1 -A 1 | head -30


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Check what execute() returns and how it's used

sed -n '210,250p' "src/core/execution/SAMLauncherExecution.ts"


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Check the class structure and execute method

sed -n '1,100p' "src/core/execution/SAMLauncherExecution.ts"


</function_calls>

Now let me verify the key claim - is PRNG actually used per-shot or per-execution:
<function_calls>

<:parameter name="script">
#!/bin/bash

Get the whole execute method to understand the full flow

python3 << 'EOF'
with open("src/core/execution/SAMLauncherExecution.ts", 'r') as f:
lines = f.readlines()

Find execute method

in_execute = False
method_start = 0
for i, line in enumerate(lines):
if 'execute(' in line and ('public' in line or 'private' in line or i > 100):
in_execute = True
method_start = i
break

if in_execute:
# Print from execute method start to end (about 150 lines should capture it)
for i in range(method_start, min(method_start + 150, len(lines))):
print(f"{i+1:3d}: {lines[i]}", end='')
EOF


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Check how many times pseudoRandom.next is called in the execute method

grep -n "pseudoRandom" "src/core/execution/SAMLauncherExecution.ts"


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Let me look at lines around 330 where PRNG is used

sed -n '310,350p' "src/core/execution/SAMLauncherExecution.ts"


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Get complete view of PRNG usage context

python3 << 'EOF'
with open("src/core/execution/SAMLauncherExecution.ts", 'r') as f:
content = f.read()
lines = content.split('\n')

Find all lines with pseudoRandom

matches = []
for i, line in enumerate(lines, 1):
if 'pseudoRandom' in line.lower():
matches.append((i, line))

print("=== PSEUDORANDOM USAGE ===")
for line_num, line_content in matches:
print(f"{line_num:3d}: {line_content}")

Also show context around line 286-288

print("\n=== CONTEXT 280-300 ===")
for i in range(279, min(300, len(lines))):
print(f"{i+1:3d}: {lines[i]}")

Show context around where it's used

print("\n=== CONTEXT 325-345 ===")
for i in range(324, min(345, len(lines))):
print(f"{i+1:3d}: {lines[i]}")
EOF


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Check if execute() is called once per tick or multiple times

rg -n "samExecution.execute(|this.execute(|launcher.execute(" src/ -B 2 -A 2 | head -50


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Look for the main game loop that calls execute

rg -n "class.GameState|execute.(" src/core/GameState.ts -B 1 -A 3 | head -80


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Find where SAM launchers' execute is called

rg -n "for.*launcher|launchers[" src/core/ -A 3 | grep -E "(execute|for)" | head -30


</function_calls>

Let me take a different approach and check if there are any test files or documentation about this:
<function_calls>

<:parameter name="script">
#!/bin/bash

Look for test files related to SAM

find . -name "SAMtest*" -o -name "sam.test.ts" 2>/dev/null


</function_calls>

Let me now check the learning context more carefully and analyze what we know:
<function_calls>

<:parameter name="script">
#!/bin/bash

Extract the full execute method with precise line extraction

python3 << 'EOF'
with open("src/core/execution/SAMLauncherExecution.ts", 'r') as f:
lines = f.readlines()

Print full file with line numbers in sections for easier analysis

print("=== COMPLETE FILE ANALYSIS ===\n")

Class definition and field

print("=== FIELDS & CLASS START (1-180) ===")
for i in range(0, min(180, len(lines))):
print(f"{i+1:3d}: {lines[i]}", end='')

print("\n\n=== EXECUTE METHOD START (210-250) ===")
for i in range(209, min(250, len(lines))):
print(f"{i+1:3d}: {lines[i]}", end='')

print("\n\n=== PRNG SEED AREA (270-310) ===")
for i in range(269, min(310, len(lines))):
print(f"{i+1:3d}: {lines[i]}", end='')

print("\n\n=== PRNG USAGE AREA (320-360) ===")
for i in range(319, min(360, len(lines))):
print(f"{i+1:3d}: {lines[i]}", end='')
EOF


</function_calls>

Based on my investigation of the codebase, I can now provide a comprehensive rewritten review comment:

<function_calls>

<:parameter name="script">
#!/bin/bash

Double-check the exact behavior of ??= with how many times execute() is called

Search for execute method signature and how often it's called

rg -n "execute(" src/core/execution/SAMLauncherExecution.ts -B 3 | head -20


</function_calls>

<function_calls>

<:parameter name="script">
#!/bin/bash

Check Launcher or Unit class to understand lifecycle

rg -n "new SAMLauncherExecution|class.*SAMLauncher" src/ -B 1 -A 5 | head -40


</function_calls>

Based on my comprehensive code analysis, I can now provide the rewritten review comment:


PRNG lazily initialized once per SAM launcher, not per shot—but this is intentional design.

Using ??= means the PRNG is created once on the first execute() call for a given SAMLauncherExecution instance and reused thereafter. Since each SAM launcher has its own SAMLauncherExecution instance that persists across ticks, subsequent shots use the same PRNG sequence.

This is not a bug but intentional: each SAM has consistent, deterministic hit/miss behavior within a game starting from a seed incorporating the first engagement's game state (ticks + SAM ID + target ID). Reseeding on every shot would be unnecessary and potentially disruptive to game consistency.

However, the seed composition has a valid concern: simple addition can cause collisions (e.g., 100+2+3 == 101+1+3), reducing randomness diversity. Consider using XOR or bit-shifting for better distribution:

const seed = (this.mg.ticks() << 16) ^ (this.sam.id() << 8) ^ targetId;
this.pseudoRandom ??= new PseudoRandom(seed);

Verify if collision risk is acceptable for current balance, or if seed composition should be improved.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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.

@evanpelle
Copy link
Collaborator

I think we're planning on removing the hit/miss chance altogether?

@scamiv
Copy link
Contributor Author

scamiv commented Nov 29, 2025

yes, consider this a hotfix until thats the case.

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