Skip to content

fix: every field excursion is a different field (#3) - #47

Open
blippip69 wants to merge 1 commit into
Bitcoindefi:mainfrom
blippip69:fix/field-seed-per-excursion
Open

fix: every field excursion is a different field (#3)#47
blippip69 wants to merge 1 commit into
Bitcoindefi:mainfrom
blippip69:fix/field-seed-per-excursion

Conversation

@blippip69

Copy link
Copy Markdown
Contributor

fix: cada salida al campo es un campo distinto (#3)

ES

Problema. new Field({ player }) no recibia semilla y lib/field.js cae a
seed ?? 1: todas las excursiones generaban el mismo mapa con los mismos bichos.

Arreglo. Contador this.excursion en el constructor y semilla derivada en
las puertas de salida al campo (glifo > via enter() y restauracion de
partida guardada en el campo):

this.excursion++
const seed = (this.excursion * 2654435761 + this.player.xp * 40503 + this.player.gold) >>> 0

Derivada y no aleatoria: una misma partida sigue siendo reproducible (misma
secuencia de acciones -> mismos campos), pero dos excursiones distintas ya no
coinciden. El progreso del personaje (xp/gold) entra en la mezcla para que ni
siquiera dos partidas nuevas clonadas compartan campos.

Test nuevo: abre excursion, vuelve con t, suma oro, abre otra vez — las
semillas difieren. Negativo verificado: sin el arreglo el contador no existe y
el test no compila/semillas iguales; con el arreglo 66/66 green.

> npm test
# tests = 66/66 pass
# asserts = 519/519 pass
# ok

EN

Field excursions now derive their seed from an excursion counter plus the
character's progress (xp/gold) instead of falling back to the hardcoded seed 1,
so every excursion is a different map while a single save stays reproducible.
Regression test asserts two consecutive excursions get different seeds. 66/66
tests green.

@gitar-bot

gitar-bot Bot commented Aug 25, 2026

Copy link
Copy Markdown
CI failed: Prettier code formatting check failed during the lint script because lib/game.js does not conform to style rules.

Overview

One change-related failure was found in the CI run where the lint script failed due to a Prettier formatting violation in lib/game.js across 1 log analyzed.

Failures

Prettier Formatting Check Failed (confidence: high)

  • Type: tooling
  • Affected jobs: 97901553559
  • Related to change: yes
  • Root cause: The file lib/game.js does not conform to the project's Prettier formatting rules.
  • Suggested fix: Run 'npx prettier . --write' locally to automatically format the modified files, then commit and push the formatting changes.

Summary

  • Change-related failures: 1 failure due to code style formatting issues in the lint step.
  • Infrastructure/flaky failures: 0 infrastructure or flaky failures.
  • Recommended action: Format the codebase using Prettier by running 'npx prettier . --write' and push the updated code.
Code Review ⚠️ Changes requested 0 resolved / 2 findings

Derives a distinct field seed per excursion using a counter and player progress to ensure unique maps. The excursion counter is neither persisted in saveState() nor reset on loadSlot(), causing the seed sequence to drift after reloading.

⚠️ Bug: excursion counter isn't persisted or reset on load

📄 lib/game.js:701-715 📄 lib/game.js:773-783 📄 lib/game.js:1184-1187

The excursion field is initialized to 0 in the constructor, incremented on every field entry, but never written to saveState() nor reset in loadSlot(). As a result the derived seed after loading a save depends on how many excursions happened earlier in the current session: reloading the same save twice yields different fields, and two players loading an identical save file (same xp/gold) get different maps. This contradicts the PR's stated goal that "una misma partida sigue siendo reproducible." Persist excursion in saveState() and restore it in loadSlot() (and reset it in the constructor path used for new games) so the seed is a pure function of saved state.

Persist and restore the excursion counter so the field seed is reproducible from saved state.
// in saveState() return object:
    return {
      name: this.name,
      player,
      location,
      excursion: this.excursion,
      ...
    }

// in loadSlot(), before deriving the seed:
    this.excursion = Number(saved.excursion) || 0
💡 Quality: Seed derivation formula duplicated in two places

📄 lib/game.js:778-781 📄 lib/game.js:1184-1187

The identical (this.excursion * 2654435761 + this.player.xp * 40503 + this.player.gold) >>> 0 expression appears in both loadSlot() and enter(). Duplicating the magic constants risks the two paths drifting apart. Extract a small helper (e.g. nextFieldSeed() that also does this.excursion++) and call it from both sites.

Centralize the seed derivation so both field entry points stay in sync.
nextFieldSeed() {
  this.excursion++
  return (this.excursion * 2654435761 + this.player.xp * 40503 + this.player.gold) >>> 0
}

// both call sites:
const seed = this.nextFieldSeed()
🤖 Prompt for agents
Code Review: Derives a distinct field seed per excursion using a counter and player progress to ensure unique maps. The excursion counter is neither persisted in saveState() nor reset on loadSlot(), causing the seed sequence to drift after reloading.

1. ⚠️ Bug: excursion counter isn't persisted or reset on load
   Files: lib/game.js:701-715, lib/game.js:773-783, lib/game.js:1184-1187

   The `excursion` field is initialized to 0 in the constructor, incremented on every field entry, but never written to `saveState()` nor reset in `loadSlot()`. As a result the derived seed after loading a save depends on how many excursions happened earlier in the current session: reloading the same save twice yields different fields, and two players loading an identical save file (same xp/gold) get different maps. This contradicts the PR's stated goal that "una misma partida sigue siendo reproducible." Persist `excursion` in `saveState()` and restore it in `loadSlot()` (and reset it in the constructor path used for new games) so the seed is a pure function of saved state.

   Fix (Persist and restore the excursion counter so the field seed is reproducible from saved state.):
   // in saveState() return object:
       return {
         name: this.name,
         player,
         location,
         excursion: this.excursion,
         ...
       }
   
   // in loadSlot(), before deriving the seed:
       this.excursion = Number(saved.excursion) || 0

2. 💡 Quality: Seed derivation formula duplicated in two places
   Files: lib/game.js:778-781, lib/game.js:1184-1187

   The identical `(this.excursion * 2654435761 + this.player.xp * 40503 + this.player.gold) >>> 0` expression appears in both `loadSlot()` and `enter()`. Duplicating the magic constants risks the two paths drifting apart. Extract a small helper (e.g. `nextFieldSeed()` that also does `this.excursion++`) and call it from both sites.

   Fix (Centralize the seed derivation so both field entry points stay in sync.):
   nextFieldSeed() {
     this.excursion++
     return (this.excursion * 2654435761 + this.player.xp * 40503 + this.player.gold) >>> 0
   }
   
   // both call sites:
   const seed = this.nextFieldSeed()

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Important

Your trial ends in 7 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

Was this helpful? React with 👍 / 👎 | Gitar

Comment thread lib/game.js
Comment on lines +1184 to +1187
this.excursion++
const seed =
(this.excursion * 2654435761 + this.player.xp * 40503 + this.player.gold) >>> 0
this.field = new Field({ player: this.player, seed })

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Bug: excursion counter isn't persisted or reset on load

The excursion field is initialized to 0 in the constructor, incremented on every field entry, but never written to saveState() nor reset in loadSlot(). As a result the derived seed after loading a save depends on how many excursions happened earlier in the current session: reloading the same save twice yields different fields, and two players loading an identical save file (same xp/gold) get different maps. This contradicts the PR's stated goal that "una misma partida sigue siendo reproducible." Persist excursion in saveState() and restore it in loadSlot() (and reset it in the constructor path used for new games) so the seed is a pure function of saved state.

Persist and restore the excursion counter so the field seed is reproducible from saved state.:

// in saveState() return object:
    return {
      name: this.name,
      player,
      location,
      excursion: this.excursion,
      ...
    }

// in loadSlot(), before deriving the seed:
    this.excursion = Number(saved.excursion) || 0
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

Comment thread lib/game.js
Comment on lines +778 to +781
this.excursion++
const seed =
(this.excursion * 2654435761 + this.player.xp * 40503 + this.player.gold) >>> 0
this.field = new Field({ script: this.scriptSource, player: this.player, seed })

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Quality: Seed derivation formula duplicated in two places

The identical (this.excursion * 2654435761 + this.player.xp * 40503 + this.player.gold) >>> 0 expression appears in both loadSlot() and enter(). Duplicating the magic constants risks the two paths drifting apart. Extract a small helper (e.g. nextFieldSeed() that also does this.excursion++) and call it from both sites.

Centralize the seed derivation so both field entry points stay in sync.:

nextFieldSeed() {
  this.excursion++
  return (this.excursion * 2654435761 + this.player.xp * 40503 + this.player.gold) >>> 0
}

// both call sites:
const seed = this.nextFieldSeed()
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

@leocagli

Copy link
Copy Markdown
Collaborator

El CI ya corrio (habilite la ejecucion, estaba esperando aprobacion de mantenedor). Dejé el detalle de todos tus PRs juntos en #50 para no repetirlo seis veces: #50

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