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 #4214

Merged
merged 7 commits into from
Feb 10, 2024
Merged

fix: differentiate more SPEAR and STAB flag #4214

merged 7 commits into from
Feb 10, 2024

Conversation

End3r991
Copy link
Contributor

@End3r991 End3r991 commented Feb 10, 2024

Purpose of change

Note

same as #4211 due to branch issues

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

If this is a C++ PR that modifies JSON loading or behavior:

  • Document the changes in the appropriate location in the doc/ folder.
  • If documentation for this feature does not exist, please write it or at least note its lack in PR description.
  • New localizable fields need to be added to the lang/bn_extract_json_strings.sh script if it does not support them yet.
  • If applicable, add checks on game load that would validate the loaded data.
  • If it modifies format of save files, please add migration from the old format.

@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
@scarf005 scarf005 changed the title Idk pass beer fix: differentiate more "SPEAR" and "STAB" flag Feb 10, 2024
@scarf005 scarf005 self-assigned this Feb 10, 2024
@scarf005 scarf005 changed the title fix: differentiate more "SPEAR" and "STAB" flag fix: differentiate more SPEAR and STAB flag Feb 10, 2024
@github-actions github-actions bot added docs PRs releated to docs page scripts related to game management scripts labels Feb 10, 2024
Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

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

Changes look good. I've also included migration script and documentation. Approving as local tests pass.

@scarf005 scarf005 merged commit 0d8e532 into cataclysmbnteam:main Feb 10, 2024
13 checks passed
@chaosvolt
Copy link
Member

Items overlooked, as pointed out in my change request for the previous PR:

  • lucern_hammer, whether it warrants this is subjective (json\items\melee\bludgeons.json)
  • lucern_hammerfake only 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)

Since the addition of the flag to glaives and halberds got approved then lucerne hammer might be acceptable to add it to but still potentially borderline. If not, it needs SPEAR to be replaced with STAB instead.

@chaosvolt
Copy link
Member

Oh wait no, lucerne hammers did at least get the treatment so it's just Magiclysm items.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs PRs releated to docs page JSON related to game datas in JSON format. mods PR changes related to mods. scripts related to game management scripts src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants