[0.17] Tags aren't processed correctly

[MrHands]

NOTE: I'm already working on a pull request to address this issue.

I have a straightforward Ink program called "test.ink":

# first tag
This is the first line

# second tag
This is the second line

Here's a sample program for loading a binary compiled with inkcpp 0.17:

#include <ink/runner.h>
#include <ink/story.h>
#include <iostream>

int main(int argc, char** argv)
{
	std::cout << "Loading \"test.bin\"" << std::endl;

	try
	{
		auto story = ink::runtime::story::from_file("test.bin");
		auto runner = story->new_runner();

		std::cout << "line: " << runner->getline();
		std::cout << "tags: " << runner->num_tags() << std::endl;
		for (size_t i = 0; i < runner->num_tags(); ++i)
		{
			std::cout << "\t" << i << ": \"" << runner->get_tag(i) << "\"" << std::endl;
		}

		std::cout << "line: " << runner->getline();
		std::cout << "tags: " << runner->num_tags() << std::endl;
		for (size_t i = 0; i < runner->num_tags(); ++i)
		{
			std::cout << "\t" << i << ": \"" << runner->get_tag(i) << "\"" << std::endl;
		}
	}
	catch (std::exception& e)
	{
		std::cerr << e.what() << std::endl;
	}

	return 0;
}

This is the result I get from this test program:

Loading "test.bin"
line: This is the first line
tags: 1
        0: "first tag"
line: This is the second line
tags: 2
        0: "first tag"
        1: "second tag"

However, when I load the same "test.ink" file in Inky 0.15.1 (Ink 1.2.0), this is the result I get:

This is the first line

# first tag

This is the second line

# second tag

Reading through the relevant code, an assumption was made that tags are only cleared when a choice is encountered. This is incorrect, as outlined in the relevant docs for Ink:

Tags for a line can be written above it, or on the end of the line:
# the first tag
# the second tag
This is the line of content. # the third tag
All of the above tags will be included in the currentTags list.

[JBenda]

I forgot about this quirk, but here's what's happening:

Currently, when using runner->num_tags(), it returns all tags encountered since the last choice, to support the usage of runne->getall().

Current workaround: You can work around this by keeping track of how many new tags are added, but this is not ideal.

It would be better if inky's behavior were the default for both num_tags() and get_tag().
The current behavior could be renamed to something like get_all_tags() to clarify its function.

Note: Keep in mind that dealing with snapshots can be tricky.

I'm looking forward to your PR!

#include <ink/runner.h>
#include <ink/story.h>
#include <iostream>

int main(int argc, char** argv)
{
	std::cout << "Loading \"test.bin\"" << std::endl;

	try
	{
		auto story = ink::runtime::story::from_file("test.bin");
		auto runner = story->new_runner();
		int num_tags = 0; // must be reset to 0 after each choice

		std::cout << "line: " << runner->getline();
		std::cout << "tags: " << (runner->num_tags() - num_tags) << std::endl;
		for (size_t i = num_tags; i < runner->num_tags(); ++i)
		{
			std::cout << "\t" << i << ": \"" << runner->get_tag(i) << "\"" << std::endl;
		}
		num_tags = runner->num_tags();

		std::cout << "line: " << runner->getline();
		std::cout << "tags: " << (runner->num_tags() - num_tags) << std::endl;
		for (size_t i = num_tags; i < runner->num_tags(); ++i)
		{
			std::cout << "\t" << (i - num_tags) << ": \"" << runner->get_tag(i) << "\"" << std::endl;
		}
		num_tags = runner->num_tags();
	}
	catch (std::exception& e)
	{
		std::cerr << e.what() << std::endl;
	}

	return 0;
}

For UE, getline() does clear the tags after each line code

The C implementation does it, not code (oversight on my end)

Thanks for the refactoring you are doing alongside the PR.

[MrHands]

Thank you for your quick response!

Unfortunately, your solution doesn't work for my game. I'm using Ink to power opponent reactions, which I can script based on the game's state. I create an ink::runtime::runner for my script and then use move_to() to start processing lines. My real script looks more like this:

= react(-> _next)
{ getNextReaction() % 3:
- 2:
    # actorCue --id opponent --expression angry
    "That all you got?"
- 1:
    # actorCue --id opponent --expression shocked
    "Hnngg..."
- else:
    # actorCue --id opponent --expression blush
    "!!"
}
-> _next

As you can see, there are no choices in this knot. This is a problem because the tags will not reset the next time the runner enters the same knot. I had the same idea to work around the issue by counting only the "unique" tags, but it's simply not a workable solution. I will address all these problems with my PR and ensure I don't break the existing implementation of getall() with more unit tests, if necessary.

Thank you for a great library, and I hope I can make it a little bit better. ;)

[MrHands]

Since it's been a few days, here's an update. All tests now pass for the TagsStory.ink script. 😁

Unfortunately, I broke some other tests in the process, so I will have to fix those before submitting my PR. I also broke the saving and loading of the tags state, so I will need to look into that, too. Overall, this was quite a fun challenge, and I expect my PR to be ready this Sunday.

You can check my current changes in the fix/tags branch: https://github.com/MrHands/inkcpp/tree/fix/tags

[JBenda]

I like your idea of the different tag_levels this will improve the code's readability.

Also, the new introduced functions make the code much more readable, hopefully it still works ^^

I'm looking forward to testing the debug logs you added.

[JBenda]

Fixed in #96