refactor modifiers

src/ttfrog/db/manager.py

@@ -59,8 +59,11 @@ class SQLDatabaseManager:
                 tm.abort()
                 raise
 
-    def add_or_update(self, *args, **kwargs):
+    def add_or_update(self, record, *args, **kwargs):
-        self.session.add(*args, **kwargs)
+        if not isinstance(record, list):
+            record = [record]
+        for rec in record:
+            self.session.add(rec, *args, **kwargs)
         self.session.flush()
 
     def query(self, *args, **kwargs):

src/ttfrog/db/schema/__init__.py

@@ -1,4 +1,5 @@
 from .character import *
 from .classes import *
 from .log import *
+from .modifiers import *
 from .property import *

src/ttfrog/db/schema/character.py

@@ -1,16 +1,14 @@
-from collections import defaultdict
+from sqlalchemy import Column, Enum, ForeignKey, Integer, String, Text, UniqueConstraint
-
-from sqlalchemy import Column, Enum, ForeignKey, Integer, Float, String, Text, UniqueConstraint
 from sqlalchemy.ext.associationproxy import association_proxy
-from sqlalchemy.orm import relationship, validates
+from sqlalchemy.orm import relationship
 
 from ttfrog.db.base import BaseObject, CreatureTypesEnum, SavingThrowsMixin, SizesEnum, SkillsMixin, SlugMixin
+from ttfrog.db.schema.modifiers import Modifier, ModifierMixin
 
 __all__ = [
     "Ancestry",
     "AncestryTrait",
     "AncestryTraitMap",
-    "AncestryModifier",
     "CharacterClassMap",
     "CharacterClassAttributeMap",
     "Character",
@@ -40,10 +38,9 @@ class AncestryTraitMap(BaseObject):
     level = Column(Integer, nullable=False, info={"min": 1, "max": 20})
 
 
-# XXX: Replace this with a many-to-many on the Modifiers table. Will need for proficiecies too.
+class Ancestry(BaseObject, ModifierMixin):
-class Ancestry(BaseObject):
     """
-    A character ancestry ("race"), which has zero or more AncestryTraits.
+    A character ancestry ("race"), which has zero or more AncestryTraits and Modifiers.
     """
 
     __tablename__ = "ancestry"
@@ -52,8 +49,7 @@ class Ancestry(BaseObject):
     creature_type = Column(Enum(CreatureTypesEnum), nullable=False, default="humanoid")
     size = Column(Enum(SizesEnum), nullable=False, default="Medium")
     speed = Column(Integer, nullable=False, default=30, info={"min": 0, "max": 99})
-    _traits = relationship("AncestryTraitMap", lazy="immediate")
+    _traits = relationship("AncestryTraitMap", cascade="all,delete,delete-orphan", lazy="immediate")
-    modifiers = relationship("AncestryModifier", lazy="immediate")
 
     @property
     def traits(self):
@@ -65,33 +61,10 @@ class Ancestry(BaseObject):
             return True
         return False
 
-    def add_modifier(self, modifier):
-        if modifier not in self.modifiers:
-            self.modifiers.append(modifier)
-            return True
-        return False
-
     def __repr__(self):
         return self.name
 
 
-class AncestryModifier(BaseObject):
-    """
-    A modifier granted to a character via its Ancestry.
-    """
-
-    __tablename__ = "ancestry_modifier"
-    id = Column(Integer, primary_key=True, autoincrement=True)
-    ancestry_id = Column(Integer, ForeignKey("ancestry.id"), nullable=False)
-    name = Column(String, nullable=False)
-    target = Column(String, nullable=False)
-    absolute_value = Column(Integer)
-    relative_value = Column(Integer)
-    multiply_value = Column(Float)
-    new_value = Column(String)
-    description = Column(String, nullable=False)
-
-
 class AncestryTrait(BaseObject):
     """
     A trait granted to a character via its Ancestry.
@@ -142,20 +115,7 @@ class CharacterClassAttributeMap(BaseObject):
     )
 
 
-class Modifier(BaseObject):
+class Character(BaseObject, ModifierMixin, SlugMixin, SavingThrowsMixin, SkillsMixin):
-    __tablename__ = "modifier"
-
-    id = Column(Integer, primary_key=True, autoincrement=True)
-    character_id = Column(Integer, ForeignKey("character.id"), nullable=False)
-    target = Column(String, nullable=False)
-    absolute_value = Column(Integer)
-    relative_value = Column(Integer)
-    multiply_value = Column(Float)
-    new_value = Column(String)
-    description = Column(String, nullable=False)
-
-
-class Character(BaseObject, SlugMixin, SavingThrowsMixin, SkillsMixin):
     __tablename__ = "character"
     id = Column(Integer, primary_key=True, autoincrement=True)
     name = Column(String, default="New Character", nullable=False)
@@ -174,7 +134,6 @@ class Character(BaseObject, SlugMixin, SavingThrowsMixin, SkillsMixin):
     class_map = relationship("CharacterClassMap", cascade="all,delete,delete-orphan")
     class_list = association_proxy("class_map", "id", creator=class_map_creator)
 
-    _modifiers = relationship("Modifier", cascade="all,delete,delete-orphan", lazy="immediate")
     _modify_ok = [
         "armor_class",
         "max_hit_points",
@@ -196,12 +155,10 @@ class Character(BaseObject, SlugMixin, SavingThrowsMixin, SkillsMixin):
 
     @property
     def modifiers(self):
-        all_modifiers = defaultdict(list)
+        unified = {}
-        for mod in self.ancestry.modifiers:
+        unified.update(**self.ancestry.modifiers)
-            all_modifiers[mod.target].append(mod)
+        unified.update(**super().modifiers)
-        for mod in self._modifiers:
+        return unified
-            all_modifiers[mod.target].append(mod)
-        return all_modifiers
 
     @property
     def classes(self):
@@ -302,7 +259,6 @@ class Character(BaseObject, SlugMixin, SavingThrowsMixin, SkillsMixin):
                 return True
         return False
 
-
     def apply_modifiers(self, target, initial):
         modifiers = list(reversed(self.modifiers.get(target, [])))
         if isinstance(initial, int):
@@ -318,13 +274,3 @@ class Character(BaseObject, SlugMixin, SavingThrowsMixin, SkillsMixin):
         if new:
             return new[0].new_value
         return initial
-
-
-
-    def add_modifier(self, modifier):
-        if modifier.absolute_value is not None and modifier.relative_value is not None and modifier.multiple_value:
-            raise AttributeError(f"You must provide only one of absolute, relative, and multiple values {modifier}.")
-        self._modifiers.append(modifier)
-
-    def remove_modifier(self, modifier):
-        self._modifiers = [mod for mod in self._modifiers if mod != modifier]

src/ttfrog/db/schema/modifiers.py

@@ -0,0 +1,102 @@
+from collections import defaultdict
+
+from sqlalchemy import Column, Float, ForeignKey, Integer, String, UniqueConstraint
+from sqlalchemy.ext.declarative import declared_attr
+from sqlalchemy.orm import relationship
+
+from ttfrog.db.base import BaseObject
+
+
+class ModifierMap(BaseObject):
+    """
+    Creates a many-to-many between Modifier and any model inheriting from the ModifierMixin.
+    """
+
+    __tablename__ = "modifier_map"
+    __table_args__ = (UniqueConstraint("primary_table_name", "primary_table_id", "modifier_id"),)
+    id = Column(Integer, primary_key=True, autoincrement=True)
+    primary_table_name = Column(String, nullable=False)
+    primary_table_id = Column(Integer, nullable=False)
+    modifier_id = Column(Integer, ForeignKey("modifier.id"), nullable=False)
+    modifier = relationship("Modifier", uselist=False, lazy="immediate")
+
+
+class Modifier(BaseObject):
+    """
+    Modifiers modify the base value of an existing attribute on another table.
+
+    Modifiers are applied by the Character class, but may be associated with any model via the
+    ModifierMixIn model; refer to the Ancestry class for an example.
+    """
+
+    __tablename__ = "modifier"
+    id = Column(Integer, primary_key=True, autoincrement=True)
+    target = Column(String, nullable=False)
+    absolute_value = Column(Integer)
+    relative_value = Column(Integer)
+    multiply_value = Column(Float)
+    new_value = Column(String)
+    name = Column(String, nullable=False)
+    description = Column(String)
+
+
+class ModifierMixin:
+    """
+    Add modifiers to an existing class.
+
+    Attributes:
+         modifier_map   - get/set a list of Modifier records associated with the parent
+         modifiers      - read-only dict of lists of modifiers keyed on Modifier.target
+
+    Methods:
+        add_modifier     - Add a Modifier association to the modifier_map
+        remove_modifier  - Remove a modifier association from the modifier_map
+
+    Example:
+
+        >>> class Item(BaseObject, ModifierMixin):
+                id = Column(Integer, primary_key=True, autoincrement=True)
+                name = Column(String, nullable=False)
+        >>> dwarven_belt = Item(name="Dwarven Belt")
+        >>> dwarven_belt.add_modifier(Modifier(name="STR+1", target="strength", relative_value=1))
+        >>> dwarven_belt.modifiers
+        {'strength': [Modifier(id=1, target='strength', name='STR+1', relative_value=1 ... ]}
+    """
+
+    @declared_attr
+    def modifier_map(cls):
+        return relationship(
+            "ModifierMap",
+            primaryjoin=f"ModifierMap.primary_table_id == foreign({cls.__name__}.id)",
+            cascade="all,delete,delete-orphan",
+            single_parent=True,
+            uselist=True,
+            lazy="immediate",
+        )
+
+    @property
+    def modifiers(self):
+        all_modifiers = defaultdict(list)
+        for mapping in self.modifier_map:
+            all_modifiers[mapping.modifier.target].append(mapping.modifier)
+        return all_modifiers
+
+    def add_modifier(self, modifier):
+        if modifier.absolute_value is not None and modifier.relative_value is not None and modifier.multiple_value:
+            raise AttributeError(f"You must provide only one of absolute, relative, and multiple values {modifier}.")
+        if [mod for mod in self.modifier_map if mod.modifier == modifier]:
+            return False
+        self.modifier_map.append(
+            ModifierMap(
+                primary_table_name=self.__tablename__,
+                primary_table_id=self.id,
+                modifier=modifier,
+            )
+        )
+        return True
+
+    def remove_modifier(self, modifier):
+        if modifier.id not in [mod.modifier_id for mod in self.modifier_map]:
+            return False
+        self.modifier_map = [mapping for mapping in self.modifier_map if mapping.modifier != modifier]
+        return True

test/test_load.py

@@ -0,0 +1,16 @@
+import pytest
+
+from ttfrog.db import schema
+
+
+@pytest.mark.skip
+def test_many_records(db):
+    with db.transaction():
+        for i in range(1, 1000):
+            obj = schema.Ancestry(name=f"{i}-ancestry")
+            db.add_or_update(obj)
+            assert obj.id == i
+
+        for i in range(1, 1000):
+            obj = schema.Character(name=f"{i}-char")
+            db.add_or_update(obj)

test/test_schema.py

@@ -113,16 +113,15 @@ def test_ancestries(db):
         assert endurance in porc.traits
 
         # add a +1 STR modifier
-        str_plus_one = schema.AncestryModifier(
+        str_plus_one = schema.Modifier(
             name="STR+1 (Pygmy Orc)",
             target="strength",
             relative_value=1,
-            description="Your Strength score is increased by 1."
+            description="Your Strength score is increased by 1.",
         )
         assert porc.add_modifier(str_plus_one) is True
         assert porc.add_modifier(str_plus_one) is False  # test idempotency
-        db.add_or_update(porc)
+        assert str_plus_one in porc.modifiers["strength"]
-        assert str_plus_one in porc.modifiers
 
         # now create an orc character and assert it gets traits and modifiers
         grognak = schema.Character(name="Grognak the Mighty", ancestry=porc)
@@ -131,7 +130,7 @@ def test_ancestries(db):
 
         # verify the strength bonus is applied
         assert grognak.strength == 10
-        assert str_plus_one in grognak.modifiers['strength']
+        assert str_plus_one in grognak.modifiers["strength"]
         assert grognak.STR == 11
 
 
@@ -145,37 +144,38 @@ def test_modifiers(db, classes_factory, ancestries_factory):
         db.add_or_update(carl)
         assert carl.speed == carl.ancestry.speed == 30
 
+        cold = schema.Modifier(target="speed", relative_value=-10, name="Cold")
+        hasted = schema.Modifier(target="speed", multiply_value=2.0, name="Hasted")
+        slowed = schema.Modifier(target="speed", multiply_value=0.5, name="Slowed")
+        restrained = schema.Modifier(target="speed", absolute_value=0, name="Restrained")
+        reduced = schema.Modifier(target="size", new_value="Tiny", name="Reduced")
+
         # reduce speed by 10
-        cold = schema.Modifier(target="speed", relative_value=-10, description="Cold")
+        assert carl.add_modifier(cold)
-        carl.add_modifier(cold)
         assert carl.speed == 20
 
         # speed is doubled
-        carl.remove_modifier(cold)
+        assert carl.remove_modifier(cold)
-        hasted = schema.Modifier(target="speed", multiply_value=2.0, description="Hasted")
+        assert carl.add_modifier(hasted)
-        carl.add_modifier(hasted)
         assert carl.speed == 60
 
         # speed is halved
-        slowed = schema.Modifier(target="speed", multiply_value=0.5, description="Slowed")
+        assert carl.remove_modifier(hasted)
-        carl.remove_modifier(hasted)
+        assert carl.add_modifier(slowed)
-        carl.add_modifier(slowed)
         assert carl.speed == 15
 
         # speed is 0
-        restrained = schema.Modifier(target="speed", absolute_value=0, description="Restrained")
+        assert carl.add_modifier(restrained)
-        carl.add_modifier(restrained)
         assert carl.speed == 0
 
         # no longer restrained, but still slowed
-        carl.remove_modifier(restrained)
+        assert carl.remove_modifier(restrained)
         assert carl.speed == 15
 
         # back to normal
-        carl.remove_modifier(slowed)
+        assert carl.remove_modifier(slowed)
         assert carl.speed == carl.ancestry.speed
 
         # modifiers can modify string values too
-        carl.add_modifier(schema.Modifier(target="size", new_value="Tiny", description="Reduced"))
+        assert carl.add_modifier(reduced)
         assert carl.size == "Tiny"
-