Correctly parse tags related to each line

[MrHands]

My major goal with this pull request was to support retrieving the correct tags per line, and I've succeeded with that. Unfortunately, I also broke several existing tests in the process and haven't been able to get them working yet. I don’t think it’s very nice of me to leave your project in a worse state than I found it, so I’m providing you with as much documentation as possible about where I suspect the problems lie. I hope we can work together to integrate my changes into your main branch.

Major issues

While all tests passed for adding tags to lines and parsing global tags, my changes caused 12 failed assertions and three failed test cases in the existing unit and integration tests. This is because my changes require looking ahead to the following line to see if we have glue or choices that could affect the current line. But this spawns several edge cases I tried to handle as they popped up from testing, and the logic is now very complicated. I suspect there is a much cleaner solution, but I just can’t see it yet. Here are the major issues that came up in my testing.

Glue is not parsed correctly across multiple lines

Specifically, the first glue command is handled correctly, but subsequent commands are not processed as part of the same line. This results in breakages like these:

Scenario: Observer
      Given: a story which changes variables
       When: Run without observers

C:\Projects\inkcpp\inkcpp_test\Observer.cpp(25): FAILED:
  CHECK( thread->getall() == "hello line 1 1 hello line 2 5 test line 3 5\n" )
with expansion:
  "hello line 1 1 hello line 2 5
  test line 3 5
  "
  ==
  "hello line 1 1 hello line 2 5 test line 3 5
  "

I suspect this is caused by my changes to read ahead in the step() method. It has proven very tricky to read ahead enough to check if the following command is part of the current line while not accidentally calling functions on the next line.

External functions assert due to command buffer depletion

Again, I suspect this is caused by my changes to look ahead in the command buffer. Here’s what the failure looks like:

C:\Projects\inkcpp\inkcpp_test\ExternalFunctionsExecuteProperly.cpp(26): FAILED:
  {Unknown expression after the reported line}
due to unexpected exception with message:
  fallback function but no function call before?

This assertion triggers on line 1129 in my changes, but I suspect the problem occurred much earlier. I suspect that the look-ahead already executed the function, and it cannot be found again on line 1228:

} else if (_output.saved() && _output.ends_with(value_type::newline, _output.save_offset()) && ! fn->lookaheadSafe()) {

Changelog

• choice class: Tags are stored as snap_tag* held by the runner_impl instead of const char*.
• runner interface: Added virtual num_global_tags() and get_global_tag() methods.
• runner interface: Added line_type for std::string or FString depending on compile settings.
• runner interface: Simplified getline() method to return line_type.
• basic_stream: Reimplemented entries_since_marker() as find_first_of() and find_last_of() methods.
• basic_stream: Removed saved_ends_with() method in favor of an offset parameter to the ends_with() method.
• basic_stream: Use size_t instead of int for offsets into the data.
• basic_stream: Protect against buffer underflow in the discard() method.
• basic_stream: Added npos constexpr for invalid positions in the buffer.
• runner_impl: Added add_tag() and clear_tags() methods to manage tag info.
• runner_impl: advance_line() clears line and choice tags while keeping the global ones.
• runner_impl: line_step() reads steps from the execution buffer until the next new line.
• runner_impl: line_step() copies global tags to the first line.
• runner_impl: line_step() continues stepping execution until it's satisfied it doesn't affect the current line.
• runner_impl: Added set_debug_enabled() to optionally write command info to a debug stream as the instance steps through the execution buffer.
• runner_impl: step() writes debug info to the debug stream, if available.
• runner_impl: Streamlined getline() implementation to work with either std::string or FString.
• runner_impl: Streamlined getall() implementation to call getline() multiple times, reducing the need to test this method separately.
• runner_impl: Simplified detect_change() to check for extending past the newline only.
• command: Added std::ostream operator for streaming Command and CommandFlag enum values as strings.
• snapshot_impl: Added text() method to retrieve stored string directly.
• snapshot_impl: Store pointers to tags start and end instead of text pointers.

Unit test changes

• Replaced many REQUIRE() calls with CHECK() to ensure test execution isn’t halted immediately.
• Tags.cpp: Added unit tests for adding and removing tags to a runner.
• Tags.cpp: Made loaded story global to ensure that WHEN() and THEN() tests can be run in sequence correctly.
• NoEarlyTags.cpp: Added checks for specific tags after each line.
• NewLines.cpp: Cleaned up tests.
• TagsStory.ink: Expanded tests for tags in different places.
• LinesStory.ink: Added a condensed test case for ignoring functions when applying glue.

[JBenda]

Works under windows, fails differently under mac and Linux ... I will look into this soon, if you have an idea, feel free to check

[MrHands]

Thanks so much for iterating on my PR! Although I only have access to Windows machines, I'm familiar with the peculiarities of Linux and Clang. I'll see what I can do. 😊

[JBenda]

This was stupid, it seems MSVC and clang have different execution order policies
The problematic line was x.insert(i) = x[i+d]
with MSVC it was first evaluated x[i+d] and then x.insert(i), clang with Unix has done it the opposite, which resulted in copying the wrong element.

Would you mind checking if this PR still looks correct ^^ If you give your ok I would bump the version and bump the version to 0.1.8

(When I finished a basic UE binding [and tested it])

[MrHands]

I'm happy to confirm your changes work perfectly for my game!

LGTM, let's ship it!! 🫡

[JBenda]

I tested it today a bit and noticed that the knot tracking fails if you jump to a stitch of another knot (you keep your current knot tags)
I think instead of knot tags we should track stitch/knot tags (whatever we are currently in) This is after a short feedback round the most intuitive functionality and is easier to implement because knot tags (without stitches) introduce many corner cases, like jump to the stitch, but missing the knot tags from above etc.

[JBenda]

Implemented the solution, added a get_current_knot() function which returns the ink::hash_string of the current knot/stitch

TODO:

• implement get_current_knot() for python and C
• change documentation to underline that get_knot... actually means knots and stitches