Skip to content

fix: reset temperament state on each run#5957

Open
Ady0333 wants to merge 1 commit intosugarlabs:masterfrom
Ady0333:fix/temperament-state-leak
Open

fix: reset temperament state on each run#5957
Ady0333 wants to merge 1 commit intosugarlabs:masterfrom
Ady0333:fix/temperament-state-leak

Conversation

@Ady0333
Copy link
Contributor

@Ady0333 Ady0333 commented Feb 27, 2026

fix: reset temperament state in runLogoCommands to prevent frequency corruption across runs

When a program uses Set Temperament, the temperament state persists into subsequent runs even after removing the block. This causes silent pitch corruption where notes play at wrong frequencies.

Root cause: runLogoCommands() resets changeInTemperament but not inTemperament, startingPitch, or noteFrequencies. With the flag false and temperament still set, _getFrequency() uses the stale lookup table.

Fix: Reset all three properties alongside existing BPM resets:

this.synth.inTemperament = "equal";
this.synth.startingPitch = "C4";
this.synth.noteFrequencies = {};

Signed-off-by: Ady0333 <adityashinde1525@gmail.com>
@Ady0333
Copy link
Contributor Author

Ady0333 commented Feb 27, 2026

@walterbender, @omsuneri and @vanshika2720 .

Please review this pr once

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

Copy link
Contributor

@vanshika2720 vanshika2720 left a comment

Choose a reason for hiding this comment

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

@Ady0333 The logic is correct and solves the state-leak problem.

However, from a design perspective, this would be stronger if:

  • Defaults were derived from the original initialization logic, or
  • A single reset method handled temperament state cleanly.

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