reko-beep / hsr-data

Honkai star rail data from starrailstation.com
MIT License
60 stars 13 forks source link

light cones model #7

Closed BestClaws closed 1 year ago

BestClaws commented 1 year ago

This is a draft pull requests. idea is to discuss , make additional changes before merging.

I'm kinda iffy about type definition for this property in light cone model (see hsr_client/datamodels/lightcone.py in PR) the type feels too long for me, how do you feel about this.

 ascension_mats: Dict[Level, List[Dict[Material, Count]]]

alternative is to create more sub types like

ascension_mats: Dict[Level, AscensionMaterials]

where, AscensionMaterials could be an iterator so we can do

for ascension_mat in ascenscion_mats:
    ...

and ascension_mat could be of type AscensionMaterial

so we can do

ascension_mat.material # the `Material` required
ascension_mat.count # the number of such materials requred.

but this introduces 2 more types , unncessarily.

reko-beep commented 1 year ago

AscensionMaterial would be end-user friendly in my opinion, and easy to access. The former is fine too, but would require more line of codes, for consumer | end-user to get the required data.

BestClaws commented 1 year ago

will go with AscensionMaterail then. ill save myself with AscensionMaterials thought since its redundant with list. Also this decision should reflect on other data models as well