Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
LilithSilver opened this issue Dec 12, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@LilithSilver
Copy link
Contributor

LilithSilver commented Dec 12, 2023

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 JBenda added the bug Something isn't working label Dec 12, 2023
@JBenda
Copy link
Owner

JBenda commented Dec 12, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor Author

LilithSilver commented Dec 13, 2023

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
Copy link
Owner

JBenda commented Dec 13, 2023

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

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
Copy link
Contributor Author

LilithSilver commented Dec 13, 2023

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants