Separate AI and player logic #4426

Garanas · 2022-11-18T19:37:27Z

While watching Relent0r work on the AI I noticed that there are many - many tables that the AI requires to function. That is fine. But these tables are also loaded when there is no need for them (no AIs, not campaign, etc).

We're talking about files like these:

We're talking about tens of thousands of tables - they should not be in memory when they are not required. The idea of this pull request is to make sure that these files do not get loaded unless there is need for them. Sadly though, there are many files that import each other and somewhere at the end of it there is always some reference to these files. Especially the base template file is strangely nested into some files.

@Hdt80bro I'd like your thoughts on this one. Should we try and combat the import madness, or shall we just assign nil to them and 'cull' them if we detect that there is no need for them?

Note that we detect the need of an AI with this value:

https://github.com/FAForever/fa/blob/deploy/fafdevelop/lua/simInit.lua#L272

So we know for sure when the files can be culled or not.

relent0r · 2022-11-21T02:42:26Z

Would this cause issues with the ScenarioInfo.Options.AIReplacement option? I've never used it so I don't know if it actually works in faf.

Garanas · 2022-11-21T06:36:02Z

It does not, it is taken into account. If that is enabled, then the game is considered to 'have AIs'

Hdt80bro · 2022-11-21T21:52:58Z

I think making the AI not load when there are no AI's is a wonderful thing. Can elaborate what you mean about controlling the imports though? If there's an AI in the game, then the AI files are going to get loaded no matter what, so I don't see a point in delaying their loading. If there's unused imports that load the AI files when there are no AI's, but you're thinking that removing them will cause compatibility issues, don't; utilizing the AI when there is none would be an error anyway, and any mod that transparently enables the AI would have those files imported somewhere else regardless. We either need to redo the AI (which completely breaks mods compatibility) or only try to fix it--and I'm pretty sure we're leaning towards the former since the latter isn't feasible.
In terms of table reduction, I count 29,360 duplicate tables of the 37,916 tables total in basetemplates.lua, so that'd be a good place to start.

…logic

Garanas · 2022-12-07T10:22:29Z

Using ShowStats, with this pull request the game loads in:

220mb, 223mb, 223mb, 227mb, 226mb, 227mb

Without it, the game loads in:

236mb, 237mb, 239mb, 235mb, 236mb, 240mb

Data is gathered by starting the game from the executable and loading in the map Theta Passage. It seems to me that it is safe to say that not loading these files at least reduce the total amount of (initial) memory by 5 - 10 megabytes.

speed2CZ · 2022-12-07T13:04:55Z

Campaign doesnt use these files either. Base templates are created in the base manager using data from map_save file. Engineers platoons are loaded by the base manager. And any attack templates are defined in the mission script and go through OPAI

lua/AI/OpAI/AirAttacks_save.lua

Garanas · 2022-12-07T15:04:34Z

Campaign doesnt use these files either. Base templates are created in the base manager using data from map_save file. Engineers platoons are loaded by the base manager. And any attack templates are defined in the mission script and go through OPAI

I can remove the exception for campaign. But how certain are you of this?

hahn-kev · 2022-12-07T15:15:37Z

I might be able to export data from the vscode extension to make a graph of the file imports. Would that be useful?

speed2CZ · 2022-12-07T15:35:43Z

Campaign doesnt use these files either. Base templates are created in the base manager using data from map_save file. Engineers platoons are loaded by the base manager. And any attack templates are defined in the mission script and go through OPAI

I can remove the exception for campaign. But how certain are you of this?

Campaign needs some basic AI logic running, but it surely doesn't use these large with with base or builder templates

Garanas · 2022-12-07T15:48:28Z

I might be able to export data from the vscode extension to make a graph of the file imports. Would that be useful?

It would definitely be interesting. Would that be dynamic?

Garanas · 2022-12-08T09:41:00Z

I'm going to merge this in. @speed2CZ if you want to remove the use of these files in campaign too, all you'll need to change is this: https://github.com/FAForever/fa/blob/deploy/fafdevelop/lua/simInit.lua#L92-L96

I don't have the time at the moment to take that into account, as it would require quite a bit of testing as I'm not familiar with the co-op code that much.

Garanas · 2022-12-08T09:45:38Z

@relent0r / @maudlin27 / @Azraeel / @Uveso as this touches the foundation of AIs - please make sure your AIs still run as expected. I've tested the base game AI, and that one works.

speed2CZ · 2022-12-08T09:46:03Z

Alright, thanks

Garanas · 2022-12-08T09:46:45Z

And note to self: I'm not sure if we need these files. In particular the basetemplates.lua file can easily do with a function that computes locations on demand, instead of having them stored like this as a gigantic lookup table.

relent0r · 2022-12-08T19:46:16Z

basetemplates.lua

Its a lookaround table, in the default AI it doesn't do much special beyond the spiral look around for locations to build from a point. But when custom templates are used its useful for having certain units only be built within a certain set of positions.

examples of how I use them currently.

Firebase where you want the shield is be in the center and other things not at the center so the shield position is always free.
a walled t1 pd

I've been meaning to learn how to rotate them so I could get things like a firebase with PD at the front and aa at the back but I haven't done it yet. Loud does it.

relent0r · 2022-12-08T19:47:06Z

Anyway this seems to have broken my AI's ability to import files. I'm not sure if its this exact PR as I didn't check the branch on its own but its me doing the latest pull on fafdevelop.

e.g

g:\code\fa\lua\system\import.lua(155): Error importing '/lua/ai/aibuildstructures.lua'
warning: stack traceback:
warning: [C]: ?
warning: g:\code\fa\lua\system\import.lua(77): in function <g:\code\fa\lua\system\import.lua:41>
warning: g:\code\fa\lua\system\import.lua(155): in function import' warning: g:\code\fa\lua\system\import.lua(131): in function import'
warning: g:\code\fa\lua\ai\aibehaviors.lua(10): in main chunk
warning: [C]: in function doscript' warning: [C]: ? warning: g:\code\fa\lua\system\import.lua(61): in function <g:\code\fa\lua\system\import.lua:41> warning: g:\code\fa\lua\system\import.lua(155): in function import'
warning: g:\code\fa\lua\system\import.lua(131): in function import' warning: g:\code\fa\lua\ai\aiutilities.lua(15): in main chunk warning: ... warning: [C]: ? warning: g:\code\fa\lua\system\import.lua(61): in function <g:\code\fa\lua\system\import.lua:41> warning: g:\code\fa\lua\system\import.lua(155): in function import'
warning: g:\code\fa\lua\system\import.lua(131): in function import' warning: ...\mods\rngai\lua\flowai\framework\mapping\mapping.lua(11): in main chunk warning: [C]: in function doscript'
warning: [C]: ?
warning: g:\code\fa\lua\system\import.lua(61): in function <g:\code\fa\lua\system\import.lua:41>
warning: g:\code\fa\lua\system\import.lua(155): in function `import'
warning: g:\code\fa\lua\siminit.lua(1765): in main chunk

Garanas · 2022-12-08T20:06:45Z

I see, I'll look at it 👍

4z0t · 2022-12-08T22:05:43Z

Does ScenarioInfo exist before game stars?

4z0t · 2022-12-08T22:11:12Z

@relent0r can you provide a full log?

relent0r · 2022-12-08T23:16:11Z

Sure
SCFAlog.txt

@4z0t

Garanas · 2022-12-09T05:20:27Z

I think it is because I removed an import.

4z0t · 2022-12-09T09:16:21Z

warning: [aibuildstructures.lua, line:771] * RNGAI: offset aibuildstructures.lua
warning: ...r forged alliance\mods\rngai\lua\ai\rngutilities.lua(4): access to nonexistent global variable GetMap

Garanas · 2022-12-09T17:55:41Z

@relent0r We found it 😄

tldr: add these two lines at the very top of your simInit.lua hook:

local AIUtils = import("/lua/ai/aiutilities.lua")
local AIAttackUtils = import("/lua/ai/aiattackutilities.lua")

long version: you have a nasty circular dependency with an immediate use on that dependency 😄 , allow me to show you the stack trace. The loop starts in simInit.lua with:

local RNGBeginSession = import('/mods/RNGAI/lua/FlowAI/framework/mapping/Mapping.lua').BeginSession

Which produces this sequence of imports:

DEBUG: Touched module '	/mods/RNGAI/lua/FlowAI/framework/mapping/Mapping.lua	' 						
DEBUG: Loading module '	/mods/rngai/lua/flowai/framework/mapping/mapping.lua	'# <-- start loading mapping.lua
DEBUG: Touched module '	/lua/AI/aiattackutilities.lua	' # marked
DEBUG: Loading module '	/lua/ai/aiattackutilities.lua	'
INFO: Hooked /lua/ai/aiattackutilities.lua with /mods/rngai/hook/lua/ai/aiattackutilities.lua
DEBUG: Touched module '	/lua/sim/scenarioutilities.lua	'
DEBUG: Touched module '	/lua/ai/aiutilities.lua	'
DEBUG: Loading module '	/lua/ai/aiutilities.lua	'
INFO: Hooked /lua/ai/aiutilities.lua with /mods/rngai/hook/lua/ai/aiutilities.lua
DEBUG: Touched module '	/lua/buildingtemplates.lua	'
DEBUG: Touched module '	/lua/unittemplates.lua	'
DEBUG: Touched module '	/lua/sim/scenarioutilities.lua	'
DEBUG: Touched module '	/lua/utilities.lua	'
DEBUG: Loading module '	/lua/utilities.lua	'
DEBUG: Touched module '	/lua/ai/aiattackutilities.lua	'
DEBUG: Touched module '	/lua/sim/buff.lua	'
DEBUG: Loading module '	/lua/sim/buff.lua	'
DEBUG: Touched module '	/lua/ai/sorianutilities.lua	' # marked
DEBUG: Loading module '	/lua/ai/sorianutilities.lua	'
DEBUG: Touched module '	/lua/ai/aiutilities.lua	' # marked
DEBUG: Touched module '	/lua/ai/aiattackutilities.lua	'
DEBUG: Touched module '	/lua/utilities.lua	'
DEBUG: Touched module '	/lua/mods.lua	'
DEBUG: Touched module '	/lua/ai/sorianlang.lua	'
DEBUG: Loading module '	/lua/ai/sorianlang.lua	'
DEBUG: Touched module '	/lua/ai/aibehaviors.lua	' # marked
DEBUG: Loading module '	/lua/ai/aibehaviors.lua	'
INFO: Hooked /lua/ai/aibehaviors.lua with /mods/rngai/hook/lua/ai/aibehaviors.lua
DEBUG: Touched module '	/lua/ai/aiutilities.lua	'
DEBUG: Touched module '	/lua/utilities.lua	'
DEBUG: Touched module '	/lua/ai/aibuildstructures.lua	'
DEBUG: Loading module '	/lua/ai/aibuildstructures.lua	'
INFO: Hooked /lua/ai/aibuildstructures.lua with /mods/rngai/hook/lua/ai/aibuildstructures.lua
DEBUG: Touched module '	/lua/basetemplates.lua	'
DEBUG: Touched module '	/lua/buildingtemplates.lua	'
DEBUG: Touched module '	/lua/utilities.lua	'
DEBUG: Touched module '	/lua/ai/aiutilities.lua	'
DEBUG: Touched module '	/lua/sim/scenarioutilities.lua	'
DEBUG: Touched module '	/lua/ai/aiattackutilities.lua	' # marked
DEBUG: Touched module '	/lua/scenariotriggers.lua	'
DEBUG: Loading module '	/lua/scenariotriggers.lua	'
DEBUG: Touched module '	/lua/sim/scenarioutilities.lua	'
WARNING: [aibuildstructures.lua, line:771] * RNGAI: offset aibuildstructures.lua
DEBUG: Touched module '	/mods/RNGAI/lua/AI/RNGUtilities.lua	'# <-- loads in mapping.lua too
DEBUG: Loading module '	/mods/rngai/lua/ai/rngutilities.lua	'								
DEBUG: Touched module '	/lua/ai/AIUtilities.lua	' # 1.
DEBUG: Touched module '	/lua/sim/ScenarioUtilities.lua	' # 2.
DEBUG: Touched module '	/lua/AI/aiattackutilities.lua	' # 3.

Note that the last file (after that it crashes) is rngutilities.lua, which looks like this:

local AIUtils = import('/lua/ai/AIUtilities.lua') -- 1.
local ScenarioUtils = import('/lua/sim/ScenarioUtilities.lua') -- 2.
local AIAttackUtils = import('/lua/AI/aiattackutilities.lua') -- 3.
local MAP = import('/mods/RNGAI/lua/FlowAI/framework/mapping/Mapping.lua').GetMap()  -- <-- note this line

Everything works, until we call GetMap, which relies on the same module that we're loading in right now: Mapping.lua! That module is still being loaded, hence the function GetMap does not yet exist as the lua file in question didn't get to that point yet. And with that - we crash because the reference is missing!

And that is a circular dependency!

Then the question: why didn't this show before this pull request? Well, that is because of these lines:

https://github.com/FAForever/fa/pull/4426/files#diff-730e929b30dd5f3d68e556afa4f560cc48e3b8c0b4026d7bcac3705a95a306eeL502-L512

Imports various builders, and those builders occasionally import one of the files that I've marked in the stack trace. Effectively they broke the circular dependency because they change the order of loading. And by adding these to the very top of your hook of simInit.lua, you do the exact same thing:

local AIUtils = import("/lua/ai/aiutilities.lua")
local AIAttackUtils = import("/lua/ai/aiattackutilities.lua")

relent0r · 2022-12-09T19:02:28Z

Thanks @Garanas and @4z0t
Damn thats really hard to troubleshoot, I'd never considered this sort of thing before.

p.s you got any advice best practice wise to avoid this in the future? or is it a matter if thinking long and hard before adding locals to the top of a module?

4z0t · 2022-12-09T21:57:30Z

Thanks @Garanas and @4z0t Damn thats really hard to troubleshoot, I'd never considered this sort of thing before.

p.s you got any advice best practice wise to avoid this in the future? or is it a matter if thinking long and hard before adding locals to the top of a module?

@relent0r Consider using lazyimport :)

Garanas · 2022-12-10T07:31:38Z

Lazy import would not have prevented this particular situation. It is trying to import:

local RNGBeginSession = import('/mods/RNGAI/lua/FlowAI/framework/mapping/Mapping.lua').BeginSession

Which ends up importing:

local MAP = import('/mods/RNGAI/lua/FlowAI/framework/mapping/Mapping.lua').GetMap()

Because of the immediate access, lazy import would not have helped.

@relent0r lazy import is something we introduced last week. To me the only actual use case is to prevent importing files that would not be touched in some situation. For example, we can prevent loading the basetemplates.lua file using lazy import, as that file is only used in AI games. When we play a PvP game the file is never accessed, and therefore never truly loaded.

This idea only applies when the code base is generic, like the base repository. In your case you likely want to use your imports or you would not have imported it. Lazy import is no 'concrete' solution either, as pointed out at the start of this post.

Circular dependencies is an issue across all languages and all libraries. Last week alone I managed to produce one at work too, but in that case the application told me that I had a circular dependency. Our best bet here is to add a better warning that indicates that you have a circular dependency too. Besides that, there's nothing you can do against it besides being careful when you import something that you end up using immediately - those are prime candidates for circular dependencies during loading as we've seen here.

KionX · 2022-12-24T11:50:53Z

BeginSessionCommonArmy() is broken. teams missing.

Hdt80bro · 2022-12-25T05:42:24Z

@Garanas did you mean to make teams global when separating the AI and player logic? (I actually nearly setup a team structure in that file myself to keep track of team recall data, but I couldn't getting it working in time so I shoved it inside the AIBrain)

Garanas · 2022-12-25T10:47:52Z

No, I just messed up 😄 !

Reduces the amount of memory used in non-ai games by 5 - 10 megabytes.

Garanas added 2 commits November 18, 2022 20:25

Simplify BeginSession

554a78d

Load in builders of loaded AIs properly

46a967a

Garanas added 5 commits December 6, 2022 21:18

Merge branch 'deploy/fafdevelop' into feature/separate-ai-and-player-…

f09df13

…logic

Lazy load base templates

3172259

Add warnings

ba0a879

Lazy load upgrade templates

238bb23

Lazy load other templates

1e61806

Add warnings

e83c138

4z0t reviewed Dec 7, 2022

View reviewed changes

lua/AI/OpAI/AirAttacks_save.lua Outdated Show resolved Hide resolved

Process feedback

aba5c6f

Garanas merged commit 3eba82a into deploy/fafdevelop Dec 8, 2022

Garanas deleted the feature/separate-ai-and-player-logic branch December 8, 2022 09:43

Garanas mentioned this pull request Dec 8, 2022

Fix teams not working #4483

Merged

Garanas added a commit that referenced this pull request Jan 28, 2023

Separate AI and player logic (#4426)

3a56c64

Reduces the amount of memory used in non-ai games by 5 - 10 megabytes.

Garanas added area: sim Area that is affected by the Simulation of the Game area: AI related to AI functions labels Feb 23, 2023

Garanas added this to the Development iteration I milestone Feb 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separate AI and player logic #4426

Separate AI and player logic #4426

Garanas commented Nov 18, 2022

relent0r commented Nov 21, 2022

Garanas commented Nov 21, 2022

Hdt80bro commented Nov 21, 2022

Garanas commented Dec 7, 2022

speed2CZ commented Dec 7, 2022

Garanas commented Dec 7, 2022

hahn-kev commented Dec 7, 2022

speed2CZ commented Dec 7, 2022

Garanas commented Dec 7, 2022 •

edited

Loading

Garanas commented Dec 8, 2022

Garanas commented Dec 8, 2022

speed2CZ commented Dec 8, 2022

Garanas commented Dec 8, 2022

relent0r commented Dec 8, 2022

relent0r commented Dec 8, 2022 •

edited

Loading

Garanas commented Dec 8, 2022

4z0t commented Dec 8, 2022

4z0t commented Dec 8, 2022

relent0r commented Dec 8, 2022 •

edited

Loading

Garanas commented Dec 9, 2022

4z0t commented Dec 9, 2022

Garanas commented Dec 9, 2022 •

edited

Loading

relent0r commented Dec 9, 2022 •

edited

Loading

4z0t commented Dec 9, 2022 •

edited

Loading

Garanas commented Dec 10, 2022

KionX commented Dec 24, 2022

Hdt80bro commented Dec 25, 2022

Garanas commented Dec 25, 2022

Separate AI and player logic #4426

Separate AI and player logic #4426

Conversation

Garanas commented Nov 18, 2022

relent0r commented Nov 21, 2022

Garanas commented Nov 21, 2022

Hdt80bro commented Nov 21, 2022

Garanas commented Dec 7, 2022

speed2CZ commented Dec 7, 2022

Garanas commented Dec 7, 2022

hahn-kev commented Dec 7, 2022

speed2CZ commented Dec 7, 2022

Garanas commented Dec 7, 2022 • edited Loading

Garanas commented Dec 8, 2022

Garanas commented Dec 8, 2022

speed2CZ commented Dec 8, 2022

Garanas commented Dec 8, 2022

relent0r commented Dec 8, 2022

relent0r commented Dec 8, 2022 • edited Loading

Garanas commented Dec 8, 2022

4z0t commented Dec 8, 2022

4z0t commented Dec 8, 2022

relent0r commented Dec 8, 2022 • edited Loading

Garanas commented Dec 9, 2022

4z0t commented Dec 9, 2022

Garanas commented Dec 9, 2022 • edited Loading

relent0r commented Dec 9, 2022 • edited Loading

4z0t commented Dec 9, 2022 • edited Loading

Garanas commented Dec 10, 2022

KionX commented Dec 24, 2022

Hdt80bro commented Dec 25, 2022

Garanas commented Dec 25, 2022

Garanas commented Dec 7, 2022 •

edited

Loading

relent0r commented Dec 8, 2022 •

edited

Loading

relent0r commented Dec 8, 2022 •

edited

Loading

Garanas commented Dec 9, 2022 •

edited

Loading

relent0r commented Dec 9, 2022 •

edited

Loading

4z0t commented Dec 9, 2022 •

edited

Loading