Create base damage class

[PaulQbFeng]

CHANGES

• Create BaseDamageInstance class that will be used by BaseSpell, ActiveItem, BaseRune
• introduce standard damage/do_damage methods
• {WIP} Attempt to create an ActiveItem(BaseItem, BaseDamageInstance)
  • BaseItem and BaseDamageInstance are 2 standalone classes, should we still do mulitiple inheritance ? https://stackoverflow.com/a/50465583
  • Should we use of one the 2 classes as MixIns ?
  • Maybe we could use composition instead of inheritance with a self.base_dmg = BaseDamageInstance() or something inside ActiveItem(BaseItem)
• items are now in item_factory.py

Comments:

• new class, file, methods names can be changed

[LinSun00]

Interesting changes. I don't know personally which one is best, but one thing to take into account in our final choice is that we are gonna need to be able to transfer certain properties to damage instances based on their type (Black Cleaver for instance). Maybe this favors composition a bit more ? idk

[PaulQbFeng]

Interesting changes. I don't know personally which one is best, but one thing to take into account in our final choice is that we are gonna need to be able to transfer certain properties to damage instances based on their type (Black Cleaver for instance). Maybe this favors composition a bit more ? idk

Yeah let's start with that

[michaelbenayoun]

I do not know enough about the subject, but:

• BaseDamageInstance can be a mixin if it only implements methods IMO, otherwise let's opt for the other solution.
• I do not know what you mean by "composition", you mean that ActiveItem could have an attribute of instance BaseDamageInstance?

[PaulQbFeng]

I do not know enough about the subject, but:

• BaseDamageInstance can be a mixin if it only implements methods IMO, otherwise let's opt for the other solution.
• I do not know what you mean by "composition", you mean that ActiveItem could have an attribute of instance BaseDamageInstance?

Yes exactly. Something like this:

• We create an item = ActiveItem(BaseItem) subclass.
• We create an active_effect = ItemActiveEffect(BaseDamageInstance)
• item.active = active_effect

then we can do item.active.damage()

[LinSun00]

Btw I remember that there was a discussion about putting the on hits in do_auto_attack / do_damage. Is this still on the table or not ?

[PaulQbFeng]

Btw I remember that there was a discussion about putting the on hits in do_auto_attack / do_damage. Is this still on the table or not ?

It means that we can't test on-hit nor apply_buff in the damage method, is it ok like that ?

[LinSun00]

It means that we can't test on-hit nor apply_buff in the damage method, is it ok like that ?

Mh, I don't know, I think it was @Lysxia 's idea. I'm also fine with keeping everything in damage method tbh

[PaulQbFeng]

UPDATE

• In the end I opted for the Mixin approach. The damage methods are all in attack.BaseDamageMixin
  • Let's discuss the current Implem @Lysxia @michaelbenayoun @LinSun00 for any questions or improvements
• For the moment I left the apply_buff mechanism inside spell, but it might be moved to BaseDamageMixin in the future

[LinSun00]

I understood everything super fast which means that at the very least, it's clear and intuitive. Gj

[PaulQbFeng]

I merge to develop but feel free to comment @Lysxia