add spell inventory ui, refined level up ui, fixed bugs

src/ttfrog/db/bootstrap/loader.py

@@ -20,8 +20,10 @@ def load(data: str = ""):
         rogue = db.CharacterClass.filter_by(name="rogue").one()
 
         sabetha = schema.Character("Sabetha", ancestry=tiefling, _intelligence=14)
-        sabetha.add_class(fighter, level=2)
+        sabetha.add_class(fighter)
-        sabetha.add_class(rogue, level=3)
+        sabetha.add_class(rogue)
+        sabetha.level_up(fighter, 2)
+        sabetha.level_up(rogue, 3)
 
         bob = schema.Character("Bob", ancestry=human)
 

src/ttfrog/db/schema/character.py

@@ -1,3 +1,4 @@
+import itertools
 from collections import defaultdict
 
 from sqlalchemy import ForeignKey, String, Text, UniqueConstraint
@@ -7,11 +8,11 @@ from sqlalchemy.orm import Mapped, mapped_column, relationship
 from ttfrog.db.base import BaseObject, SlugMixin
 from ttfrog.db.schema.classes import CharacterClass, ClassFeature
 from ttfrog.db.schema.constants import DamageType, Defenses
-from ttfrog.db.schema.modifiers import Modifier, ModifierMixin, Stat
 from ttfrog.db.schema.inventory import Inventory, InventoryMap, InventoryType
+from ttfrog.db.schema.item import ItemType
+from ttfrog.db.schema.modifiers import Modifier, ModifierMixin, Stat
 from ttfrog.db.schema.skill import Skill
 
-
 __all__ = [
     "Ancestry",
     "AncestryTrait",
@@ -53,6 +54,17 @@ def attr_map_creator(fields):
     return CharacterClassFeatureMap(**fields)
 
 
+class SpellSlot(BaseObject):
+    __tablename__ = "spell_slot"
+    id: Mapped[int] = mapped_column(init=False, primary_key=True, autoincrement=True)
+    character_id: Mapped[int] = mapped_column(ForeignKey("character.id"))
+    character_class_id: Mapped[int] = mapped_column(ForeignKey("character_class.id"))
+    character_class = relationship("CharacterClass", lazy="immediate")
+
+    spell_level: Mapped[int] = mapped_column(nullable=False, info={"min": 1, "max": 9})
+    expended: Mapped[bool] = mapped_column(nullable=False, default=False)
+
+
 class HitDie(BaseObject):
     __tablename__ = "hit_die"
     id: Mapped[int] = mapped_column(init=False, primary_key=True, autoincrement=True)
@@ -145,14 +157,12 @@ class CharacterClassMap(BaseObject):
     __tablename__ = "class_map"
     __table_args__ = (UniqueConstraint("character_id", "character_class_id"),)
     id: Mapped[int] = mapped_column(init=False, primary_key=True, autoincrement=True)
-    character: Mapped["Character"] = relationship(uselist=False, viewonly=True)
+    character_id: Mapped[int] = mapped_column(ForeignKey("character.id"), nullable=False)
-    character_class: Mapped["CharacterClass"] = relationship(lazy="immediate")
+    character_class_id: Mapped[int] = mapped_column(ForeignKey("character_class.id"), nullable=False)
-
-    character_id: Mapped[int] = mapped_column(ForeignKey("character.id"), init=False, nullable=False)
-    character_class_id: Mapped[int] = mapped_column(ForeignKey("character_class.id"), init=False, nullable=False)
-
     level: Mapped[int] = mapped_column(nullable=False, info={"min": 1, "max": 20}, default=1)
 
+    character_class: Mapped["CharacterClass"] = relationship(lazy="immediate", init=False, viewonly=True)
+
 
 class CharacterClassFeatureMap(BaseObject):
     __tablename__ = "character_class_feature_map"
@@ -255,6 +265,35 @@ class Character(BaseObject, SlugMixin, ModifierMixin):
     inventories = association_proxy("_inventories", "id", creator=inventory_creator)
 
     _hit_dice = relationship("HitDie", uselist=True, cascade="all,delete,delete-orphan", lazy="immediate")
+    _spell_slots = relationship("SpellSlot", uselist=True, cascade="all,delete,delete-orphan", lazy="immediate")
+
+    @property
+    def spells(self):
+        return [inv for inv in self._inventories if inv.inventory_type == InventoryType.SPELL][0]
+
+    @property
+    def prepared_spells(self):
+        hashmap = dict([(mapping.item.name, mapping) for mapping in self.spells if mapping.prepared])
+        return list(hashmap.values())
+
+    @property
+    def spell_slots(self):
+        return list(itertools.chain(*[slot for lvl, slot in self.spell_slots_by_level.items()]))
+
+    @property
+    def spell_slots_by_level(self):
+        pool = defaultdict(list)
+        for slot in self._spell_slots:
+            pool[slot.spell_level].append(slot)
+        return pool
+
+    @property
+    def spell_slots_available(self):
+        available = defaultdict(list)
+        for slot in self._spell_slots:
+            if not slot.expended:
+                available[slot.spell_level].append(slot)
+        return available
 
     @property
     def hit_dice(self):
@@ -333,6 +372,10 @@ class Character(BaseObject, SlugMixin, ModifierMixin):
     def levels(self):
         return dict([(mapping.character_class.name, mapping.level) for mapping in self.class_map])
 
+    @property
+    def spellcaster_level(self):
+        return max(slot.spell_level for slot in self.spell_slots)
+
     @property
     def class_features(self):
         return dict([(mapping.class_feature.name, mapping.option) for mapping in self.character_class_feature_map])
@@ -353,9 +396,34 @@ class Character(BaseObject, SlugMixin, ModifierMixin):
         mapping.equipped = False
         return True
 
-    @property
+    def prepare(self, mapping):
-    def spells(self):
+        if mapping.item.item_type != ItemType.SPELL:
-        return [inv for inv in self._inventories if inv.inventory_type == InventoryType.SPELL][0]
+            return False
+        if mapping.item.level > 0 and not self.spell_slots_by_level[mapping.item.level]:
+            return False
+        return self.equip(mapping)
+
+    def unprepare(self, mapping):
+        if mapping.item.item_type != ItemType.SPELL:
+            return False
+        return self.unequip(mapping)
+
+    def cast(self, mapping: InventoryMap, level=0):
+        if not mapping.prepared:
+            return False
+        if not level:
+            level = mapping.item.level
+
+        # cantrips
+        if level == 0:
+            return True
+
+        # expend the spell slot
+        avail = self.spell_slots_available[level]
+        if not avail:
+            return False
+        avail[0].expended = True
+        return True
 
     def level_in_class(self, charclass):
         mapping = [mapping for mapping in self.class_map if mapping.character_class_id == charclass.id]
@@ -407,30 +475,40 @@ class Character(BaseObject, SlugMixin, ModifierMixin):
         # return the initial value plus any modifiers.
         return self._apply_modifiers(f"{attr}_{'save' if save else 'check'}", initial)
 
-    def add_class(self, newclass, level=1):
+    def level_up(self, charclass, num_levels=1):
-        if level == 0:
+        for _ in range(num_levels):
-            return self.remove_class(newclass)
+            self._level_up_once(charclass)
+        return self.level_in_class(charclass)
 
-        # add the class mapping and/or set the character's level in the class
+    def _level_up_once(self, charclass):
-        mapping = self.level_in_class(newclass)
+        current = self.level_in_class(charclass)
-        if not mapping:
+        if not current:
-            self.class_list.append(CharacterClassMap(character=self, character_class=newclass, level=level))
+            return False
-        else:
+        current.level += 1
-            mapping.level = level
 
-        # add class features with default values
+        # add new features
-        for lvl in range(1, level + 1):
+        for feature in charclass.features_at_level(current.level):
-            for attr in newclass.features_at_level(lvl):
+            self.add_class_feature(charclass, feature, feature.options[0])
-                self.add_class_feature(newclass, attr, attr.options[0])
 
-        # add default class skills
+        # add new spell slots
+        for slot in charclass.spell_slots_by_level[current.level]:
+            self._spell_slots.append(
+                SpellSlot(character_id=self.id, character_class_id=charclass.id, spell_level=slot.spell_level)
+            )
+
+        # add a new hit die
+        self._hit_dice.append(HitDie(character_id=self.id, character_class_id=charclass.id))
+
+        return current
+
+    def add_class(self, newclass):
+        if self.level_in_class(newclass):
+            return False
+        self.class_list.append(CharacterClassMap(character_id=self.id, character_class_id=newclass.id, level=0))
+        self._level_up_once(newclass)
         for skill in newclass.skills[: newclass.starting_skills]:
             self.add_skill(skill, proficient=True, character_class=newclass)
-
+        return True
-        # add hit dice
-        existing = len([die for die in self._hit_dice if die.character_class_id == newclass.id])
-        for lvl in range(level - existing):
-            self._hit_dice.append(HitDie(character_id=self.id, character_class_id=newclass.id))
 
     def remove_class(self, target):
         self.class_map = [m for m in self.class_map if m.character_class != target]
@@ -440,6 +518,7 @@ class Character(BaseObject, SlugMixin, ModifierMixin):
             for skill in target.skills:
                 self.remove_skill(skill, proficient=True, expert=False, character_class=target)
         self._hit_dice = [die for die in self._hit_dice if die.character_class != target]
+        self._spell_slots = [slot for slot in self._spell_slots if slot.character_class != target]
 
     def remove_class_feature(self, feature):
         self.character_class_feature_map = [
@@ -455,7 +534,7 @@ class Character(BaseObject, SlugMixin, ModifierMixin):
         mapping = self.level_in_class(character_class)
         if not mapping:
             return False
-        if feature not in mapping.character_class.features_at_level(mapping.level):
+        if feature not in character_class.features_at_level(mapping.level):
             return False
         self.features.append(
             CharacterClassFeatureMap(
@@ -574,6 +653,9 @@ class Character(BaseObject, SlugMixin, ModifierMixin):
     def spend_hit_die(self, die):
         die.spent = True
 
+    def expend_sell_splot(self, slot):
+        slot.expended = True
+
     def __after_insert__(self, session):
         """
         Called by the session after_flush event listener to add default joins in other tables.
@@ -583,8 +665,6 @@ class Character(BaseObject, SlugMixin, ModifierMixin):
         ):
             self.add_skill(skill, proficient=False, expert=False)
 
-        for inventory_type in InventoryType:
+        self._inventories.append(Inventory(inventory_type=InventoryType.EQUIPMENT, character_id=self.id))
-            self._inventories.append(
+        self._inventories.append(Inventory(inventory_type=InventoryType.SPELL, character_id=self.id))
-                Inventory(inventory_type=inventory_type, character_id=self.id)
-            )
         session.add(self)

src/ttfrog/db/schema/classes.py

@@ -12,6 +12,7 @@ __all__ = [
     "ClassFeatureMap",
     "ClassFeature",
     "ClassFeatureOption",
+    "ClassSpellSlotMap",
     "CharacterClass",
     "Skill",
     "ClassSkillMap",
@@ -44,6 +45,14 @@ class ClassFeatureMap(BaseObject):
     feature = relationship("ClassFeature", uselist=False, viewonly=True, lazy="immediate")
 
 
+class ClassSpellSlotMap(BaseObject):
+    __tablename__ = "class_spell_slot_map"
+    id: Mapped[int] = mapped_column(init=False, primary_key=True, autoincrement=True)
+    character_class_id: Mapped[int] = mapped_column(ForeignKey("character_class.id"))
+    class_level: Mapped[int] = mapped_column(nullable=False, info={"min": 1, "max": 20}, default=1)
+    spell_level: Mapped[int] = mapped_column(nullable=False, info={"min": 1, "max": 9}, default=1)
+
+
 class ClassFeature(BaseObject):
     __tablename__ = "class_feature"
     id: Mapped[int] = mapped_column(init=False, primary_key=True, autoincrement=True)
@@ -81,6 +90,7 @@ class CharacterClass(BaseObject):
     starting_skills: int = mapped_column(nullable=False, default=0)
 
     features = relationship("ClassFeatureMap", cascade="all,delete,delete-orphan", lazy="immediate")
+    spell_slots = relationship("ClassSpellSlotMap", cascade="all,delete,delete-orphan", lazy="immediate")
 
     _skills = relationship("ClassSkillMap", cascade="all,delete,delete-orphan", lazy="immediate")
     skills = association_proxy("_skills", "skill", creator=skill_creator)
@@ -104,6 +114,16 @@ class CharacterClass(BaseObject):
             return True
         return False
 
+    @property
+    def spell_slots_by_level(self):
+        by_level = defaultdict(list)
+        for mapping in self.spell_slots:
+            by_level[mapping.class_level].append(mapping)
+        return by_level
+
+    def spell_slots_at_level(self, level: int):
+        return list(itertools.chain(*[mapping for lvl, mapping in self.spell_slots_by_level.items() if lvl <= level]))
+
     @property
     def features_by_level(self):
         by_level = defaultdict(list)

src/ttfrog/db/schema/inventory.py

@@ -1,6 +1,7 @@
 from sqlalchemy import ForeignKey, UniqueConstraint
 from sqlalchemy.ext.associationproxy import association_proxy
 from sqlalchemy.orm import Mapped, mapped_column, relationship
+
 from ttfrog.db.base import BaseObject, EnumField
 from ttfrog.db.schema.item import Item, ItemType
 
@@ -15,9 +16,7 @@ inventory_type_map = {
         ItemType.ITEM,
         ItemType.SCROLL,
     ],
-    InventoryType.SPELL: [
+    InventoryType.SPELL: [ItemType.SPELL],
-        ItemType.SPELL
-    ]
 }
 
 
@@ -29,7 +28,7 @@ def inventory_map_creator(fields):
 
 class Inventory(BaseObject):
     __tablename__ = "inventory"
-    __table_args__ = (UniqueConstraint("character_id", "inventory_type"), )
+    __table_args__ = (UniqueConstraint("character_id", "inventory_type"),)
     id: Mapped[int] = mapped_column(init=False, primary_key=True, autoincrement=True)
     character_id: Mapped[int] = mapped_column(ForeignKey("character.id"))
     inventory_type: Mapped[InventoryType] = mapped_column(nullable=False)
@@ -88,6 +87,13 @@ class InventoryMap(BaseObject):
     equipped: Mapped[bool] = mapped_column(default=False)
     count: Mapped[int] = mapped_column(nullable=False, default=1)
 
+    always_prepared: Mapped[bool] = mapped_column(default=False)
+
+    @property
+    def prepared(self):
+        if self.item.item_type == ItemType.SPELL:
+            return self.equipped or self.always_prepared
+
     def use(self, count=1):
         if count < 0:
             return False

src/ttfrog/db/schema/item.py

@@ -18,10 +18,7 @@ class ItemType(EnumField):
 
 class Item(BaseObject, ConditionMixin):
     __tablename__ = "item"
-    __mapper_args__ = {
+    __mapper_args__ = {"polymorphic_identity": ItemType.ITEM, "polymorphic_on": "item_type"}
-        "polymorphic_identity": ItemType.ITEM,
-        "polymorphic_on": "item_type"
-    }
     id: Mapped[int] = mapped_column(init=False, primary_key=True, autoincrement=True)
     name: Mapped[str] = mapped_column(String(collation="NOCASE"), nullable=False, unique=True)
     consumable: Mapped[bool] = mapped_column(default=False)
@@ -31,9 +28,7 @@ class Item(BaseObject, ConditionMixin):
 
 class Spell(Item):
     __tablename__ = "spell"
-    __mapper_args__ = {
+    __mapper_args__ = {"polymorphic_identity": ItemType.SPELL}
-        "polymorphic_identity": ItemType.SPELL
-    }
     id: Mapped[int] = mapped_column(ForeignKey("item.id"), primary_key=True, init=False)
     level: Mapped[int] = mapped_column(nullable=False, info={"min": 0, "max": 9}, default=0)
     concentration: Mapped[bool] = mapped_column(default=False)

src/ttfrog/db/schema/modifiers.py

@@ -283,7 +283,6 @@ class ModifierMixin:
 
 
 class ConditionMixin:
-
     @property
     def modifiers(self):
         """

test/conftest.py

@@ -91,28 +91,50 @@ def bootstrap(db):
         assert acrobatics in fighter.skills
         assert athletics in fighter.skills
 
+        wizard = schema.CharacterClass(
+            "wizard",
+            hit_die_name="1d6",
+            hit_die_stat_name="_intelligence",
+        )
+        db.add_or_update(wizard)
+        wizard.spell_slots = [
+            schema.ClassSpellSlotMap(wizard.id, class_level=1, spell_level=1),
+            schema.ClassSpellSlotMap(wizard.id, class_level=1, spell_level=1),
+            schema.ClassSpellSlotMap(wizard.id, class_level=2, spell_level=1),
+            schema.ClassSpellSlotMap(wizard.id, class_level=3, spell_level=1),
+            schema.ClassSpellSlotMap(wizard.id, class_level=3, spell_level=2),
+            schema.ClassSpellSlotMap(wizard.id, class_level=3, spell_level=2),
+            schema.ClassSpellSlotMap(wizard.id, class_level=4, spell_level=2),
+            schema.ClassSpellSlotMap(wizard.id, class_level=5, spell_level=3),
+            schema.ClassSpellSlotMap(wizard.id, class_level=5, spell_level=3),
+            schema.ClassSpellSlotMap(wizard.id, class_level=6, spell_level=3),
+            schema.ClassSpellSlotMap(wizard.id, class_level=7, spell_level=4),
+        ]
+
         rogue = schema.CharacterClass("rogue", hit_die_name="1d8", hit_die_stat_name="_dexterity")
-        db.add_or_update([rogue, fighter])
+        db.add_or_update([rogue, fighter, wizard])
 
-        # characters
+        # create a character: Carl the Wizard
-        foo = schema.Character("Foo", ancestry=tiefling, _intelligence=14)
+        carl = schema.Character("Carl", ancestry=tiefling, _intelligence=14)
-        db.add_or_update(foo)
+        carl.add_class(wizard)
-        foo.add_class(fighter, level=2)
+        db.add_or_update(carl)
-        foo.add_class(rogue, level=3)
 
-        bar = schema.Character("Bar", ancestry=human)
-
-        # persist all the records we've created
-        db.add_or_update([foo, bar])
 
 @pytest.fixture
-def carl(db, bootstrap, tiefling):
+def carl(db, bootstrap):
-    return schema.Character(name="Carl", ancestry=tiefling)
+    return db.Character.filter_by(name="Carl").one()
+
+
+@pytest.fixture
+def wizard(db, bootstrap):
+    return db.CharacterClass.filter_by(name="wizard").one()
+
 
 @pytest.fixture
 def tiefling(db, bootstrap):
     return db.Ancestry.filter_by(name="tiefling").one()
 
+
 @pytest.fixture
 def human(db, bootstrap):
     return db.Ancestry.filter_by(name="human").one()

test/test_inventories.py

@@ -1,4 +1,4 @@
-from ttfrog.db.schema.item import Item, Spell, ItemType
+from ttfrog.db.schema.item import Item, ItemType, Spell
 
 
 def test_equipment_inventory(db, carl):
@@ -83,3 +83,65 @@ def test_inventory_bundles(db, carl):
         # consume all remaining arrows
         assert quiver.use(19) == 0
         assert arrows not in carl.equipment
+
+
+def test_spell_slots(db, carl, wizard):
+    with db.transaction():
+        prestidigitation = Spell(name="Prestidigitation", level=0, concentration=False)
+        fireball = Spell(name="Fireball", level=3, concentration=False)
+        db.add_or_update([carl, prestidigitation, fireball])
+
+        carl.spells.add(prestidigitation)
+        carl.spells.add(fireball)
+        db.add_or_update(carl)
+
+        # verify carl has the spell slots granted by wizard at 1st level
+        print(carl.levels)
+        print(carl.spell_slots)
+        assert len(carl.spell_slots) == 2
+        assert carl.spell_slots[0].spell_level == 1
+        assert carl.spell_slots[1].spell_level == 1
+
+        # carl knows the spells but hasn't prepared them
+        assert prestidigitation in carl.spells
+        assert fireball in carl.spells
+        assert prestidigitation not in carl.prepared_spells
+        assert fireball not in carl.prepared_spells
+
+        # prepare the cantrip
+        carls_prestidigitation = carl.spells.get(prestidigitation)[0]
+        assert carl.prepare(carls_prestidigitation)
+        assert carl.cast(carls_prestidigitation)
+
+        # prepare() and cast() require a spell from the spell inventory
+        carls_fireball = carl.spells.get(fireball)[0]
+
+        # can't prepare a 3rd level spell if you don't have 3rd level slots
+        assert carl.spellcaster_level == 1
+        assert not carl.prepare(carls_fireball)
+
+        # make carl a 5th level wizard so he gets a 3rd level spell slot
+        carl.level_up(wizard, num_levels=4)
+        assert carl.level == 5
+        assert carl.spellcaster_level == 3
+
+        # cast fireball until he's out of 3rd level slots
+        assert carl.prepare(carls_fireball)
+        assert carl.cast(carls_fireball)
+        assert carl.cast(carls_fireball)
+        assert not carl.cast(carls_fireball)
+
+        # level up to 7th level, gaining 1 4th level slot and 1 more 3rd level slot
+        carl.add_class(wizard)
+        carl.level_up(wizard, num_levels=2)
+        assert carl.spellcaster_level == 4
+        assert len(carl.spell_slots_available[4]) == 1
+        assert len(carl.spell_slots_available[3]) == 1
+
+        # cast at 4th level
+        assert carl.cast(carls_fireball, 4)
+        assert not carl.cast(carls_fireball, 4)
+
+        # use the last 3rd level slot
+        assert carl.cast(carls_fireball)
+        assert not carl.cast(carls_fireball)

test/test_schema.py

@@ -12,7 +12,7 @@ def test_manage_character(db, bootstrap):
         # create a human character (the default)
         char = schema.Character(name="Test Character", ancestry=human)
         db.add_or_update(char)
-        assert char.id == 3
+        assert char.id
         assert char.name == "Test Character"
         assert char.ancestry.name == "human"
         assert char.armor_class == 10
@@ -67,7 +67,7 @@ def test_manage_character(db, bootstrap):
         rogue = db.CharacterClass.filter_by(name="rogue").one()
 
         # assign a class and level
-        char.add_class(fighter, level=1)
+        assert char.add_class(fighter)
         db.add_or_update(char)
         assert char.levels == {"fighter": 1}
         assert char.level == 1
@@ -81,7 +81,7 @@ def test_manage_character(db, bootstrap):
         assert char.class_features == {}
 
         # level up
-        char.add_class(fighter, level=7)
+        char.level_up(fighter, num_levels=6)
         db.add_or_update(char)
         assert char.levels == {"fighter": 7}
         assert char.level == 7
@@ -109,7 +109,7 @@ def test_manage_character(db, bootstrap):
         char._dexterity = 10
 
         # multiclass
-        char.add_class(rogue, level=1)
+        char.add_class(rogue)
         db.add_or_update(char)
         assert char.level == 8
         assert char.levels == {"fighter": 7, "rogue": 1}
@@ -132,8 +132,7 @@ def test_manage_character(db, bootstrap):
         assert char.hit_dice["fighter"][0].name == fighter.hit_die_name == "1d10"
         assert char.hit_dice["fighter"][0].stat == fighter.hit_die_stat_name == "_constitution"
 
-        # remove remaining class by setting level to zero
+        char.remove_class(fighter)
-        char.add_class(fighter, level=0)
         db.add_or_update(char)
         assert char.levels == {}
         assert char.class_features == {}
@@ -205,9 +204,8 @@ def test_ancestries(db, bootstrap):
         assert grognak.check_modifier(strength, save=True) == 1
 
 
-def test_modifiers(db, bootstrap, carl, tiefling, human):
+def test_modifiers(db, bootstrap, carl, tiefling, human, wizard):
     with db.transaction():
-
         # no modifiers; speed is ancestry speed
         marx = schema.Character(name="Marx", ancestry=human)
         db.add_or_update([carl, marx])
@@ -301,32 +299,33 @@ def test_modifiers(db, bootstrap, carl, tiefling, human):
         assert carl.add_modifier(temp_proficiency)
         assert carl.check_modifier(athletics) == carl.expertise_bonus + carl.strength.bonus == 2
         assert carl.remove_modifier(temp_proficiency)
+        assert carl.check_modifier(athletics) == 0
 
         # fighters get proficiency in athletics by default
         fighter = db.CharacterClass.filter_by(name="fighter").one()
         carl.add_class(fighter)
         db.add_or_update(carl)
-        assert carl.check_modifier(athletics) == 1
+        assert carl.check_modifier(athletics) == carl.proficiency_bonus
 
         # add the skill directly, which will grant proficiency but will not stack with proficiency from the class
         carl.add_skill(athletics, proficient=True)
         db.add_or_update(carl)
         assert len([s for s in carl.skills if s == athletics]) == 2
-        assert carl.check_modifier(athletics) == 1
+        assert carl.check_modifier(athletics) == 2
 
         # manually override proficiency with expertise
         carl.add_skill(athletics, expert=True)
-        assert carl.check_modifier(athletics) == 2
+        assert carl.check_modifier(athletics) == 2 * carl.proficiency_bonus
         assert len([s for s in carl.skills if s == athletics]) == 2
 
         # remove expertise
         carl.add_skill(athletics, proficient=True, expert=False)
-        assert carl.check_modifier(athletics) == 1
+        assert carl.check_modifier(athletics) == carl.proficiency_bonus
 
         # remove the extra skill entirely, but the fighter proficiency remains
         carl.remove_skill(athletics, proficient=True, expert=False, character_class=None)
         assert len([s for s in carl.skills if s == athletics]) == 1
-        assert carl.check_modifier(athletics) == 1
+        assert carl.check_modifier(athletics) == carl.proficiency_bonus
 
         # ensure you can't remove a skill already removed
         assert not carl.remove_skill(athletics, proficient=True, expert=False, character_class=None)