Skip to content

Conversation

@tvararu
Copy link
Contributor

@tvararu tvararu commented Nov 3, 2025

Fixes #912.

Checklist

  • The new code additions passed the tests (npm test).
  • The linter ran and found no issues (npm run-script lint).

Description

This preserves the isInvisibleDefault flag by adding it to serialisation and deserialisation.

Add a regression test to ensure it works correctly. I've also tested this manually on an actual story.

When the story runs normally, Story.ProcessChoice copies the runtime ChoicePoint.isInvisibleDefault into each generated Choice:

let choice = new Choice();
choice.targetPath = choicePoint.pathOnChoice;
choice.sourcePath = choicePoint.path.toString();
choice.isInvisibleDefault = choicePoint.isInvisibleDefault; // <-- Flag
choice.threadAtGeneration = this.state.callStack.ForkThread();
choice.tags = tags.reverse(); //C# is a stack
choice.text = (startText + choiceOnlyText).replace(/^[ \t]+|[ \t]+$/g, "");

return choice;

Story.currentChoices filters those with isInvisibleDefault:

get currentChoices() {
  let choices: Choice[] = [];

  if (this._state === null) {
    return throwNullException("this._state");
  }
  for (let c of this._state.currentChoices) {
    if (!c.isInvisibleDefault) { // <-- Filtering out invisible choices
      c.index = choices.length;
      choices.push(c);
    }
  }

  return choices;
}

JsonSerialisation uses WriteChoice to store the game state, but that payload omits isInvisibleDefault:

// No invisible flag anywhere
public static WriteChoice(writer: SimpleJson.Writer, choice: Choice) {
  writer.WriteObjectStart();
  writer.WriteProperty("text", choice.text);
  writer.WriteIntProperty("index", choice.index);
  writer.WriteProperty("originalChoicePath", choice.sourcePath);
  writer.WriteIntProperty("originalThreadIndex", choice.originalThreadIndex);
  writer.WriteProperty("targetPath", choice.pathStringOnChoice);
  this.WriteChoiceTags(writer, choice);
  writer.WriteObjectEnd();
}

On reload, JObjectToChoice reconstructs the choice instances, without the flag:

// No invisible flag anywhere
public static JObjectToChoice(jObj: Record<string, any>) {
  let choice = new Choice();
  choice.text = jObj["text"].toString();
  choice.index = parseInt(jObj["index"]);
  choice.sourcePath = jObj["originalChoicePath"].toString();
  choice.originalThreadIndex = parseInt(jObj["originalThreadIndex"]);
  choice.pathStringOnChoice = jObj["targetPath"].toString();
  choice.tags = this.JArrayToTags(jObj);
  return choice;
}

That hits the same currentChoices method as above, but now it doesn't filter anything out.

Fixes y-lohse#912.

This preserves the `isInvisibleDefault` flag by adding it to
serialisation and deserialisation.

Add a regression test to ensure it works correctly. I've also tested
this separately manually on an actual story.

When the story runs normally, `Story.ProcessChoice` copies the runtime
`ChoicePoint.isInvisibleDefault` into each generated `Choice`:

```ts
let choice = new Choice();
choice.targetPath = choicePoint.pathOnChoice;
choice.sourcePath = choicePoint.path.toString();
choice.isInvisibleDefault = choicePoint.isInvisibleDefault; // <-- Flag
choice.threadAtGeneration = this.state.callStack.ForkThread();
choice.tags = tags.reverse(); //C# is a stack
choice.text = (startText + choiceOnlyText).replace(/^[ \t]+|[ \t]+$/g, "");

return choice;
```

`Story.currentChoices` filters those with `isInvisibleDefault`:

```ts
get currentChoices() {
  let choices: Choice[] = [];

  if (this._state === null) {
    return throwNullException("this._state");
  }
  for (let c of this._state.currentChoices) {
    if (!c.isInvisibleDefault) { // <-- Filtering out invisible choices
      c.index = choices.length;
      choices.push(c);
    }
  }

  return choices;
}
```

`JsonSerialisation` uses `WriteChoice` to store the game state, but that
payload omits `isInvisibleDefault`:

```ts
// No invisible flag anywhere
public static WriteChoice(writer: SimpleJson.Writer, choice: Choice) {
  writer.WriteObjectStart();
  writer.WriteProperty("text", choice.text);
  writer.WriteIntProperty("index", choice.index);
  writer.WriteProperty("originalChoicePath", choice.sourcePath);
  writer.WriteIntProperty("originalThreadIndex", choice.originalThreadIndex);
  writer.WriteProperty("targetPath", choice.pathStringOnChoice);
  this.WriteChoiceTags(writer, choice);
  writer.WriteObjectEnd();
}
```

On reload, `JObjectToChoice` reconstructs the choice instances, without
the flag:

```ts
// No invisible flag anywhere
public static JObjectToChoice(jObj: Record<string, any>) {
  let choice = new Choice();
  choice.text = jObj["text"].toString();
  choice.index = parseInt(jObj["index"]);
  choice.sourcePath = jObj["originalChoicePath"].toString();
  choice.originalThreadIndex = parseInt(jObj["originalThreadIndex"]);
  choice.pathStringOnChoice = jObj["targetPath"].toString();
  choice.tags = this.JArrayToTags(jObj);
  return choice;
}
```

That hits the same `currentChoices` method as above, but now it doesn't
filter anything out.
@smwhr
Copy link
Collaborator

smwhr commented Nov 3, 2025

I have reported the issue and linked to this fix over in the inkle/ink repo, since it must be happening in the reference implementation also.

@tvararu
Copy link
Contributor Author

tvararu commented Nov 4, 2025

Thanks @smwhr! I haven’t touched C# in 7 years but I’ll see if I have time to open a PR on the main repo.

tvararu added a commit to tvararu/ink that referenced this pull request Nov 4, 2025
Fixes inkle#959.

This preserves the `isInvisibleDefault` flag by adding it to
serialisation and deserialisation.

Add a regression test to ensure it works correctly. I haven't tested the
C# version, but I did test this as part of [my PR to
inkjs](y-lohse/inkjs#1130).

That PR contains more details about the exact analysis of the bug; I
haven't duplicated it here as it's basically the same.

All tests pass:

```sh
Starting test execution, please wait...
A total of 1 test files matched the specified pattern.

Passed!  - Failed:     0, Passed:   358, Skipped:     0, Total:   358, Duration: 535 ms
```
@smwhr smwhr merged commit 2217c7a into y-lohse:master Nov 5, 2025
1 check passed
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.

Fallback choices become visible after reloading the story state

2 participants