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

Bodyswap Improvements and Additions #1164

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

Crystalwarrior
Copy link
Contributor

  • If bodyswap is ran with no arguments, bring up a list of units on the map to bodyswap to
  • Bodyswapping adjusts the extra members list, not core party - consistent with vanilla behavior for retiring/unretiring with core party.
  • Reveal tiles near the bodyswapped unit
  • When bodyswapping to someone with no name, forcibly give them a name

… map to bodyswap to

Bodyswapping adjusts the extra members list, not core party - consistent with vanilla behavior for retiring/unretiring with core party.
Reveal tiles near the bodyswapped unit
bodyswap.lua Outdated Show resolved Hide resolved
@myk002
Copy link
Member

myk002 commented Jun 14, 2024

pre-commit.ci autofix

Copy link
Member

@myk002 myk002 left a comment

Choose a reason for hiding this comment

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

remember to update the bodyswap.rst docs with the new behavior

bodyswap.lua Outdated Show resolved Hide resolved
bodyswap.lua Outdated Show resolved Hide resolved
@myk002
Copy link
Member

myk002 commented Jun 25, 2024

I agree with subsuming linger into bodyswap. Remember to tombstone the old command, though!

docs/bodyswap.rst Show resolved Hide resolved
@@ -14,15 +14,27 @@ Usage
::

bodyswap [--unit <id>]
bodyswap [--linger]
Copy link
Member

Choose a reason for hiding this comment

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

I prefer the form you wrote in the docs below (and in the tombstone entry): bodyswap linger, not bodyswap --linger

in that case, you should write the usage as bodyswap linger, not bodyswap [linger], since the zero option case is already covered by the first usage line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh crap yah I prefer bodyswap linger too, I'm just dumb about the arguments and dunno how to handle it properly

bodyswap.lua Show resolved Hide resolved
changelog.txt Outdated Show resolved Hide resolved
Comment on lines 16 to 17
bodyswap unit <id>
bodyswap linger
Copy link
Member

Choose a reason for hiding this comment

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

This usage block should be:

    bodyswap [--unit <id>]
    bodyswap linger

--unit is an option go bodyswap when it is run without a subcommand. linger is a subcommand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh the -- part is no longer around cuz I dismantled the arg parse

Copy link
Member

Choose a reason for hiding this comment

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

you still need argparse for the --unit option

docs/bodyswap.rst Outdated Show resolved Hide resolved
bodyswap.lua Outdated Show resolved Hide resolved
bodyswap.lua Outdated Show resolved Hide resolved
bodyswap.lua Outdated
Comment on lines 7 to 13
local args = { ... }
local options = {
help = false,
unit = -1,
}

local positionals = argparse.processArgsGetopt(args, {
Copy link
Member

Choose a reason for hiding this comment

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

all arg processing should be done after the module check; it shouldn't be run when this script is loaded as a module

@@ -99,6 +99,7 @@ Template for new versions:
- `max-wave`: merged into `pop-control`
- `devel/find-offsets`, `devel/find-twbt`, `devel/prepare-save`: remove development scripts that are no longer useful
- `fix/item-occupancy`, `fix/tile-occupancy`: merged into `fix/occupancy`
- `linger`: merged into `bodyswap` as ``bodyswap linger``
Copy link
Member

Choose a reason for hiding this comment

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

needs to be moved into changelog for current version

If said unit has no name, a new name is randomly generated for it, based on the unit's race.
If no valid language is found for that race, it will use the DIVINE language.

If you run bodyswap linger, the killer is identified by examining the historical event generated
Copy link
Member

Choose a reason for hiding this comment

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

I think you need to mention that bodyswap linger must be run immediately after you have died, and that it will put you into the body of your killer.

Copy link
Member

Choose a reason for hiding this comment

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

please also remove docs/linger.rst

Takes control of the selected unit.
``bodyswap --unit 42``
Takes control of the selected unit, or brings up a list of swappable units if no unit is selected.
``bodyswap unit 42``
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
``bodyswap unit 42``
``bodyswap --unit 42``

end

function lingerAdvUnit(unit)
if not unit.flags2.killed then
Copy link
Member

Choose a reason for hiding this comment

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

use dfhack.units.isKilled(unit) instead of accessing the flag directly

end
if not slayer then
qerror("Slayer not found!")
elseif slayer.flags2.killed then
Copy link
Member

Choose a reason for hiding this comment

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

use dfhack.units.isKilled

Comment on lines +201 to +204
local slayerName = ""
if slayer.name.has_name then
slayerName = ", " .. dfhack.TranslateName(slayer.name) .. ","
end
Copy link
Member

Choose a reason for hiding this comment

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

use dfhack.units.getReadableName(slayer) to get the name of the histfig

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