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

fix: differentiate more "SPEAR" and "STAB" flag #4211

Closed
wants to merge 4 commits into from
Closed

fix: differentiate more "SPEAR" and "STAB" flag #4211

wants to merge 4 commits into from

Conversation

End3r991
Copy link
Contributor

Purpose of change

Splitting up "SPEAR" flag and "STAB" flag so they can be used separately, allowing the ability to reach attack through bars with cutting weapons that should be able to do it.

Describe the solution

"SPEAR" flag does not transform cut damage into pierce damage anymore, still allowing the ability to attack through bars with reach weapons, while only "STAB" flag change cut damage into pierce damage. Added the "STAB" flag to all weapons that previously had only "SPEAR" flag, so they continue do pierce damage, and added the "SPEAR" flag to some weapons that should be able to reach attack through bars, while still doing cut damage.

Describe alternatives you've considered

Not adding the "SPEAR" flag to any cut damage weapon, waiting for an actual flag/technique (need hardcode) to allow to cut damage weapons to do pierce damage while performing reach attack through bars.

Testing

Weapons with adjusted "SPEAR" and "STAB" flag work as inteded.

Additional context

Ideally, it would have more sense that weapons that do cut damage and can reach attack through bars, would do pierce damage while attacking through bars, and not continuing doing cut damage, but that would need follow-up PR to make it possible.

Checklist

@github-actions github-actions bot added src changes related to source code. JSON related to game datas in JSON format. mods PR changes related to mods. labels Feb 10, 2024
Copy link
Contributor

autofix-ci bot commented Feb 10, 2024

The Autofix app has found code style violation and automatically formatted this Pull Request.

I locally edit my commits (e.g: git, github desktop)

Please choose following options:

I'd like to accept the automated commit
  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.

This PR is complete and I don't want to edit it anymore

It's safe to ignore this message.

I edit this PR through web UI

You can ignore this message and continue working.

I have no idea what this message is talking about

You can ignore this message and continue working. If you find any problem, please ask for help and ping @scarf005.

@chaosvolt
Copy link
Member

The basic code idea is fine, should 100% work and give us freedom to play around with the flags more. I'm gonna want to double-check for any items that will need updating that this missed, will list the item IDs and file locations for them after i find them all (or maybe I'll just checkout the branch and update it myself, heh).

Only real point of contention is whether it's vital that the SPEAR flag automatically convert reach attacks through poles into stab-type damage when attacking through obstacles, and if so whether that's actually vital to add now or whether it can wait for a follow-up PR. It being more realistic that way is...honestly really the only functional advantage that change would have.

Game-balance wise, stab armor is basically always lower than cutting armor except for whatever few monsters are for whatever reason set to ignore stabbing but crumple if slashed. So a 1:1 conversion would be even more busted OP than a full-power glaive strike through metal bars would already by. Even if there was some loss in damage, you'd have to have it bake in a damage multiplier drastically lower than 0.8 to really compensate for this for basically any amount of enemy armor. I can see it being okay to add if we feel like it but not really a priority balance-wise.

Copy link
Member

@chaosvolt chaosvolt left a comment

Choose a reason for hiding this comment

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

Items that still need both flags applied to them:

  • lucern_hammer, if you feel it should allow it (json\items\melee\bludgeons.json)
  • lucern_hammerfake if given to regular lucerne hammer (json\items\melee\bludgeons.json)
  • rune_biomancer_weapon (mods\Magiclysm\items\enchanted_melee.json)
  • bonespear (mods\Magiclysm\items\ethereal_items.json)
  • lizardfolk_trident (mods\Magiclysm\items\weapons.json)

Seems to thankfully be a short list.

@scarf005
Copy link
Member

superseded by #4214

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. mods PR changes related to mods. src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants