scriptcomp: add initializers to for, while, switch statements

[tinygiant98]

This change adds the ability to use initialization and arbitrary statements within for, while and switch statements. Any variables defined or initialized within a for, while or switch statement will be scoped within the associated compound statement and any child scopes.

All legacy constructs are supported. Initialization statements are optional in for, while or switch statements.

Testing

Simple initializer in for statement:

    for (int i = 0; i < 5; i++)
    {
        Debug(c("    > iteration " + _i(i), COLOR_GREEN_LIGHT));
    }

Initializer/declaration list and comma-separated increment statements in for statement:

    for (int x, y = 10, z = -1; y > z; y--, z++, x++)
    {
        Debug(c("    > iteration " + _i(x), COLOR_GREEN_LIGHT));
        Debug(c("      > x = " + _i(x) + ", y = " + _i(y) + ", z = " + _i(z), COLOR_RED_LIGHT));
    }

Function calls in initializer and increments statements in for statement:

    for (object o = GetFirstPC(); GetIsObjectValid(o); o = GetNextPC())
    {
        Debug(c("    > pc found -> " + GetName(o), COLOR_GREEN_LIGHT));
    }

Function calls in nested for loops:

    for (object oArea = GetFirstArea(); GetIsObjectValid(oArea); oArea = GetNextArea())
    {
        Debug(c("    > area found -> " + GetName(oArea), COLOR_GREEN_LIGHT));
        for (object o = GetFirstObjectInArea(oArea); GetIsObjectValid(o); o = GetNextObjectInArea(oArea))
        {
            Debug(c("      > object found -> " + GetName(o) + " [" + GetTag(o) + "]", COLOR_ORANGE));
            if (GetObjectType(o) == OBJECT_TYPE_CREATURE)
            {
                for (effect e = GetFirstEffect(o); GetIsEffectValid(e); e = GetNextEffect(o))
                {
                    Debug(c("        > effect found -> " + _i(GetEffectType(e)), COLOR_RED_LIGHT));
                }
            }
        }
    }

Arbitrary statements in initializers and incrementers in for statement:

    int x, y;
    object o = GetFirstPC();
    for (SendMessageToPC(o, "    Loop Init"), y = 10; x < y; SendMessageToPC(o, "        Loop Iteration"), x++, y--)
    {
        SendMessageToPC(o, "      Loop Body");
        SendMessageToPC(o, "        x = " + _i(x) + ", y = " + _i(y));
    }

Simple initializer in while statement:

    while (int x = 7; x > 0)
    {
        Debug(c("    > x = " + _i(x--), COLOR_GREEN_LIGHT));
    }

Function call in initializer for switch statement

    // SWITCH_INIT = 5
    switch (int x = GetLocalInt(GetFirstPC(), "SWITCH_INIT"); x)
    {
        case 0: Debug(c("    > 0")); break;
        case 1: Debug(c("    > 1")); break;
        case 2: Debug(c("    > 2")); break;
        case 3: Debug(c("    > 3")); break;
        case 4: Debug(c("    > 4")); break;
        case 5: Debug(c("    > 5")); break;
    }

Changelog

Added

• Added initializer statements to for, while and switch statements.
• Added comma-separated increment statements to for statements.

Licence

• I am licencing my change under the project's MIT licence, including all changes to GPL-3.0 licenced parts of the codebase.

[Daztek]

How does it handle nested loops that use the same variable name? Are they scoped properly?

for (int x = 0; x < 100; x++)
{
    for (int x = 0; x < 10; x++)
    {
        
    }
}

[mtijanic]

Just leaving a few nits here while I actually make sense of the bigger change. This will take a while.

[tinygiant98]

How does it handle nested loops that use the same variable name? Are they scoped properly?

for (int x = 0; x < 100; x++)
{
    for (int x = 0; x < 10; x++)
    {
        
    }
}

Ran a nested scope test. Results are on discord, but I'll put there here for record-keeping purposes.

Test code snippet:

for (int x = 0; x < 5; x++)
{
    Debug(c("  start outer loop -> x = " + _i(x), COLOR_ORANGE));
    for (int x = 3; x > 0; x--)
    {
        Debug(c("    inner loop -> x = " + _i(x), COLOR_RED_LIGHT));
    }
    Debug(c("  end outer loop -> x = " + _i(x), COLOR_GREEN_LIGHT));
}

I think it meets your intent, but if this test is too simple, let me know.

[tinygiant98]

I've run all the tests I can think of here, but I also know how it's supposed to work, so I'm probably missing some ideas on how to break it. If you guys have any tests you'd like run against this PR, let me know and I'll run 'em.

[hendrikgit]

I ran a test on the scrips from my PW, about 1300 files, some are includes of course.

% ./nwn_script_comp --version
...
neverwinter 2.0.2 (master/f407d7, nim 2.2.2)

% ./nwn_script_comp -d ~/sfee/out_master -c ~/sfee/unpacked/nss
I [2025-03-14T18:17:16] 1210 successful, 91 skipped, 0 errored


% ./nwn_script_comp --version
...
neverwinter 2.0.2 (initializers-incrementers/efd9bf, nim 2.2.2)

% ./nwn_script_comp -d ~/sfee/out_tinygiant -c ~/sfee/unpacked/nss 
I [2025-03-14T18:24:09] 1210 successful, 91 skipped, 0 errored

% diff ~/sfee/out_master ~/sfee/out_tinygiant

No output from diff, all ncs files are the same.

I rebased tinygiants PR branch on master for this test, there were no conflicts. Otherwise I got some errors during compile, I think possibly because of a newer nim version.

[hendrikgit]

Compiling the scripts from the frozen north also shows no differences.

neverwinter 2.0.2 (initializers-incrementers/efd9bf, nim 2.2.2)
% ./nwn_script_comp -d ~/git/tfn-module/out_tinygiant -c ~/git/tfn-module/src/nss
I [2025-03-14T18:47:11] 2099 successful, 165 skipped, 0 errored

neverwinter 2.0.2 (master/f407d7, nim 2.2.2)
% ./nwn_script_comp -d ~/git/tfn-module/out_master -c ~/git/tfn-module/src/nss
I [2025-03-14T18:50:23] 2099 successful, 165 skipped, 0 errored

% diff out_master out_tinygiant
%

[cHackz18]

Hello and thank you for this submission, I had a lot of fun looking through this code and really appreciate all the hard work everyone's putting in to make NWN a better game.

Given that I would also like to contribute to this project I figured I'd share some thoughts an opinions on the code herein:

General feedback and nitpicks submitted. Just some thoughts and opinions on code quality more than anything. Reasoning about the changes herein is very difficult as a newcomer and even harder with the amount of magic numbers that pervade this change.

There is significant complexity in these changes with various cases that are difficult to understand given they all look identical except for the magic numbers. Naming those magic numbers would go a long way to explaining 'what' is going and why each case is unique.

Though I could be wrong and that may not be the case. If that's so, then perhaps comments or some additional helper functions with names might help elevate this issue.

If you have any questions about the contents of this code review, please reach out :) I'm in the official discord as cHackz18 but I will also check in here too.

Thanks again,
Tom W

[mtijanic]

If you resolve the merge conflicts and fix the whitespace nits above, I'll click merge.

[tinygiant98]

Ok, will try to clean all that up this week.

[tinygiant98]

@cHackz18 There really aren't any magic numbers in the parser. The parser is broken up into grammar sections and each section has a set of numbered rules that are usually defined in the comments above the grammar section. Within each rule are a set of terms which are essentially just an ordered set of operations based on the possible token combinations under that rule.

If you include me, I believe there are a total of zero people that really understand the compiler in full. If you're interested in understanding more about the parser methodologies, contact me on Discord and we can muddle through it.