Correctly parse tags related to each line (#96)

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.

# 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.
- keep track of global tags and current knot/stitch tags
    - knot tags are tags in the current knot/stitch before the first line of ink code
    - tags from parent knots are not stored!
- function to fetch current knot/stitch hash to detect knot/stitch changes
- UE: new event: EnterKnot(GlobalTags, KnotTags)

# 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.

---------

Co-authored-by: Julian Benda <Julian.m.benda@gmail.com>

Author: MrHands

.clang-format

@@ -2,11 +2,34 @@
 AccessModifierOffset: -2
 AlignAfterOpenBracket: BlockIndent
 AlignArrayOfStructures: Right
-# readd padOperation if bumped from clang 14
-AlignConsecutiveAssignments: AcrossComments
-AlignConsecutiveBitFields: AcrossComments
-AlignConsecutiveDeclarations: AcrossComments
-AlignConsecutiveMacros: AcrossComments
+AlignConsecutiveAssignments:
+  Enabled: true
+  AcrossEmptyLines: false
+  AcrossComments: true
+  AlignCompound: false
+  AlignFunctionPointers: false
+  PadOperators: true
+AlignConsecutiveBitFields:
+  Enabled: true
+  AcrossEmptyLines: false
+  AcrossComments: true
+  AlignCompound: false
+  AlignFunctionPointers: false
+  PadOperators: true
+AlignConsecutiveDeclarations:
+  Enabled: true
+  AcrossEmptyLines: false
+  AcrossComments: true
+  AlignCompound: false
+  AlignFunctionPointers: false
+  PadOperators: true
+AlignConsecutiveMacros:
+  Enabled: true
+  AcrossEmptyLines: false
+  AcrossComments: true
+  AlignCompound: false
+  AlignFunctionPointers: false
+  PadOperators: true
 AlignEscapedNewlines: Left
 AlignOperands: AlignAfterOperator
 AlignTrailingComments: true
@@ -25,7 +48,6 @@ AlwaysBreakTemplateDeclarations: Yes
 BinPackArguments: true
 BinPackParameters: true
 BreakBeforeBraces: Linux
-# BreakAfterAttributes: Always
 BreakBeforeBinaryOperators: All
 UseTab: ForIndentation
 Standard: Latest
@@ -58,9 +80,6 @@ SortUsingDeclarations: false
 SortIncludes: Never
 ShortNamespaceLines: 0
 SeparateDefinitionBlocks: Always
-# RequiresExpressionIndentation: OuterScope
-# BreakBeforeConceptDeclarations: Always
-# BreakBeforeInlineASMColon: Always
 BreakBeforeTernaryOperators: true
 BreakConstructorInitializers: BeforeComma
 BreakInheritanceList: BeforeComma
@@ -77,23 +96,17 @@ IndentCaseLabels: true
 IndentExternBlock: Indent
 IndentGotoLabels: false
 IndentPPDirectives: AfterHash
-# IndentRequiresClause: true
 IndentWidth: 2
 TabWidth: 2
 IndentWrappedFunctionNames: true
-# InsertBraces: true
-# InsertNewlineAtEOF: true
 KeepEmptyLinesAtTheStartOfBlocks: true
 LambdaBodyIndentation: Signature
 Language: Cpp
-# LineEnding: LF
 MaxEmptyLinesToKeep: 2
 NamespaceIndentation: Inner
 QualifierAlignment: Left
 ReferenceAlignment: Left
 ReflowComments: true
 RemoveBracesLLVM: false
-# RemoveSemicolon: false
-# RequiresClausePosition: OwnLine
 UseCRLF: false
 DeriveLineEnding: false

.editorconfig

@@ -0,0 +1,9 @@
+root = true
+
+[*]
+guidelines = 100
+indent_style = tab
+indent_size = 4
+# Unix-style newlines
+end_of_line = lf
+insert_final_newline = true

.github/workflows/build.yml

@@ -117,9 +117,9 @@ jobs:
       working-directory: ${{github.workspace}}/build
       shell: bash
       run: |
-        cmake $GITHUB_WORKSPACE -DINKCPP_UNREAL_TARGET_VERSION="5.5"
+        cmake $GITHUB_WORKSPACE -DINKCPP_UNREAL_TARGET_VERSION="5.5" -DINKCPP_UNREAL=ON
         cmake --install . --config $BUILD_TYPE --prefix comp_unreal_5_5 --component unreal
-        cmake $GITHUB_WORKSPACE -DINKCPP_UNREAL_TARGET_VERSION="5.4"
+        cmake $GITHUB_WORKSPACE -DINKCPP_UNREAL_TARGET_VERSION="5.4" -DINKCPP_UNREAL=ON
         cmake --install . --config $BUILD_TYPE --prefix comp_unreal_5_4 --component unreal
     - name: Upload UE 5.5
       if: ${{ matrix.unreal }}
@@ -287,12 +287,12 @@ jobs:
           git fetch origin master
       - name: Check clang-format
         run: |
+          git clang-format-18 --extensions c,cpp,h,hpp --style file -q --diff origin/master
           diff=$(git clang-format-18 --extensions c,cpp,h,hpp --style file -q --diff origin/master)
-          echo $diff
+          echo "::error Format diff >$diff<"
           if [ "$diff" != "" ]; then
-            echo run git clang-format --extensions c,cpp,h,hpp --style file master
+            echo run git clang-format-18 --extensions c,cpp,h,hpp --style file master
             echo or upstream/master depending on your setup
-            clang
           fi
 
   reporting:

.gitignore

@@ -22,4 +22,4 @@ Bin/
 
 # Visual Studio
 /out/build/
-/.vs
+.vs

CMakeLists.txt

@@ -7,17 +7,17 @@ include(FetchContent)
 FetchContent_Declare(inklecate_mac
 URL https://github.com/inkle/ink/releases/download/v1.1.1/inklecate_mac.zip
 URL_HASH SHA256=c516402bca5fa249a7712e62591b048b137eba3098c53f9fb85a4253f9b9e2c0
-SOURCE_DIR "inklecate/mac"
+SOURCE_DIR "${CMAKE_BINARY_DIR}/inklecate/mac"
 )
 FetchContent_Declare(inklecate_windows
 URL https://github.com/inkle/ink/releases/download/v1.1.1/inklecate_windows.zip
 URL_HASH SHA256=6f317cb4c59bf1b31c6dd61e80c6a2287a1d8c241a703f0586f736ae00871aab
-SOURCE_DIR "inklecate/windows"
+SOURCE_DIR "${CMAKE_BINARY_DIR}/inklecate/windows"
 )
 FetchContent_Declare(inklecate_linux
 URL https://github.com/inkle/ink/releases/download/v1.1.1/inklecate_linux.zip
 URL_HASH SHA256=26f4e188e02536d6e99e73e71d9b13e2c2144187f1368a87e82fd5066176cff8
-SOURCE_DIR "inklecate/linux"
+SOURCE_DIR "${CMAKE_BINARY_DIR}/inklecate/linux"
 )
 set(FETCHCONTENT_QUIET OFF)
 mark_as_advanced(FETCHCONTENT_QUIET)
@@ -41,7 +41,7 @@ mark_as_advanced(PYBIND11_USE_CROSSCOMPILING)
 enable_testing()
 
 # Project setup
-project(inkcpp VERSION 0.1.7)
+project(inkcpp VERSION 0.1.8)
 SET(CMAKE_CXX_STANDARD 20)
 SET(CMAKE_CXX_STANDARD_REQUIRED ON)
 SET(CMAKE_INSTALL_LIBRARY_DIR lib)

Documentation/unreal/InkCPP_DEMO.zip

@@ -10,6 +10,8 @@
 #include "system.h"
 #include "traits.h"
 
+#include <limits>
+
 namespace ink::runtime::internal
 {
 template<typename T, bool dynamic, size_t initialCapacity>
@@ -26,7 +28,7 @@ class managed_array : public snapshot_interface
 		}
 	}
 
-	~managed_array()
+	virtual ~managed_array()
 	{
 		if constexpr (dynamic) {
 			delete[] _dynamic_data;
@@ -78,12 +80,37 @@ class managed_array : public snapshot_interface
 				extend();
 			}
 		} else {
-			inkAssert(_size <= _capacity, "Stack Overflow!");
-			/// FIXME silent fail!!
+			inkAssert(_size <= _capacity, "Try to append to a full array!");
+			// TODO(JBenda): Silent fail?
 		}
 		return data()[_size++];
 	}
 
+	virtual T& insert(size_t position)
+	{
+		inkAssert(
+		    position <= _size,
+		    "Array must be dense, cannot insert value at position larger then array.size."
+		);
+		push();
+		if (_size >= 2) {
+			for (size_t i = _size - 2; i >= position && i < std::numeric_limits<size_t>::max(); --i) {
+				data()[i + 1] = data()[i];
+			}
+		}
+		return data()[position];
+	}
+
+	virtual void remove(size_t begin, size_t end)
+	{
+		inkAssert(end <= _size, "can not delete behind end of array.");
+		inkAssert(begin <= end, "can not remove negative range.");
+		for (size_t i = 0; i < (end - begin) && end + i < _size; ++i) {
+			data()[begin + i] = data()[end + i];
+		}
+		_size -= end - begin;
+	}
+
 	void clear() { _size = 0; }
 
 	void resize(size_t size)
@@ -154,7 +181,23 @@ class managed_restorable_array : public managed_array<T, dynamic, initialCapacit
 	{
 	}
 
-	void restore() { base::resize(_last_size); }
+	virtual T& insert(size_t position) override
+	{
+		inkAssert(position >= _last_size, "Cannot insert data before last save point.");
+		return base::insert(position);
+	}
+
+	virtual void remove(size_t begin, size_t end) override
+	{
+		inkAssert(begin >= _last_size, "Cannot delete data before last save point.");
+		base::remove(begin, end);
+	}
+
+	void restore()
+	{
+		base::resize(_last_size);
+		_last_size = 0;
+	}
 
 	void save() { _last_size = this->size(); }
 
@@ -287,7 +330,6 @@ class basic_restorable_array : public snapshot_interface
 	const T _null;
 };
 
-
 template<typename T>
 inline void basic_restorable_array<T>::set(size_t index, const T& value)
 {

inkcpp/array.h

@@ -7,48 +7,62 @@
 #include "choice.h"
 
 #include "output.h"
+#include "snapshot_impl.h"
 #include "string_table.h"
 #include "string_utils.h"
 
-namespace ink {
-namespace runtime {
-	choice& choice::setup( internal::basic_stream& in, internal::string_table& strings, internal::list_table& lists, int index, uint32_t path, thread_t thread, const char* const* tags )
+namespace ink
+{
+namespace runtime
+{
+
+	size_t choice::num_tags() const { return std::distance(_tags_start, _tags_end); }
+
+	const char* choice::get_tag(size_t index) const
+	{
+		return (index < num_tags()) ? _tags_start[index].text() : nullptr;
+	}
+
+	choice& choice::setup(
+	    internal::basic_stream& in, internal::string_table& strings, internal::list_table& lists,
+	    int index, uint32_t path, thread_t thread, const internal::snap_tag* tags_start,
+	    const internal::snap_tag* tags_end
+	)
 	{
+		// Index/path
+		_index      = index;
+		_path       = path;
+		_thread     = thread;
+		_tags_start = tags_start;
+		_tags_end   = tags_end;
+
 		char* text = nullptr;
 		// if we only have one item in our output stream
-		if ( in.queued() == 2 )
-		{
+		if (in.queued() == 2) {
 			// If it's a string, just grab it. Otherwise, use allocation
 			const internal::value& data = in.peek();
-			switch ( data.type() )
-			{
-			case internal::value_type::string:
-				text = strings.duplicate( data.get<internal::value_type::string>() );
-				in.discard( 2 );
-				break;
-			default:
-				text = in.get_alloc( strings, lists );
+			switch (data.type()) {
+				case internal::value_type::string:
+					text = strings.duplicate(data.get<internal::value_type::string>());
+					in.discard(2);
+					break;
+				default: text = in.get_alloc(strings, lists);
 			}
-		}
-		else
-		{
+		} else {
 			// Non-string. Must allocate
-			text = in.get_alloc( strings, lists );
+			text = in.get_alloc(strings, lists);
 		}
+
 		char* end = text;
-		while ( *end )
-		{
+		while (*end != '\0') {
 			++end;
 		}
-		end   = ink::runtime::internal::clean_string<true, true>( text, end );
-		*end  = 0;
+		end  = ink::runtime::internal::clean_string<true, true>(text, end);
+		*end = '\0';
+
 		_text = text;
-		// Index/path
-		_index  = index;
-		_path   = path;
-		_thread = thread;
-		_tags   = tags;
+
 		return *this;
 	}
-}
-}   // namespace ink::runtime
+} // namespace runtime
+} // namespace ink