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(balance): throwing weapons sanity-checking #5474

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

chaosvolt
Copy link
Member

@chaosvolt chaosvolt commented Sep 30, 2024

Checklist

Required

Optional

Purpose of change

followup of #5088.

Scarf said to just PR this as a draft to look at. WIP because this fucking nonsense is producing wildly different results despite seemingly near-identical input:
image

Also the item.cpp version of the function is fucking up and adding the weight of the weapon the ammo is loaded into, but we're not even remotely close to fixing that fuckery just yet.

Describe the solution

Describe alternatives you've considered

Testing

Additional context

@github-actions github-actions bot added the src changes related to source code. label Sep 30, 2024
Copy link
Contributor

autofix-ci bot commented Sep 30, 2024

Autofix has formatted code style violation in this PR.

I edit commits locally (e.g: git, github desktop) and want to keep autofix
  1. Run git pull. this will merge the automated commit into your local copy of the PR branch.
  2. Continue working.
I do not want the automated commit
  1. Format your code locally, then commit it.
  2. Run git push --force to force push your branch. This will overwrite the automated commit on remote with your local one.
  3. Continue working.

If you don't do this, your following commits will be based on the old commit, and cause MERGE CONFLICT.

@chaosvolt
Copy link
Member Author

So, throwing rocks at 12 in stats and 10 throwing skill deals about 12 damage prior to this change, and about 19 with the change. I'm trying to figure out where I bungled the math in the process of writing this.

@chaosvolt
Copy link
Member Author

Oh no. This is with 8 in all stats and zero skills.
image

So all those fixes have since made the math even more wrong that they were before, this used to be 6+16.

@chaosvolt
Copy link
Member Author

Okay, tests pass after giving it a second look, and new testing done:

In build 2024-09-30:
* Rock, no skill, 8 in stats: Throwing range 16 tiles, damage 8-10.
* Rock, 10 skills, 12 in stats: Throwing range 24 tiles, damage 15-17.
* Iron javelin, no skill, 8 in stats: Throwing range 7 tiles, damage 23-26.
* Iron javelin, 10 skills, 12 in stats: Throwing range 11 tiles, damage 36-43.
Current damage ranges seen, tested by tossing at a debug monster:
* Rock, no skill, 8 in stats: Throwing range 16 tiles, damage 8-10.
* Rock, 10 skills, 12 in stats: Throwing range 24 tiles, damage 15-17.
* Iron javelin, no skill, 8 in stats: Throwing range 7 tiles, damage 23-27.
* Iron javelin, 10 skills, 12 in stats: Throwing range 11 tiles, damage 36-43.
Sling claims the throwing damage bonus is actually:
* No skill, 8 in stats: 24 (16 expected)
* 10 skills, 12 in stats: 30 (23 expected)

So it seems Scarf did succeed in fixing the damage values, it's just the UI is displaying them very wrongly for some reason.

How was it that simple this whole fucking time aaaaa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants