-
Notifications
You must be signed in to change notification settings - Fork 46
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
Make "Starting AoE Caster Item" guarantee that an AoE attack magic item exists (if possible) #1098
base: dev
Are you sure you want to change the base?
Conversation
…he caster item magic pool is set to "None"
… is an AoE attack spell. This might be overly specific, but I do not know if a more generic function would be useful.
…ic from its new shared location
…em magic is not entirely disabled then make sure at least one item in the game has an AoE attack spell
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.
You can test your change by generating several seeds or placing breakpoints in the code you added. You could add a temporary loop to the feature so it keep running and you can make sure the result is always what's intended.
@@ -82,6 +82,16 @@ public void ShuffleItemMagic(MT19337 rng, Flags flags) | |||
Spells = GetAllSpells(rng); | |||
} | |||
|
|||
if ((bool)flags.StartingEquipmentRandomAoe && flags.ItemMagicMode != ItemMagicMode.None) { |
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.
so that doesn't seem to do what is intended, the aoe spell is placed at the end of the list so it will never be picked, and because it doesn't discriminate it could introduce back excluded aoe spells (like nuke in balanced)
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.
That's a good point. Is there an existing "exclude spells" utility which could be used to filter the list before picking a new random spell?
And yes, I did think that we may be required to also remove a spell from the pool, good to know.
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.
as madmartin said you don't need to create a new list, the generated list has already all the valid spells, you need to find the aoe's in that list and move it up
} | ||
} | ||
} | ||
using System.ComponentModel; |
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.
seems formatting was accidentally changed for this file, you should revert it and add back your code
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.
The formatting wasn't accidentally changed, I deliberately normalized the indentation because it mixed tabs and spaces and I couldn't read it well enough to understand and make the changes.
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.
if you want to update formatting you should do it in a separate pr, mixing it up makes it difficult to see what changed
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.
The changes are in separate commits. If you want to see the non-whitespace changes, look at the individual commits. That's a good idea in any case, to see what changed and why at each step.
I'll explain how the code works. If you randomize item magic and for example pick "support" magic as the pool, then there will be 42(including duplicates) Spells in the pool. The pool is shuffled. There are 12(not fixed) items that want magic. So the first 12 Spells from the shuffled pool will be assigned to the items.
In case there really isn't and aoe spell in the pool, I'd personally fail with a message(to reroll). It can only happen in combination with spellcrafter, probably will never happen and if it does a new seed will fix it. |
i'm pretty sure spellcrafter has guarantee aoe elemental spells anyway, so there's no scenario where the spellbook has no aoe spells |
Yes, but the Support Pool does not include InflictDamage spells, only InflictStatus and Buffs. But Spellcrafter is pretty much guaranteed to generate at least one AoE InflictStatus. |
This has definitely happened in the while, though it probably is quite rare |
I think this will work, but it's hard for me to test. The scenario is "we generated a random set of spells for item magic, none of which happened to include a spell in the 'AoE attack' set". This code should, in that case, pick a random AoE attack spell and add it to the list to be applied to items.
It's arguable that if it does so it should also remove a random spell from the list so that the total number of items with magic does not change.