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

ninja stun katana #1225

Merged
merged 11 commits into from
Jan 4, 2025
Merged

ninja stun katana #1225

merged 11 commits into from
Jan 4, 2025

Conversation

iaada
Copy link

@iaada iaada commented Jan 4, 2025

This PR is trying to diversify ninja gameplay by pushing them towards being a non-lethal antagonist. They're one of the only antagonists that has reasonable non-lethals, and I think it fits the flavor. As a side effect, this makes it much harder for ninja to murderbone.

Changelog
🆑

  • tweak: The ninja's katana now deals stun damage. 4 hits to down.
  • add: The energy katana can be activated to return to its previous damage. It gets 10 attacks before needing to be charged.

Copy link

@hivehum hivehum left a comment

Choose a reason for hiding this comment

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

i am pro stun katana. this is expanding the scope of the pr a bit but i think you also need to heavily nerf the ability for their gloves to stun people or else this pr is functionally "katana has lethal charges". im hitting request changes but the only thing you HAVE to change is my other comment, this is merely a suggestino

@iaada
Copy link
Author

iaada commented Jan 4, 2025

Reworked the PR. I created a new space katana in _Imp folder, and gave that to the ninja instead of the regular one. Much more resilient to merge issues, and leaves the old katana available for shenanigans.

Copy link

@hivehum hivehum left a comment

Choose a reason for hiding this comment

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

lgtm, rework is a cleaner implementation as well imo. ty for putting the legwork in here

edit: also you did not type your changelog correctly but I fixed that before merge. something to keep in mind in the future

@hivehum hivehum merged commit ac87029 into impstation:master Jan 4, 2025
5 checks passed
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