Skip to content

Add ModifyMaxSquadSize DLC hook - #954

Draft
robojumper wants to merge 4 commits into
X2CommunityCore:masterfrom
robojumper:953-squad-size
Draft

robojumper wants to merge 4 commits into
X2CommunityCore:masterfrom
robojumper:953-squad-size

Conversation

@robojumper

@robojumper robojumper commented Jan 13, 2021 •

Copy link
Copy Markdown
Member

I personally don't plan to publish any mod depending on this, but I think this could come in helpful for a mod that gives more control over squad size overall (base size, squad size upgrades, special missions).

396b40b...e33a82f

@robojumper robojumper added the ready-to-review A pull request is ready to be reviewed label Jan 13, 2021
@Xymanek

Xymanek commented Feb 11, 2021

Copy link
Copy Markdown
Member

Why a DLCInfo hook and not an event?

@Iridar Iridar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't have strong objections to anything in this PR, and a hook like that is no doubt gonna come in handy on many occasions, although same as Xymanek, I wonder if perhaps making this a delegate method (like the one Xym and I made for #889) would have been better.

Although on second thought, delegates have more cumbersome setup, while mission squad size is not particularly hot code, so it's probably okay to have it as a DLC hook.

///
/// The final max squad size will be calculated with the expression `Min(Max(1, MaxBaseSize + SquadSizeUpgradeMod + SituationMod), SitRepMax)`.
///
/// This hook is best combined with `GetValidFloorSpawnLocations` or `SPAWN_EXTRA_TILE` (TODO: Docs for these) in order to support

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure we'd want to ship a "TODO", but I assume this is here just because it's a Draft PR? It is marked as "ready for review", so I'm getting some mixed signals here.

///
/// if (MissionSite != none && MissionSite.GeneratedMission.Mission.MissionName == 'AssaultFortressLeadup')
/// {
/// // Leviathan gets four more soldiers because reasons

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Personally I had no trouble following these docs and I find the provided explanations perfectly sufficient, but perhaps they should be a bit lighter on the joke-ish wordings like "cargo cult" and "because reasons".

@Iridar Iridar added this to the 1.27.0 milestone Aug 10, 2023
@Iridar Iridar modified the milestones: 1.27.0, 1.28.0 Oct 29, 2023
@Iridar Iridar modified the milestones: 1.28.0, 1.29.0 May 1, 2024

This branch has not been deployed

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

Labels

ready-to-review A pull request is ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants