PaulQbFeng / too-tanky

3 stars 0 forks source link

Inventory #39

Closed LinSun00 closed 1 year ago

LinSun00 commented 1 year ago

The Inventory class is created to have more clarity in the code and because it makes it a lot easier to handle mythic/legendary items interactions.

Inventory now handles how we get stats from all items, how items are added and removed from the inventory and which rules they cannot cross when doing so (a champions can't carry more than one mythic item).

The implementation of mythic passives still isn't done. I only prepared the ground by naming a method mythic_passive_stats

PaulQbFeng commented 1 year ago

Ayaya controversial. J'avais fait une classe inventory puis je suis repasse par une liste sur les conseils de Sam @Lysxia

PaulQbFeng commented 1 year ago

Tu l'as deja implem though 🫣

PaulQbFeng commented 1 year ago

Jregarderai though

LinSun00 commented 1 year ago

I think it's quite useful, but if there's another way, I'm up for it! (and ofc all of this could be done in BaseChampion, but it would be a bit messy imo)

Lysxia commented 1 year ago

It's a minor thing but I find it weird that this means that there are stats for Inventory that you then have to apply to the champion in a separate step. An alternative to consider is to use a mixin, which still lets you logically group methods but also lets you have a single object acting as both champion and inventory, so that you don't need boilerplate to forward operations between them (like Champion.equip_item and Inventory.add_item would be just one method).

PaulQbFeng commented 1 year ago

It's a minor thing but I find it weird that this means that there are stats for Inventory that you then have to apply to the champion in a separate step. An alternative to consider is to use a mixin, which still lets you logically group methods but also lets you have a single object acting as both champion and inventory, so that you don't need boilerplate to forward operations between them (like Champion.equip_item and Inventory.add_item would be just one method).

I will create an issue for that, I am not too familiar with mixins I would like to try that in a branch some day

PaulQbFeng commented 1 year ago

For the moment let's keep the inventory then, we will refactor it later if needed

Lysxia commented 1 year ago

OK