This feels to me like the wrong approach. From what I understand.

• The server creates a new AI; this causes a command to be queued
• Clients + server receive this command; they validate again if the server should have said this. If the server clearly should not have said this, everyone ignores the command.

This feels like a very dirty hack. The clients should never ignore a command from the server like that. It feels this needs more thinking, and possibly a better approach to resolve.

This feels to me like the wrong approach. From what I understand.

The server creates a new AI; this causes a command to be queued
Clients + server receive this command; they validate again if the server should have said this. If the server clearly should not have said this, everyone ignores the command.

This feels like a very dirty hack. The clients should never ignore a command from the server like that. It feels this needs more thinking, and possibly a better approach to resolve.

This is a problem that happens in multiplayer. Single player does not apply.

Problem is difficult to follow, but gonna try explain with an example.

In multiplayer, when multiple AIs are set to start with 0 days delay between each other, OnTick_Companies() is called every tick to start them. Let's imagine I've set max_no_competitors to 5 and network.max_companies to 15.

Tick 1:
On the first tick, OnTick_Companies() is called, MaybeStartNewCompany() is tested and it will give Company::GetNumItems() == 0 (assuming dedicated server), and will count the number of AIs, n == 0. Both tests fail, which allows DoCommandP to be sent.

This command will run in DC_TEST mode. CmdCompanyCtrl returns with CommandCost(). Since this is a network, the command is sent into a queue, it's not executed right away. It does a NetworkSendCommand and puts it in _local_wait_queue, then returns Succeeded.

For the rest of this tick, nothing happens. Game proceeds to next tick.

Tick 2:
NetworkDistributeCommands() is called, which runs DistributeQueue(). There is a commands_per_frame setting there, set to 2. I see it moving the command from _local_wait_queue into _local_execution_queue via DistributeCommandPacket. Since there's only 1 command in _local_wait_queue for now, commands_per_frame limit is not reached.

After that splitting, NetworkExecuteLocalCommandQueue() is called. The command from _local_execution_queue is run. CmdCompanyCtrl is first run in DC_TEST mode, returning with CommandCost(), and then it is run again in DC_EXEC mode, where it actually creates the AI company.

Now the cycle repeats, OnTick_Companies() is called, MaybeStartNewCompany() is tested and it will give Company::GetNumItems() == 1, and will count the number of AIs, n == 1. Both tests fail, which allows DoCommandP to be sent.

This command will run in DC_TEST mode. CmdCompanyCtrl returns with CommandCost(). Since this is a network, the command is sent into a queue, it's not executed right away. It does a NetworkSendCommand and puts it in _local_wait_queue, then returns Succeeded.

For the rest of this tick, the first AI that started does its stuff and also queues one command in _local_wait_queue. So _local_wait_queue now has 2 commands.

Tick 3:
NetworkDistributeCommands() is called, which runs DistributeQueue(). It is now moving 2 commands from _local_wait_queue to _local_execution_queue. commands_per_frame limit is reached, but since there was only 2 commands, _local_wait_queue still becomes empty.

NetworkExecuteLocalCommandQueue() will run the commands in _local_execution_queue in the order they were queued. First, CmdCompanyCtrl in DC_TEST mode and DC_EXEC mode, where the 2nd AI company is created. And second, the command that the 1st AI had queued.

The cycle repeats, OnTick_Companies() is called, MaybeStartNewCompany() is tested and it will give Company::GetNumItems() == 2, and will count the number of AIs, n == 2. Both tests fail, which allows DoCommandP to be sent.

CmdCompanyCtrl() is run in DC_TEST mode, then NetworkSendCommand puts it in _local_wait_queue and the command returns Succeeded.

For the rest of this tick, the first AI queues a command and the second AI also queues a command. _local_wait_queue now has 3 commands.

Tick 4:
NetworkDistributeCommands() is called, which runs DistributeQueue(). It is moving 2 commands from _local_wait_queue to _local_execution_queue. commands_per_frame limit is reached, but there were 3 commands, _local_wait_queue remains with one command, the last that was queued, which is the command from the 2nd AI.

NetworkExecuteLocalCommandQueue() will run the commands in _local_execution_queue in the order they were queued. First, CmdCompanyCtrl in DC_TEST mode and DC_EXEC mode, where the 3nd AI company is created. And second, the command that the 1st AI had queued. The command from the 2nd AI is not executed.

Repeat, OnTick_Companies() is called, MaybeStartNewCompany() is tested and it will give Company::GetNumItems() == 3, and will count the number of AIs, n == 3. Both tests fail, which allows DoCommandP to be sent.

CmdCompanyCtrl() is run in DC_TEST mode, then NetworkSendCommand puts it in _local_wait_queue and the command returns Succeeded. _local_wait_queue now contains 2 commands, the command from the 2nd AI, and creation of the 4th AI company.

For the rest of this tick, the first AI queues a command, the second AI does nothing, and the third AI queues a command. _local_wait_queue now has 4 commands: command from the 2nd AI, creation of the 4th AI company, command from 1st AI, and command from 3rd AI.

Tick 5:
NetworkDistributeCommands() moves 2 commands from _local_wait_queue to _local_execution_queue.

NetworkExecuteLocalCommandQueue() will run 2 commands from _local_execution_queue, first, the command from the 2nd AI, then CmdCompanyCtrl in DC_TEST, then DC_EXEC mode, creating the 4th AI company.

OnTick_Companies(), MaybeStartNewCompany()… Company::GetNumItems() == 4, n == 4. Both tests fail, which allows DoCommandP to be sent.

CmdCompanyCtrl() is run in DC_TEST mode, then NetworkSendCommand puts it in _local_wait_queue and the command returns Succeeded. _local_wait_queue now contains 3 commands, the command from the 1st AI and 3rd AIs, and creation of the 5th AI company.

For the rest of this tick, 2nd AI queues a command, and 4th AI also queues a command. _local_wait_queue contains 5 commands: command from 1st AI, command from 3rd AI, creation of 5th company, command from 2nd AI, and command from 4th AI.

Tick 6 (is where the problem first arises):
NetworkDistributeCommands() moves 2 commands from _local_wait_queue to _local_execution_queue. _local_wait_queue will remain with 3 commands, which are creation of 5th company, command from 2nd AI, and command from 4th AI.

NetworkExecuteLocalCommandQueue() will run 2 commands from _local_execution_queue, which are command from 1st AI and command from 3rd AI. This means the 5th company is not created this tick!

OnTick_Companies() happens, MaybeStartNewCompany() is doing tests and Company::GetNumItems() == 4, n == 4. It does not know that there was a 5th company! The tests in this situation are incorrect, but they will still fail, allowing the queue of the 6th AI company.

CmdCompanyCtrl() is run in DC_TEST mode, then NetworkSendCommand puts it in _local_wait_queue and the command returns Succeeded. _local_wait_queue now contains 4 commands: the creation of the 5th AI company, command from the 2nd AI, command from the 4th AI and creation of the 6th company.

For the rest of the tick, commands from 1st and 3rd AIs are queued. _local_wait_queue now contains 6 commands: the creation of the 5th AI company, command from the 2nd AI, command from the 4th AI, creation of the 6th company, command from 1st AI and command from 3rd AI.

Tick 7:
NetworkDistributeCommands() moves 2 commands from _local_wait_queue to _local_execution_queue. _local_wait_queue will remain with 4 commands which are command from the 4th AI, creation of 6th company, command from 1st AI and command from 3rd AI.

NetworkExecuteLocalCommandQueue() runs 2 commands from _local_execution_queue. First, CmdCompanyCtrl in DC_TEST and DC_EXEC modes. The 5th company is created. And then, the command from the 2nd AI is executed.

OnTick_Companies(), MaybeStartNewCompany() this time gets Company::GetNumItems() == 5 and n == 5. Since 5 was the limit for the max_no_competitors, the test will pass, allowing this function to return false. No more companies are created… but this comes too late. The 6th AI company is still in the queue, remember.

For the rest of the tick, commands from 2nd and 5th AI are queued. _local_wait_queue contains 6 commands: command from 4th AI, creation of 6th AI company, command from 1st AI, command from 3rd AI, command from 2nd AI and command from 5th AI.

Tick 8 (the final rest of the problem):
NetworkDistributeCommands() moves 2 commands from _local_wait_queue to _local_execution_queue. _local_wait_queue will remain with 4 commands: command from 1st AI, command from 3rd AI, command from 2nd AI and command from 5th AI.

NetworkExecuteLocalCommandQueue() runs 2 commands from _local_execution_queue. First, the command from the 4th AI is executed. And finally, CmdCompanyCtrl in DC_TEST and DC_EXEC modes. Without this patch, the 6th company is created. With it, both DC_TEST and DC_EXEC fail. No 6th AI company is created.

The rest doesn't matter anymore.

This big example may not truly reflect what happens in the code, it's just my mere observations following the code in visual studio. I have not really checked how different the execution is on a client other than the server. I assumed that, since _settings_client.network.max_companies is not a game setting, I decided to pass it through p2. Client and server could probably have different values for this setting, that's why I avoid testing against it.

Tnx for the lengthy explanation. So basically what I said holds. So that means your solution is more a hack than a real solution, as you didn't tackle the real issue here.
Why I say it is a hack, if you look a bit further: say there are 100 commands in the queue, and you want 10 AIs. This would mean that for 20 ticks (100 / 5), 10 AI commands are queued. So by the time the first AI wants to be create, 20 * 10 commands more are in the queue. Of course 19 * 10 commands are later dismissed. This is what I mean with: this feels like a hack. You are not solving the issue, but an underlying symptom.

So we should look for how to solve this properly.

I think it all originates in the original patch: looping over a function to create multiple sounds like a dirty approach on its own. If you want to start 10 AIs in a single tick, there should be a single command creating those 10 AIs. Not loop over creating 1 AI 10 times, hoping it can be done in a single tick.
So possibly, instead of looking in this direction, you can look in that (or any other) direction?

Idea for making a single command to start multiple AIs: Use 16 bits of either p1 or p2 to CCA_NEW_AI as a bitmask of companies to start AI for. Then loop over all set bits in that mask inside the command execution.

PS: please fix commit message length. And please please, never ever again make title of Pull Request this long please. It is not helping, and kinda makes me want to walk away ..

Given 011257d has long since been reverted, what is this PR doing, exactly?

LordAro, #7376 is trying to generate the correct number of AIs in multiplayer. Just tested 20191230-master-gf6ce5c4563 and the bug is still present.

Going to close this one. It's not been touched in a while, and there's been no real consensus on it, other than "this is the wrong solution"