Add: GSAsyncMode(bool) class to set asynchronous mode of game script commands

[JGRennison]

Motivation / Problem

Currently all GS commands cause the script to be suspended for at least 1 tick, such that it can receive the eventual result of the command as executed on all clients (in single-player, this delay is artificially added).
The vast majority of the time, the result of a command is not used by the GS.
A typical GS spends almost all of its time in a loop issuing commands to set town/industry/etc. text values, or update town growth values, for all towns/industries/etc.
The net effect of this is that GSs are excessively slow to update the game state.

Description

Add a GSAsyncMode(bool) class to set async mode of game script commands.
The script syntax and mode of operation is broadly equivalent to the GSTestMode class.
In asynchronous mode, don't wait for result of executed command, just fire-and-forget, and return estimated cost/result.
The GS does not lose its time slice when executing a command asynchronously, so charge a nominal opcode fee instead.

Limitations

Commands issued by the GS are still limited by the network.commands_per_frame setting.
I can create another PR for that if this is merged.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

• The bug fix is important enough to be backported? (label: 'backport requested')
• This PR touches english.txt or translations? Check the guidelines
• This PR affects the save game format? (label 'savegame upgrade')
• This PR affects the GS/AI API? (label 'needs review: Script API')
  • ai_changelog.hpp, gs_changelog.hpp need updating.
  • The compatibility wrappers (compat_*.nut) need updating.
• This PR affects the NewGRF API? (label 'needs review: NewGRF')
  • newgrf_debug_data.h may need updating.
  • PR must be added to API tracker

[PeterN]

It feels like this should somehow be enabled by default for GS but I don't know if that is feasible.

[TrueBrain]

I really like the simplicity of the solution you created here. Initially I was afraid people wanted to know the actual result when it eventually executed, but this is something that could be added to this solution if that question arises.

And yes, please PR the increase of commands-per-frame; we will need it :)

[ldpl]

As I said many times in discord I don't think async is the right way to go for GS. That 1 tick delay is entirely superstitious and can easily be removed, I even did that in vanilla-compatible way in cmserver. And while that would require server running 1 tick ahead of clients I don't see how is that a problem. In return GS will get instant command execution with exact result of the action and no changes to existing scripts required. Would also solve race conditions between script and the game. Even if callbacks are to be added to GS in the future they still can fire-and-forget commands like in this pr.

PoC implementation of GS without delay here: https://github.com/ldpl/OpenTTD/tree/insta-gs

[PeterN]

Usual riules, saying something a 1000 times elsewhere doesn't get you anywhere.

[TrueBrain]

It feels like this should somehow be enabled by default for GS but I don't know if that is feasible.

We talked about this on Discord, but to sync back here:

The current GSes will fail in that case. All instances where you get a result which you have to use for the next call, will not work. For example, creating anything that returns an index. So Goal, Sign, etc. So no, we can't enable this by default for those.

Enabling it by default for new GSes also wouldn't be my advise, because all documentation out there excepts the old behaviour. So I think it is better to have the author do this explicitly. Explicit is good!

[TrueBrain]

And while that would require server running 1 tick ahead of clients I don't see how is that a problem.

In core, this is the issue with your solution. You assume a frame_freq of zero, which is not guaranteed. In result, for people running with higher values, it can desync easily. So this is just not a good solution.

We could work towards removing frame_freq, and then revisit this. And by the time we get there, we can just make this class a noop. As I don't expect anyone working on the network code for a while, the solution in this PR is far more practical: it is ready to go, doesn't impact anything else, is condense, and does the job. And is easily removed if we ever get to a point it is no longer needed. All the benefits, none of the cost.

[ldpl]

@TrueBrain, right, forgot to mention frame_freq. I'm curious for telemetry results though to see if there is even anyone using non-standard frequency as that's just "add lag" setting afaict. Doesn't really affect my solution much though, server will just have to run that many frames ahead.

And, well, there is always some cost of having a redundant api but, other than that, I agree there is no downside to this PR.

[FLHerne]

This will make GS a lot more useful, thanks!

It might be useful to (optionally?) stop executing the queue of async commands if one raises an exception, to avoid building a lot of useless infrastructure in case of a failure or otherwise running into unexpected states.

No idea if that can be reasonably added as implemented. I did try reading the patch to find out but it goes completely over my head without more knowledge of how GS execution works, sorry.

[andythenorth]

\o/