Runner doesn't break on newlines between knots/stitches

[LilithSilver]

When using the function std::string runner::getline(), it should only go line-by-line. But currently, running the following ink:

Line 1

Line 2

-> line_3

== line_3

Line 3

-> line_4

= line_4

Line 4

-> DONE

... produces this output:

> Line 1\n
> Line 2\nLine 3\nLine 4\n

It seems knots and stitches don't properly cause the runner to break, even if they correctly generate a newline.

To confirm, I tested this Ink on the latest Inklecate and C# Runtime API, and using Continue() produced the correct behavior of:

> Line 1\n
> Line 2\n
> Line 3\n
> Line 4\n

I tried to fix this myself, but I'm not familiar enough with the internals of inkcpp or Ink to understand where the issue is. But, in the process, I made a test that could be helpful to anyone wanting to tackle this:

LinesTest.zip

[JBenda]

Good to know, thank you for providing a test case ^^ let take a look in these mysteries.

[JBenda]

Heureka? It seems like it was a confusion between return and break in the step function. I will open the PR in the next days (just to verify that the change is right)

[LilithSilver]

I found a related bug. It seems that the same issue appears with functions:

Function Line

~ funcTest()

-> DONE


=== function funcTest()
Function Result

This will output:

> Function Line\nFunction Result\n

Instead of:

> Function Line\n
> Function Result\n

I have attached a modified NewLines.cpp and LinesStory.ink for test purposes. I also added a tunnel test just in case, although tunnels seem to be working fine.

EDIT: tests.zip

Stepping through the code, I've identified that it's not a regression from #45; it was broken even when change_type::no_change was return false instead of break. However, I think the prior line as forget() is the issue. The function pointer is a no_change operation, but should still be restore()'d. The reason the function causes the failure is that value_type::func_start is a no-change operation that isn't a newline, so the code forgets the old save state, and then never even saves a new save-state in the following if-statement. Thus the next line is appended as usual, and restore() is called after that second line.

If I remove that forget() statement, all the tests pass, including my new ones; i.e. the save state properly restores to the correct location. Thus, the new lookahead determination code becomes:

bool runner_impl::line_step()
{
    ...
    case change_type::no_change:
	    break;
    ...
}

@JBenda does that seem correct? If so, let me know; I can create a PR for that change. That said, I might be totally off the mark; I'm still new to the codebase!

[JBenda]

Thanks for the digging, it seems like a miss placement of some forget/save thing, but just removing the forged leads to wrong visit count from look aheads. So more likely something else in wrong, maybe we also need a new case? (don't think so currently)

Removing the forget at no_changes looks like a good starting point.

Look at #48 for an implemented version of your new test, feel free to also start your own PR based on this. The missing counting error was located when running ink-proof

[JBenda]

#48 seemst to work now, the forget was missplaced

no_chache should not trigger a forget (as you descripetd)

but if we encounter a second newline (through tunnels or multiple glues) we need to move the save state with us to restore to the correct location, for that detecting a new line needs to forget the old save state and create a new one

[LilithSilver]

Awesome, thankye.