Better comm system

[XNTEABDSC]

merged related datas about commanders to single files. (just find, move and Include, so they keeps origin structure, just in another files)

Tested with Chicken comm, start select, morph, weapon, shield, damage and range mod.

[GoogleFrog]

Typo in file name.

[GoogleFrog]

Thanks, I have had a bit of a look although won't have time for a full look for a few weeks. I like the approach of splitting things into multiple files like this. I am concerned about the global ModularCommDefsShared table for maintainability, especially since files sometimes use it and sometimes don't. Relatedly, I don't know what SharedEnv is for and would really much rather avoid using metatables. They hurt maintainability and I've found them to be a confusing way to lose performance, unless everyone who might edit this system knows what they are doing (which they won't).

I'd be a lot happier without the interfile globals and the metatables.With them, this might end up being too much of a maintainability risk.

[GoogleFrog]

Also it should be indented with tabs.

[XNTEABDSC]

Updated. Removed global ModularCommDefsShared and SharedEnv and used tabs

[sprunk]

Scary amount of changes for so few commits. Would be nice if this was cut into minimal commits

[GoogleFrog]

This is too large to review. To start with, there are a lot of random whitespace changes. Eg

Most of these changes are good but they really spam up the log when trying to review it. They shouldn't be in this PR. I would pull a PR that was only whitespace changes though.

• Change A is bad. Some people like to put that extra tab there, some don't. I don't think it is worth the line in a commit to switch from one to the other. If we had to make a clean choice for the entire repository, then I would retain the tab, but short of doing that they shouldn't be randomly touched.
• Change B is bad. From my point of view it doesn't mean anything at all. For whoever it means something for, it will bitrot.

[GoogleFrog]

This is concerning. There is not currently a way to crash context menu by putting a weapon def in the wrong file.

[GoogleFrog]

What does this mean?

[sprunk]

AFAICT the hardcodedID is there to prevent a breaking change to Circuit as per #4796

[GoogleFrog]

There are a bunch of files which I expect to only have one or two changes, but the diff generator decided that they have been entirely rewritten. This will not be reviewed or pulled unless the unnecessary changes are removed.

[XNTEABDSC]

some unnecessary changes are removed