Fixed spell number calculation for spontaneous casters#923
Open
BuckAMayzing wants to merge 8 commits intocabarius:mainfrom
Open
Fixed spell number calculation for spontaneous casters#923BuckAMayzing wants to merge 8 commits intocabarius:mainfrom
BuckAMayzing wants to merge 8 commits intocabarius:mainfrom
Conversation
BuckAMayzing
commented
May 25, 2023
Comment on lines
200
to
206
| return spellbook.SureKnownSpells(level).Where(sp => sp.IsTemporary | ||
| || sp.CopiedFromScroll | ||
| || sp.IsFromMythicSpellList | ||
| || sp.SourceItem != null | ||
| || sp.SourceItemEquipmentBlueprint != null | ||
| || sp.SourceItemUsableBlueprint != null | ||
| || sp.IsMysticTheurgeCombinedSpell).Count(); |
Contributor
Author
There was a problem hiding this comment.
@mcb5637 - I inverted your boolean here to make the count easier to get.
BuckAMayzing
commented
May 25, 2023
Comment on lines
203
to
206
| if(spellbook1.Blueprint.Spontaneous) { | ||
| actual -= CasterHelpers.CountExternallyAddedSpells(spellbook1, index); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
In theory, we shouldn't have to worry about this on non-spontaneous spell books. If that's not the case, let me know. I think doing this for all spell books might cause extra spells to be learned for memorized books, though?
Owner
There was a problem hiding this comment.
We need to test the heck out of this
Contributor
Author
There was a problem hiding this comment.
No arguments here. Seems good in theory, but theory and practice are totally different things.
cabarius
reviewed
May 25, 2023
This was referenced May 26, 2023
BuckAMayzing
force-pushed
the
fix-spontaneous-spell-calculations-for-scrolls
branch
2 times, most recently
from
636771a to
abd67de
Compare
BuckAMayzing
force-pushed
the
fix-spontaneous-spell-calculations-for-scrolls
branch
2 times, most recently
from
8427dc9 to
2daea8b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please don't merge until reviewed by @mcb5637 and @cabarius. I'd like you both to take a quick look at the logic and double-check it.
It might also be worthwhile to pull this branch down and build just to verify that it's working fine for your test cases. It has worked for mine so far; I will check a couple more.
I didn't change the patch notes; this is more or less just the fix @mcb5637 put in, but done in a way that shouldn't have any unintended side effects.