dnd-character icon indicating copy to clipboard operation
dnd-character copied to clipboard

Add Spellbook Functionality

Open stevebelew opened this issue 1 year ago • 1 comments

Hi,

This pull request introduces the Spellbook class along with methods for adding, removing, and validating spells based on D&D 5e rules. Additionally, it includes tests to ensure the correct functionality of these methods.

  • Added Spellbook class in a new file spellbook.py.
  • Implemented methods add_spell, remove_spell, validate_spell, and others to manage spells in a spellbook.
  • Added test_spellbook.py with tests for various scenarios, ensuring the correct behavior of the spellbook methods.
  • All test cases pass, verifying the implementation aligns with the expected behavior.

Your review and feedback are highly appreciated.

stevebelew avatar Oct 04 '23 02:10 stevebelew

Thanks!

I like how you added the scribe cost in gold pieces... I forgot about that. However, this cost doesn't apply when leveling up -- it's only for copying spells out of another spellbook, even though it makes no logical sense. That's game design trumping logic, I suppose. Maybe there should be a separate copy_spell function?

Your fetch_wizard_spells_from_json function is actually duplicating work done elsewhere. The spellcasting module has functions already for getting spell lists.

from dnd_character.spellcasting import spells_for_class_level
wizard_spells_by_level = {i: spells_for_class_level("wizard", i) for i in range(9)}

(That is a "dict comprehension")

The only difference between this code and your existing code is: that this wizard_spells_by_level has indexes instead of names (e.g., "acid-arrow" instead of "Acid Arrow"). You can then get the spell data from SPELLS['acid-arrow'] (also in spellcasting module)

I would recommend using these spell objects from SPELLS in your test function too, rather than using mocks. Mocks are typically for expensive resources such as those returned from the internet. (An example would be, mocking S3 storage to pretend that a file failed to upload, then testing how the program reacts to that failure which wouldn't otherwise have occurred.)

Rename run_tests to test_spellbook, and remove the final line where you call that function -- pytest (the test runner) automatically runs any function that starts with test_

Otherwise I don't see other significant issues. I'll be happy to help more in the future :)

tassaron avatar Oct 05 '23 01:10 tassaron