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

rewrite how monster attacks read spells #38585

Merged
merged 5 commits into from
Mar 8, 2020

Conversation

KorGgenT
Copy link
Member

@KorGgenT KorGgenT commented Mar 6, 2020

Summary

SUMMARY: Bugfixes "Rewrite fake_spell code to fix crazy monster spells"

Purpose of change

Fixes #33938 (hopefully?)

Describe the solution

While looking into the code that caused the above linked bug, I realized that fake_spell::get_spell() was really... messy. so much so, in fact, that it wasn't exactly clear what it did. So i rearranged it a little bit and renamed some of the variables to make it a lot clearer what's going on:
You have a couple important values. first, the bottom values, which can be a maximum of (0, min_level, min_level_override). min_level is fake_spell::level. min_level_override is actually specifically from casting spells that have additional effects; the level of the parent spell gets passed down as a minimum spell, and it's stored as a fake_spell.
The max level can either be defined in the fake_spell, or the spell's max level, whichever is lower.
I needed to also add some error checking here, just in case the minimum got to be greater than the maximum: i reset the min to be max. Therefore, the priority is as follows: <= spell max level. <= fake_spell::max level. >= 0. >= min_level_override. >= fake_spell::level.

Then, I noticed that the spell data that was loaded in for monster spell attacks was actually more of an ad-hoc code, rather than reusing the fake_spell code i implemented (which is a pretty general-use and lightweight way of storing a spell) so i made that happen.
These two things combined should overpower this bug! also it's clearer. and it reuses code.

Describe alternatives you've considered

stare into the eye of a krabgek and have my insides becomes my outsides

Testing

set a breakpoint in the load_internal function in the monster spell attack class to see that the spell it stores (a spell, not a fake spell) is the right level. It's an issue with loading; it's only loaded the one time.

Additional context

i also fixed a minor thing with the black pudding's message. Additionally, this will break 3rd-party mods that use monster spells, as I changed how the json works. It's pretty important to take a look at the other mods in repo to see if they have monster spells, though i don't think there are any. anyway, it's WIP until i confirm.

@KorGgenT KorGgenT added [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) Mods: Magiclysm Anything to do with the Magiclysm mod labels Mar 6, 2020
@KorGgenT KorGgenT changed the title [WIP] rewrite how monster attacks read spells rewrite how monster attacks read spells Mar 6, 2020
@KorGgenT
Copy link
Member Author

KorGgenT commented Mar 6, 2020

i'm taking the tests' word for it that we don't have any unvisited members, meaning there aren't any other monsters i've missed changes for.

@ZhilkinSerg ZhilkinSerg merged commit 49d6ded into CleverRaven:master Mar 8, 2020
KorGgenT pushed a commit that referenced this pull request Mar 8, 2020
ZhilkinSerg pushed a commit that referenced this pull request Mar 10, 2020
ZhilkinSerg pushed a commit that referenced this pull request Mar 15, 2020
@KorGgenT KorGgenT deleted the fix-fake-spell-again branch August 13, 2020 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Mods: Magiclysm Anything to do with the Magiclysm mod
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Black Pudding's acid attack does 63 damage, not 8 as intended by KorGgenT
2 participants