the-marenga / sf-api

Manage Shakes & Fidget characters via simple commands. Handles encryption, response parsing and everything else for you
MIT License
15 stars 3 forks source link

unintutitive names of variable in FortressRessources #58

Closed tiquis0290 closed 3 months ago

tiquis0290 commented 5 months ago

When i tried accessing resources in fortress i felt very confused especially because of property limit (currently in storage), why call it limit ??? and the i looked at max_limit_next and thought, oh its probably current storage limit in fortress, but it isn't so maybe in next release change would be welcoming

i proposed something but i don't think that it will be ideal, but i was not able to thought of anything better

current pub struct FortessRessource { pub limit: u64, //currently in storage pub current: u64, //currently in resource gathering building pub max_in_building: u64, pub max_save: u64, pub per_hour: u64, /// The limit after the next upgrade pub max_limit_next: u64, } proposed

pub struct FortessRessource { pub storage: u64, //currently in storage pub gathering: u64, //currently in resource gathering building pub max_gathering: u64, pub max_in_storage: u64, pub per_hour: u64, /// The limit after the next upgrade pub max_limit_next: u64, }

the-marenga commented 5 months ago

Jup, the naming there does not make much sense. Correct me if I am wrong, but I think the way underworld resources (mainly souls) are handled is more understandable and correctly mapped:

pub struct UnderWorldResource {
    pub current: u32, // The amount you currently have to spend
    pub limit: u32, // The limit of how much you can currently have
    pub in_building: u32, // The amount in the production building
    pub max_in_building: u32, // The max in the production building
    pub max_limit_next: u32, // The max in the production building after the next upgrade
    pub per_hour: u32, // The production per hour
}

I thought this was also how fortress resources were handles, but I guess I was wrong. Either S&F changed the ordering of these, or I messed something up during parsing (these values come from different places and changed during development, so this is likely what happened).

But yeah, seems like I have to correct that, thanks for the report!