search

JBenda / inkcpp

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

Fix issue related to pointer storage. #86

Closed kalmard0 closed 6 months ago

kalmard0 commented 6 months ago

I'm not sure what the "ci" field does, but I noticed while testing inkcpp on various platforms that an unexpected crash happens related to it (it having a value of 255).

After some investigation I realized that in every case other than storage, ci is treated as an int. As a matter of fact, various pieces of code expect it to be able to have the values of "255", "0" and "-1", which is impossible for an 8-bit type.

This change fixes my crash and I believe is correct.

JBenda commented 6 months ago

Thanks for spotting it, your fix looks correct. Could you provide your crash? Then I can add a new test case ^^

kalmard0 commented 6 months ago

The crash was due to an assertion in stack.cpp line 86: inkAssert(ci == -1 || ci == 0, "only support ci == -1, for now!"); It could be 100% reproduced by building inkcpp with emscripten, and playing through The Intercept always picking the first choice. It happened after 3-4 choices.

I could not reproduce it on an easy-to-debug platform. I suspect it may be possible by finding a compiler flag that changes the signedness of char.

JBenda commented 6 months ago

Ah, jea, funny that it was working until now. I'm currently thinking about using a int8_t but I think this does not matter much.

The ci is a call stack reference to distinguish between variables with the same name