-
Notifications
You must be signed in to change notification settings - Fork 112
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
Fixed spell number calculation for spontaneous casters #923
base: main
Are you sure you want to change the base?
Fixed spell number calculation for spontaneous casters #923
Conversation
return spellbook.SureKnownSpells(level).Where(sp => sp.IsTemporary | ||
|| sp.CopiedFromScroll | ||
|| sp.IsFromMythicSpellList | ||
|| sp.SourceItem != null | ||
|| sp.SourceItemEquipmentBlueprint != null | ||
|| sp.SourceItemUsableBlueprint != null | ||
|| sp.IsMysticTheurgeCombinedSpell).Count(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mcb5637 - I inverted your boolean here to make the count easier to get.
if(spellbook1.Blueprint.Spontaneous) { | ||
actual -= CasterHelpers.CountExternallyAddedSpells(spellbook1, index); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to test the heck out of this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No arguments here. Seems good in theory, but theory and practice are totally different things.
636771a
to
abd67de
Compare
8427dc9
to
2daea8b
Compare
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.