JBenda / inkcpp

Inkle Ink C++ Runtime with JSON>Binary Compiler
MIT License
70 stars 13 forks source link

Fully-bracketed choices followed by a single space causes an assertion error #63

Closed LilithSilver closed 9 months ago

LilithSilver commented 9 months ago

Here's a test that demonstrates the issue. If someone manages to fix this, I recommend including it as part of the commit:

inkcpp choice bracket bug.zip

But in summary:

// This is valid:
*    [A choice!]
// This is invalid:
*    [A choice!] 
JBenda commented 9 months ago

I have added the test, and it runs without problem? #64 Which version of inklecat are you using? We are using currently the v1.1.1.

LilithSilver commented 9 months ago

I just grabbed a fresh 1.1.1 from https://github.com/inkle/ink/releases, and added it to my PATH, and still broken...

However, I did just notice: The assertion failure only happens in a debug configuration. It works fine in Release mode. So can you try replicating it with Debug?

Additionally, I noticed a separate bug with tests not actually able to use the Inklecate grabbed by CMake (when I removed it elsewhere from my path, the tests wouldn't properly run). So that's interesting.

LilithSilver commented 9 months ago

An update: It's not just a space after bracketed choice, it's some type of non-outputting content in general, I think?

For example, if a temporary variable is assigned immediately after a non-bracketed choice:

-> content
== function GET_SOMETHING
~ return -1

== content

*   [Go!]
-

~ temp whoof = GET_SOMETHING()

Testing…

-> DONE

It will produce the same assertion failure (again only in Debug mode [edit: and also only from a function call]).

If it helps, the assertion error comes from STL, and this bit of code (output.cpp:138):

        // Return processed string
        // remove mulitple accourencies of ' '
        std::string result = str.str();
        if ( !result.empty() ) {
            auto end = clean_string<true, false>( result.begin(), result.end() );
            _last_char = *( end - 1 );

The error is from end - 1 on a string of " \n" (for the provided test) or "\n" (for the above Ink); the end is actually pointing to the start of the string in both cases. clean_string therefore isn't working properly in those cases; or those strings shouldn't be going into clean_string at all. I don't know which it is.

JBenda commented 9 months ago

You're right, but actually, we forgot to check if the cleaned string is not empty ... :sweat_smile:

I still do not get it to throw an assertion, which compiler and OS do you use? I would like to use it too ^^

Anyway, I pushed on #64 a fix, please try it. Since I cannot currently reproduce it.

Thank you for the investigation !!

LilithSilver commented 9 months ago

I'll give it a shot!

I'm using Visual Studio on Windows. In particular, make sure in Exception Settings, that break on all exceptions is checked (although it should error on STL asserts in debug mode by default).

In MSVC I believe it's this, or something similar: https://learn.microsoft.com/en-us/cpp/standard-library/iterator-debug-level?view=msvc-170

I'll test it out tomorrow, thank you!