Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sim: remove redundant KillUnit() parameter #963

Merged

Conversation

nbusseneau
Copy link
Contributor

Previously, KillUnit() (and its variants) exposed an optional showDeathSequence parameters that defaulted to true, and if set to false then no death script was run.

In this commit, we remove the parameter as it is entirely redundant:

  • The 3rd mandatory parameter reclaimed is used for exactly the same purpose: if reclaimed is true, then no death script is run.
  • The default value of showDeathSequence is true and none of the callers actually set it to false, except 1, but this one also sets reclaimed to true... thereby overriding showDeathSequence.

doc/changelog.txt Outdated Show resolved Hide resolved
Previously, `KillUnit()` (and its variants) exposed an optional
`showDeathSequence` parameters that defaulted to `true`, and if set to
`false` then no death script was run.

In this commit, we remove the parameter as it is entirely redundant:

- The 3rd mandatory parameter `reclaimed` is used for exactly the same
  purpose: if `reclaimed` is `true`, then no death script is run.
- The default value of `showDeathSequence` is `true` and none of the
  callers actually set it to `false`, except 1, but this one also sets
  `reclaimed` to `true`... thereby overriding `showDeathSequence`.
@nbusseneau nbusseneau force-pushed the pr/remove-redundant-killunit-param branch from 77acc9c to ee146a7 Compare August 21, 2023 17:08
@nbusseneau nbusseneau requested a review from sprunk August 21, 2023 17:08
@sprunk sprunk merged commit b9505e1 into beyond-all-reason:BAR105 Aug 22, 2023
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.

2 participants