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

Spells: Adds optional bash_scaling fields to the SPELL type and unduplicate spells that used to require sub-spells to bash terrain #76739

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

b3brodie
Copy link
Contributor

Summary

Infrastructure "Adds bash_scaling fields to use with spells"

Purpose of change

If you wanted a spell to damage enemies and also damage the terrain, this necessitated using two separate spells with one that calls the other. Implements the min_bash_scaling, max_bash_scaling, and bash_scaling_increment to dynamically determine bash damage instead of having to implement separate spells.

Describe the solution

Only functions for attack spell types currently since its the only one I see that has a good correlation with directly bashing terrain.

Adds optional min_bash_scaling, max_bash_scaling, and bash_scaling_increment values to the spell type. If not defined these values default to 0.
The spells damage is multiplied by the calculated bash_scaling value, which starts at min_bash_scaling and moves towards max_bash_scaling depending on the bash_scaling_increment. If the random damage flag is used then a random bash scaling is chosen between the min and max.

Largely duplicates the bash effects code for attack: Cannot reuse the bash function because to pass the bash scaling value the parameters would have to be modified, and since its part of a map every spell effect type would have to be altered. Cannot alter the bash effect to check if bash_scaling is <= 0 to determine if the attack should bash or not because that would effect that actual bash effect too.

Adds the bash scaling fields to all of the spells that utilize the duplicate spell definitions to bash terrain that I found in aftershock, magiclysm, mom, and xedra.

Describe alternatives you've considered

Adding a bash flag: Doesn't allow for choosing between specific bash damage values.
Hooking bash scaling up to other spell effects. None really seemed to fit, but if there's any suggestions it would be easy to do.

Testing

Created a character with each mod and tested all the spells.
Ex: MOM:
mind hammer still wrecks

Additional context

@github-actions github-actions bot added <Documentation> Design documents, internal info, guides and help. [JSON] Changes (can be) made in JSON Mods Issues related to mods or modding Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies [C++] Changes (can be) made in C++. Previously named `Code` Monsters Monsters both friendly and unfriendly. [Markdown] Markdown issues and PRs Mods: Magiclysm Anything to do with the Magiclysm mod Mechanics: Enchantments / Spells Enchantments and spells Mods: Xedra Evolved Anything to do with Xedra Evolved Mods: Mind Over Matter Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style labels Sep 30, 2024
@github-actions github-actions bot added Mods: Aftershock Anything to do with the Aftershock mod astyled astyled PR, label is assigned by github actions labels Sep 30, 2024
@RedMisao
Copy link
Contributor

Three questions:

  1. What's the difference or advantages in using this bash in the same spell instead of chaining a bash spell?
  2. If min_bash and max_bash are not used, the spell deals 0 bash damage under its aoe, or no bash damage instance at all? In other words, are all attack spells dealing 0 bash damage or the function is not called unless max_bash >= 0 ?
  3. What happens if you use negative bash values?

@github-actions github-actions bot added the json-styled JSON lint passed, label assigned by github actions label Sep 30, 2024
@b3brodie
Copy link
Contributor Author

b3brodie commented Sep 30, 2024

Three questions:

  1. What's the difference or advantages in using this bash in the same spell instead of chaining a bash spell?
  2. If min_bash and max_bash are not used, the spell deals 0 bash damage under its aoe, or no bash damage instance at all? In other words, are all attack spells dealing 0 bash damage or the function is not called unless max_bash >= 0 ?
  3. What happens if you use negative bash values?
  1. The primary advantage is that using bash_scaling is 1-3 lines of code to write, in comparison with the current setup where a user has to write up to 30 lines of essentially duplicate code. In terms of functional effect there should be no difference, at least in all the spells I have seen in the current codebase.
  2. This gets into the confusing terminology around an attack spell that deals bash damage and a bash spell. Of these two spell types, the attack spell that deals bash damage will damage enemies (with bash damage) but not damage the terrain. The bash spell effect will damage the terrain but not damage enemies. Bash Scaling links the bash spell type to the attack spell type. If bash scaling is not defined then the attack spell will do zero damage to the terrain, but will have no change to the amount of bash damage to enemies that it may have defined. Codewise, the extra part to cause bash damage to the terrain is not called at all unless bash_scaling() > 0.
  3. Mentioned prior, but unless bash_scaling() > 0 nothing happens. This probably means you could have a min bash scaling of -1 and a max bash scaling of 1 and then it would only bash terrain based on how the spell is set up to move between those numbers, but generally negative numbers don't do anything.

@Psithief
Copy link
Contributor

Is this related to XAEA_line at all? I can't figure out if that one is supposed to do terrain and creature damage, or just terrain damage.

@b3brodie
Copy link
Contributor Author

Is this related to XAEA_line at all? I can't figure out if that one is supposed to do terrain and creature damage, or just terrain damage.

I didn't touch the line spell because I'm not sure why it casts the bash spell about 6 additional times. Potentially the line could be converted over though if I knew why it did that. For my testing though the line does do terrain and creature damage

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style <Documentation> Design documents, internal info, guides and help. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions [Markdown] Markdown issues and PRs Mechanics: Enchantments / Spells Enchantments and spells Mods: Aftershock Anything to do with the Aftershock mod Mods: Magiclysm Anything to do with the Magiclysm mod Mods: Mind Over Matter Mods: Xedra Evolved Anything to do with Xedra Evolved Mods Issues related to mods or modding Monsters Monsters both friendly and unfriendly. Mutations / Traits / Professions/ Hobbies Mutations / Traits / Professions/ Hobbies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants