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

feat: fix and add STR_RELOAD to crossbows #3575

Merged
merged 1 commit into from
Nov 6, 2023

Conversation

chaosvolt
Copy link
Member

Summary

SUMMARY: Balance "Sanity-check behavior of STR_RELOAD flag, re-add it to crossbows"

Purpose of change

So it was a while back brought to my attention that crossbows don't seem to be benefiting from strength anyway. I went into this assuming that the STR_RELOAD flag doesn't work, and on looking at its code it is indeed a bit janky and weird-looking.

Then I eventually found the real problem: most crossbows had the flag removed from them ages ago, but their description still implies they should be using them. Instead only a few items use them. There was a "fix" on DDA's end that just amounted to removing it from most weapons and fixing the description, but this fix was still pretty half-baked. Yes, in theory, drawing the crossbow to full draw is only part of the reloading process and being super-strong will only help with that part of the process.

But the rest of those steps should not take any longer than the usual 100-300 moves it takes to insert ammo into an item. Reload times for crossbows do not reflect this, instead they take several hundreds of moves and are clearly designed with the assumption that a weak character will be struggling their way through reloading, while a strong character won't take much longer than it'd take to physically pull something back and slip a bolt in. Needless to say, their "fix" for this did not account for this concept, and you still see fixed reload rates in excess of a thousand moves on DDA's crossbows to this very day.

So while I do want to make the STR_RELOAD code a bit less weird behavior-wise, the bulk of the fix is just making more weapons actually have it.

Describe the solution

C++ changes:

  1. In character.cpp, changed Character::item_reload_cost to go from subtracting 20 moves for every point of strength you have to a multiplicative function. If you have 10 strength or less it does nothing, but beyond that it reduces reload rates up until it maxes at cutting it into a quarter, hit at 40 strength.

Idea is, the fixed subtractive method is simple and all but it also rapidly pushes a light enough crossbow into "ZA WARUDO" territory, meaning the end of the function caps it off at a quarter of a second after relatively low amounts of strength. Meanwhile, for massive heavy crossbows you need increasingly wacky amounts of super strength to see a reasonable benefit from them, as you'd need that 40 strength (normally the peak for a lot of mutant builds, plus or minus some Hydraulic Muscles or mod abuse). This way however, having 20+ strength is immediately and visibly useful even for heavy crossbows, while still ensuring weak baby child crossbows will still take take a second or two to load.

JSON changes:

  1. Gave the STR_RELOAD flag directly to most crossbows via slapping it on xbow_base.
  2. In turn, removed the injection of this flag from bullet crossbows, as it will inherit the flag from its copy-from anyway.
  3. Reduced the base reload of s_xbow_base, and as a result pistol crossbows, from 1200 to 600. Having them take even more effort to reload than normal crossbows while dealing less damage is a bit weird, but its damage isn't THAT much lower than the basic medieval crossbow so doesn't need to be reduced to a downright tiny amount.
  4. Set the repeating crossbow to delete the STR_RELOAD flag from itself, and reduced reload rate from 800 down to 200. It's noted to be drawn and fired using the lever, reloading instead loads bolts into a magazine, so it needing to take 8 seconds per bolt to reload is a tad odd.

Describe alternatives you've considered

  1. Given the above musings on repeating crossbows, we could tweak it to have a slower aim spread to simulate the need to cock the lever and span the crossbow during the firing process.
  2. We could also audit base reload times either further. If we made it so that strength below 10 is actually penalized and slower to reload, we'd need to do this so that the base reload time is made to be representative of how fast we want it to be at exactly the "baseline" level of strength.
  3. Making the baseline point 8 instead of 10 strength.

Testing

  1. Checked affected JSON files for syntax and lint errors.
  2. Compiled and load-tested.
  3. Spawned in a basic crossbow, and two bolts, one on the floor and another loose in inventory.
  4. Checked affected C++ file for astyle.

With both 1 and 10 strength (confirming strength below 10 doesn't currently penalize player):
image

Was done to match the test setup I used in my desktop playthrough release.

20 strength:
image

It's not quite half but other functions do stuff with Character::item_reload_cost so chicanery is to be expected.

Additional context

Relevant PRs and issues:

  1. DDA issue reporting STR_RELOAD no longer works, by @Dacendeth: Strength doesnt improve crossbow reload speed. CleverRaven/Cataclysm-DDA#40704
  2. DDA's "fix" for this, removing it from most weapons, by @kevingranade: STR_RELOAD audit CleverRaven/Cataclysm-DDA#40730

Also gonna at some point tweak bows to in some way benefit from something like this, evidently it works differently from STR_RELOAD weapons so might as well save that ofr a later PR.

Bonus, along the way for some reason the reload time for quivers differs now and is slower than reloading from inventory and off the floor.

In build 2023-10-24-1439:
image

Contrast with build 2023-11-05-1454, and also this PR's branch:
image

@github-actions github-actions bot added src changes related to source code. JSON related to game datas in JSON format. labels Nov 6, 2023
@scarf005 scarf005 changed the title Sanity-check STR_RELOAD and use it in crossbows again feat: fix and reintroduce STR_RELOAD for crossbows Nov 6, 2023
@scarf005 scarf005 changed the title feat: fix and reintroduce STR_RELOAD for crossbows feat: fix and add STR_RELOAD to crossbows Nov 6, 2023
@scarf005 scarf005 self-requested a review November 6, 2023 09:57
@scarf005 scarf005 added this pull request to the merge queue Nov 6, 2023
Merged via the queue into cataclysmbnteam:upload with commit 6f13082 Nov 6, 2023
14 of 17 checks passed
@chaosvolt chaosvolt deleted the yes-str-reload branch November 7, 2023 01:08
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. src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants