Skip to content

Conversation

@XNTEABDSC
Copy link
Contributor

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.

@@ -0,0 +1,245 @@
return {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in file name.

@GoogleFrog
Copy link
Contributor

GoogleFrog commented Nov 26, 2025

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
Copy link
Contributor

Also it should be indented with tabs.

@XNTEABDSC
Copy link
Contributor Author

Updated. Removed global ModularCommDefsShared and SharedEnv and used tabs

@GoogleFrog GoogleFrog requested a review from sprunk December 6, 2025 06:08
@sprunk
Copy link
Member

sprunk commented Dec 10, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants