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

Remove AoE from buckshot and flechette ammo until it's fixed #2792

Merged
merged 1 commit into from
May 13, 2023

Conversation

chaosvolt
Copy link
Member

@chaosvolt chaosvolt commented May 11, 2023

Summary

SUMMARY: Balance "Remove AoE cone effects for buckshot and flechette ammo for now"

Purpose of change

Discovered while testing, shotgun AoE cones are in a bad state right now and not suitable for use with standard ammo in its current form. Its flaws aren't severe enough to be a problem with birdshot (which no sane person is going to use on NPCs anyway since even if it was function the base damage is too low to be worth it), but for buckshot it's a big problem.

Relevant bugs are:

  1. AoE shotgun ammo reports hitting appendix #2791, this is the big one that makes this feature unsuitable for use with buckshot.
  2. AoE cone ammo does not apply ammo_effects properly. #2756, this would be a good reason to revert this from dragon's breath shells too, but I feel this is less severe of a problem because players are also quite unlikely to use dragon's breath on NPCs that often either.

Describe the solution

Reverted use of AoE cone for all buckshot ammo, giving it its old stats. I left the abstract inheritance from #2619 in for now instead of just reverting the PR wholesale, because it'll be useful to us to have those abstracts so it's less work to re-implement this when it's fixed.

I kept the nerf to flintlock flechette ammo since it having 60 range (having inherited from the base ammo with no range override) seems like a mistake, but I did not keep the massive boost to paper shot's range since that seems to be a copypaste issue. Then again, it seems AoE ammo ALSO doesn't gain anything from range bonuses on the gun (a third bug for me to report it seems) so it having double the range of regular paper ammo may have been partially intentional, since that would equal the range of paper cartridge through a flintlock musket.

Describe alternatives you've considered

  1. Removing AoE from dragon's breath, if we decided the second bug related to it is annoying enough to justify not using the feature until it's fixed.
  2. Making AoE used only for debug ammo until either or both bugs are solved.

Testing

  1. Checked affected file for syntax and lint errors.
  2. Load-tested in compiled build.
  3. Blasted a naked NPC 1 tile with a shotgun after granting myself 10 in all skills, they take 78 damage to the head and barely survive.
  4. Follow this up with two unaimed shots, both hit the torso for standard damage and this is adequate to kill them.

Additional context

Relevant PR, this partially reverts changes from: #2619

@github-actions github-actions bot added the JSON related to game datas in JSON format. label May 11, 2023
@Firestorm01X2
Copy link
Collaborator

Firestorm01X2 commented May 11, 2023

Summoning @Coolthulhu . Need advice.
Probably feature is regressed after two years being disabled.
I am not sure that disabling is good idea- we may forget about AoE feature forever. We almost did already.

On the other hand considering its condition maybe it is good idea to disable it.

@scarf005
Copy link
Member

we could wait for a week until a fix comes out.

@chaosvolt
Copy link
Member Author

It depends on how hard it'd be to actually fix the underlying issues, I suppose?

@Firestorm01X2
Copy link
Collaborator

For now I recommend to keep it hold until @Coolthulhu decision.

@Coolthulhu
Copy link
Member

I'm fine with removing AoE effects from "real ammo", it's still in "not done yet" phase.

Copy link
Collaborator

@Firestorm01X2 Firestorm01X2 left a comment

Choose a reason for hiding this comment

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

Ready

@Firestorm01X2 Firestorm01X2 merged commit b3377c9 into cataclysmbnteam:upload May 13, 2023
@chaosvolt chaosvolt deleted the no-aoe-until-fix branch May 13, 2023 17:26
@scarf005 scarf005 mentioned this pull request Jul 7, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JSON related to game datas in JSON format. require-feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants