Honor new-script markers in the preprocessor

[fernewelten]

As a first attempt to mitigate #2609

The preprocessor tracks double-quoted strings that begin in the first column and start with __NEWSCRIPTSTART_. Whenever it detects such a string, it resets the line numbering and begins a new script. The new script name is whatever follows the new-script marker.

In consequence, preprocessor errors in dialogs will be reported to the Editor in such a way that you can double-click the error message to get to the dialog that has the error.

Moreover, when the preprocessor encounters an unterminated string, it will state the missing character: ' or ".

NOTE: AGS has two preprocessor files: Preprocessor.cs in the Editor project and preprocessor.cpp in the "classic" compiler project. I've tried to modify both in tandem , but the modifications in the "classic" compiler project are untested – mainly because I don't know how to set up appropriate tests for that compiler and preprocessor .

[ivan-mogilko]

That's a wrong header for "strlen", should be <string.h> instead.

Alternatively, there's std::strlen in header <cstring>.

[ivan-mogilko]

Formatting may be done using our String class, see String::Format or String::FromFormat.

like

String msg = String::FromFormat("Unterminated string: '%c' is missing", text[i]);
LogError(ErrorCode::UnterminatedString, msg);

[ivan-mogilko]

I confirm that C# Preprocessor works as expected. This seems to resolve both the problem reported by #2609 (misleading error message) and #2389 (preprocessor not reporting an actual dialog).

[ivan-mogilko]

In regards to the C++ preprocessor, the changes seem to fix the reporting of unterminated string, but neither the correct line counter, nor the section name are included into the reported error.

The _lineNumber and _scriptName that you set in code are simply not propagated further into LogError. Instead it uses currentline global variable internally, which is not adjusted here. I suppose the quick fix would be to also reset currentline where you reset _lineNumber, but I do not remember how the section name is included in the classic compiler's error struct.

Maybe this could be left out from this pr and dealt with separately.
EDIT: I double checked, it's been written into another global variable called ccCurScriptName. :(

After this is done in ags4 branch, the changes need to be backported to ags3 too.

---

In any case, following are the tests which I propose to add to preprocessor tests.
They may be added in here:
https://github.com/adventuregamestudio/ags/blob/ags4/Compiler/test/preprocessor_test.cpp

TEST(Preprocess, UnterminatedStringSingleQuote) {
    Preprocessor pp = Preprocessor();
    const char* inpl = R"EOS(
int Func1()
{
    'unterminated string
    return 0;
}
)EOS";

    clear_error();
    String res = pp.Preprocess(inpl, "UnterminatedStringSingleQuote");

    EXPECT_STREQ(last_seen_cc_error(), "Unterminated string: ' is missing");
    auto cc_err = cc_get_error();
    // script lines are 1-based, because line 0 is a NEW SCRIPT MARKER added by preproc
    EXPECT_EQ(cc_err.Line, 4);
}

TEST(Preprocess, UnterminatedStringDoubleQuote) {
    Preprocessor pp = Preprocessor();
    const char* inpl = R"EOS(
int Func1()
{
    "unterminated string
    return 0;
}
)EOS";

    clear_error();
    String res = pp.Preprocess(inpl, "UnterminatedStringDoubleQuote");

    EXPECT_STREQ(last_seen_cc_error(), "Unterminated string: \" is missing");
    auto cc_err = cc_get_error();
    // script lines are 1-based, because line 0 is a NEW SCRIPT MARKER added by preproc
    EXPECT_EQ(cc_err.Line, 4);
}

TEST(Preprocess, MultipleNewScriptMarkers) {
    Preprocessor pp = Preprocessor();
    const char* inpl = R"EOS(
\"__NEWSCRIPTSTART_Dialog1\"
int Func1()
{
    return 0;
}
\"__NEWSCRIPTSTART_Dialog2\"
int Func2()
{
    "unterminated string
    return 0;
}
\"__NEWSCRIPTSTART_Dialog3\"
int Func3()
{
    return 0;
}
)EOS";

    clear_error();
    String res = pp.Preprocess(inpl, "MultipleNewScriptMarkers");

    EXPECT_STREQ(last_seen_cc_error(), "Unterminated string: \" is missing");
    auto cc_err = cc_get_error();
    EXPECT_STREQ(ccCurScriptName, "Dialog2");
    // Line count starts from the nearest NEW SCRIPT MARKER
    EXPECT_EQ(cc_err.Line, 3);
}

[ivan-mogilko]

To summarize, the quick fix for the C++ preprocessor would be to assign 2 global variables called currentline and ccCurScriptName along with the _scriptName and _lineNumber:

// Start the new script
_scriptName = literal.Mid(strlen(NEW_SCRIPT_TOKEN_PREFIX));
_lineNumber = 0;
ccCurScriptName = _scriptName;
currentline = 0;

In the long run it would be better to refactor the code and get rid of global variables which contain error reporting details etc. Just like the new compiler does now.

[fernewelten]

Updating the PR:

• Rewrite preprocessor.cpp so that it sets currentlineand ccCurScriptName immediately before calling cc_error() and only there, to facilitate a later rewriting of the class for another error reporting mechanism.
• Include @ivan-mogilko 's Googletests
• Use AGS String and String::FromFormat to format the error messages, as per @ivan-mogilko 's suggestion.
• Fix the #include

[ivan-mogilko]

I forgot to mention previously, but I pushed a fix to the ags4 branch, and you do not need a static buffer for the script name anymore:
3facb6a

This may be removed if you rebase on top of the ags4 branch.

[ivan-mogilko]

Please fix this other way around, make "i" variable be of size_t type instead.

[fernewelten]

Done. (Had to force-push due to the rebase)