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

AGS 4.0: rewrite ccInstance into non-forking ScriptExecutor #2621

Draft
wants to merge 13 commits into
base: ags4
Choose a base branch
from

Conversation

ivan-mogilko
Copy link
Contributor

@ivan-mogilko ivan-mogilko commented Dec 14, 2024

Purpose:

  • reorganize ccInstance class, separate "runtime script data", "script thread state" and "script execution";
  • make it work without "forks", which are the copies of ccInstance, required to run "repeatedly_execute_always" while another script is suspended;
  • as a consequence, in theory support any number of nested script calls made through the nested engine API calls, i.e. interleaved engine -> script -> engine -> script -> etc, which may be wanted for delegates/function pointers to work (see discussion in Let's implement delegates, as a callback and function pointer mechanism #1409).
  • also in theory may be expanded to support suspended and saved script threads, in case we want to try and implement coroutines in AGS at some point.

What is done

  1. The former organization ccScript - ccInstance is expanded into: ccScript - RuntimeScript - ScriptThread - ScriptExecutor, where
    • ccScript is a loaded script data;
    • RuntimeScript is a script prepared for execution, with fixups and imports resolved, and linked with other scripts;
    • ScriptThread contains data stack, callstack and execution position;
    • ScriptExecutor is a "virtual machine" that executes RuntimeScript on chosen ScriptThread.
  2. Engine no longer keeps ccScripts, only use them for creating RuntimeScripts.
  3. RuntimeScripts don't make any "forks", there's strictly 1 per each script, and no need for more.
  4. There's a single ScriptExecutor object, which tracks a callstack of RuntimeScripts passed into it, capable of starting nested script runs either on the same or different script thread, while there's another/other script execution suspended.

In the situation where a script would call a blocking action, such as Wait() function, for example,
previously engine would do this:

  • run base ccInstance for some script event
  • call Wait()
  • run forked ccInstance for "repeatedly_execute_always"

after this change it will do:

  • call ScriptExecutor::Run for some script event
  • call Wait()
  • call ScriptExecutor::Run for "repeatedly_execute_always"
  • ScriptExecutor should correctly handle Run calls while there's a callstack of previously suspended scripts, and resume those after nested calls are done.

@ivan-mogilko ivan-mogilko added ags 4 related to the ags4 development context: script vm labels Dec 14, 2024
@ivan-mogilko
Copy link
Contributor Author

I think we can push these changes further, and introduce a "Script Thread" class. The "Script Thread" will contain its own data stack and call stack, leaving only registers in the ScriptExecutor. ScriptExecutor will be told to run this or that script on a specific "script thread".

Then there will be 2 default "threads": the "main" one, and a "non-blocking" one to run "rep-exec-always" on. This will nicely mimic the previous behavior.

But the "script thread" objects may become more useful somewhen in the future if we try to implement coroutines in AGS.

@ericoporto
Copy link
Member

I see there are changes to the script compiler, does this change requires rebuild of the auto test game with it or should be able to run the previously built ags4 auto test game ?

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Dec 14, 2024

I see there are changes to the script compiler, does this change requires rebuild of the auto test game with it or should be able to run the previously built ags4 auto test game ?

No, there are no changes to compiler itself, there are only corrections to data types in ccScript.
I've been testing this with a game compiled by regular ags4 version.

I did not have time earlier, but I will check out what happens when this runs a test game.

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Dec 30, 2024

Just a small note, this pr is working when the engine is run on its own, but it's crashing whenever run from IDE. Possibly the debugger connection is broken. I did not have time to look into this yet, but will finish this in the new year. Besides the mentioned bug there are couple of modifications that I wanted to make. And then this also will need to be tested for performance. The main execution loop did not change, it's mostly just copied from one class to another, but it's better be safe.

@ericoporto
Copy link
Member

ericoporto commented Dec 30, 2024

it's crashing whenever run from IDE.

tested and confirmed, weird error though.

An exception 0xC0000005 occurred in ACWIN.EXE at EIP = 0x00541D4A; program pointer is -199, engine version 4.0.0.12, gtags (0,0)

I guess -199 is after here

set_our_eip(-199);

@ivan-mogilko ivan-mogilko force-pushed the experiment--nonforkingscriptrunner branch from 2fadfb7 to e78fa60 Compare January 6, 2025 16:39
Save original input code[] values meant for fixups in _fixupValues[] array, as code[] array will be overwritten with resolved addresses etc.
Use saved _fixupValues[] when resolving imports.
1. Push and pop callstack on calls to ScriptExecutor::Run() in case we detect a nested Run. Thus we don't have to push/pop callstack on any CALLEXT, so not slowing it down further.
2. Add a Busy bit flag which is set when we start running bytecode, and temporarily unset when we call an external function. This flag indicates whether it's safe to Run another nested script.
3. Reorganized code a bit, try to keep push/pop callstack and saving previous stack pointers as immediately close to the script function call as possible, in sake of the code clarity.
@ivan-mogilko ivan-mogilko force-pushed the experiment--nonforkingscriptrunner branch from e78fa60 to 2beb19c Compare January 6, 2025 23:00
@ivan-mogilko ivan-mogilko changed the title [WIP] AGS 4.0: rewrite ccInstance into non-forking ScriptExecutor AGS 4.0: rewrite ccInstance into non-forking ScriptExecutor Jan 6, 2025
@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Jan 6, 2025

The debugger connection is fixed.

As an additional rework - implemented ScriptThread class which contains a saved state of a "script thread": a data stack, callstack and execution position. The ScriptExecutor now has to be told to execute the script on a particular "thread".
If there's any active script thread, and a new script is run on a different one, ScriptExecutor will push previous thread to the threads stack, and pop afterwards, restoring its state.

There are 2 standard threads created by the engine: "Main" and "Non-blocking" (for running "rep-exec-always"), but more could be added if needed.

My belief is that this system potentially allows to implement "coroutines" in AGS at some point in the future, because it allows to pause a script thread, save its state, and then resume later at some point. I am referring to this coroutine idea I once posted on AGS forums.

ScriptThread class contains a saved state of a running script thread, with a data stack, a callstack and current exec position.
ScriptExecutor always runs a script on a certain thread.
ScriptExecutor maintains a nested list of active threads. These may only be run one at a time in AGS, so when a new script runs while there is another active thread, that thread gets pushed to a thread stack and popped back after that new script finishes running.

Engine creates 2 default script threads: "Main" and "Non-blocking" for running rep-exec-always kind of callbacks.

There could be more ScriptThread instances created as necessary.
@ivan-mogilko
Copy link
Contributor Author

Not planned as a part of this PR, and strictly as a "proof of concept", I made this dumb coroutine test:
https://github.com/ivan-mogilko/ags-refactoring/tree/experiment--dumb-coroutine

Not done fully right, but works in practice. Normally you'd make a script keyword for async or iterating operations (like async or yield in C#); here I did not want to bother with compiler so added WaitAsync(loops) function that converts current script into "coroutine".

In principle it works like this:

  • WaitAsync is called, it sets "async wait counter" and asks executor to suspend;
  • script executor suspends current script thread and returns;
  • engine checks execution result, finds that thread was suspended, copies its full state to the special "coroutine thread";
  • game continues to run as normal, with all the player input, GUI, etc;
  • engine updates "async wait counter" on each game tick;
  • as soon as that reaches zero, it tells script executor to run the previously saved "coroutine thread";
  • more suspends are possible, coroutine thread may be suspended again and again after each start, until it finishes.

The current implementation cannot have multiple simultaneous coroutines, any new one will overwrite previous completely, but that's just because I was lazy. Normally it should have a list of wait counters mapped to coroutine threads, or something similar.

Example of script:

function RunCoroutineTest()
{
    Display("RunCoroutineTest: 1");
    WaitAsync(60);
    Display("RunCoroutineTest: 2");
    WaitAsync(60);
    Display("RunCoroutineTest: 3");
    WaitAsync(60);
    Display("RunCoroutineTest: 4");
}

function on_key_press(eKeyCode key)
{
    if (key == eKeyW) RunCoroutineTest();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ags 4 related to the ags4 development context: script vm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants