Skip to content

scenario load: preserve control values across a port-set change - #2111

Open
bltzr wants to merge 2 commits into
ossia:masterfrom
bltzr:fix/preserve-values-on-port-migration
Open

scenario load: preserve control values across a port-set change#2111
bltzr wants to merge 2 commits into
ossia:masterfrom
bltzr:fix/preserve-values-on-port-migration

Conversation

@bltzr

@bltzr bltzr commented Jul 13, 2026

Copy link
Copy Markdown
Member

Problem

When an avnd process's port count changes — e.g. an add-on gains a new port in an update — opening a scenario saved with the older version reloads that process with every value reset to default (soundfile path blank, all controls back to their init values). This breaks backward-compat for any existing document once an object grows a port.

Cause

The count mismatch makes ProcessModel::check_all_ports() recreate the ports (with default values) and migrate the saved data through the 3-arg Dataflow::reloadPortsInNewProcess(). That path called loadData(old_p.data) without DontReloadValue, and ControlInlet::loadData drops the value unless that flag is set — so the recreated ports keep their defaults.

Fix

Pass Process::PortLoadDataFlags::DontReloadValue in that migration, exactly as LoadPresetCommand already does. It's guarded by the existing type()==type() && name()==name() match, so a value is only ever restored into an identical port.

This is the same fix as 6fed47b ("presets: fix that values sometimes did not reload"), applied to the scenario-load migration path rather than the preset path.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AvD2PFHZtW6pfsh3gGhayg

When an avnd process's port count changes (e.g. an updated add-on that
gained a port), opening an existing scenario runs
ProcessModel::check_all_ports(), which recreates the ports with default
values and migrates the saved data via the 3-arg reloadPortsInNewProcess().
That path called loadData() without DontReloadValue, so
ControlInlet::loadData dropped every value and the process reloaded with all
defaults (soundfile path blank, every control reset to its init value).

Pass DontReloadValue there, exactly as LoadPresetCommand already does, so a
matched port (guarded by type()==type() && name()==name()) keeps its value.
This is the same fix as 6fed47b ("presets: fix that values sometimes did
not reload"), applied to the scenario-load migration path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AvD2PFHZtW6pfsh3gGhayg
@bltzr
bltzr requested a review from jcelerier July 13, 2026 22:43

@jcelerier jcelerier left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

need to test this as there's various cases in which this could break

{
new_p->loadData(old_p.data);
// Preserve values across a port-set change (e.g. an updated add-on that
// gained a port): without DontReloadValue, loadData drops the value and

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you ask Claude to not put this kind of slop comments ? 😁

Review feedback on ossia#2111: the multi-line rationale block was excessive;
keep a single line stating the intent.

Co-Authored-By: Claude <noreply@anthropic.com>
@bltzr

bltzr commented Jul 26, 2026

Copy link
Copy Markdown
Member Author

Comment trimmed (9bf42e5).

On the cases that could break — the restore only fires inside the pre-existing positional guard (type() && name() match at the same index), and this overload is only reached from Crousti::ProcessModel::check_all_ports when the deserialized port count differs from the compiled spec; a normal load never enters it. Enumerating:

  • append at the end (the motivating case): prefix restored, new port keeps its default
  • removal/insertion in the middle, rename, reorder: name mismatch at the shifted indices → those values drop to defaults, same as before the PR — never restored into a different port
  • retype (audio/midi/message): type() mismatch → default
  • ValueInlet ↔ ControlInlet swap (both PortType::Message, so the guard passes): defused by the has_value stream bool from 03ef545ValueInlet::saveData writes has_value=false, so nothing restores
  • widget value-type change inside a same-name ControlInlet: ossia::value carries its own type tag and oscr::from_ossia_value converts-or-defaults; same exposure the preset path has had since 6fed47b, no new failure class
  • presets (already passing DontReloadValue) and ScriptEditCommand (flagless, still resets) unchanged; outlets unaffected (ControlOutlet::saveData serializes no value)

Side note found while auditing: since 03ef545 the flag reads inverted — passing DontReloadValue is what makes the saved value restore (loadData clears has_value unless the flag is present). This PR uses it consistently with LoadPresetCommand::redo, but the flag may deserve a rename in a follow-up.

@jcelerier

Copy link
Copy Markdown
Member

@claude fix the broken build due to changed flag

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