• https://docs.rs/roboat/0.8.1/roboat/struct.Client.html doesn't tell you obviously how to get one, because new isn't the first item. I recommend that you rearrange the code so that new lands at the top, and since there are multiple constructors, also describe in the type docs an overview of which methods to look at.
• In general, every type should have an explanation of the typical means by which the user will construct or fetch it, if the answer isn't “this is a struct literal with public methods”.
• https://docs.rs/roboat/0.8.1/roboat/presence/index.html is an empty module; possibly a mistake?
• https://docs.rs/roboat/0.8.1/roboat/enum.Limit.html This is a weird type — why does it exist, why not an integer? If it is part of the API requirements then say so. Maybe make it #[non_exhaustive].
• https://docs.rs/roboat/0.8.1/roboat/enum.RoboatError.html This is a bit unusual, but I recommend making your error type implement Clone if you can. It may be useful to users.
#Code Review of Roboat
spark blaze
• A couple of client methods say “The default limit is Limit::Ten.” but the Limit isn't actually optional
overall it looks pretty straightforwardly okay
of course there could be Roblox-specific stuff that I don't know about and can't tell you will be a problem
spark blaze
How would I rearrange the code so that new is at the top?
…huh, actually I'm not sure.
random umbra
One thing i've noticed is that you use alot of serde rename macros. Serde has this neat feature called renameAll that you can put above the struct definition that will automatically convert all snake_case to camel case. I believe it's something like #[serde(rename_all = "camelCase")]
this would allow you to remove all the #[serde(rename = "")] from your struct fields
so for example, this code in https://github.com/Chloe-Woahie/roboat/blob/main/src/catalog/avatar_catalog/reqwest_types.rs would become this:
#[derive(Serialize, Deserialize, Debug, Clone)]
#[serde(rename_all = "camelCase")]
pub(super) struct ItemDetailsRaw {
pub id: Option<u64>,
pub item_type: Option<ItemType>,
pub bundle_type: Option<u64>,
pub asset_type: Option<u64>,
pub name: Option<String>,
pub description: Option<String>,
pub product_id: Option<u64>,
pub genres: Option<Vec<Genre>>,
pub item_statuses: Option<Vec<ItemStatus>>,
pub item_restrictions: Option<Vec<ItemRestriction>>,
pub creator_has_verified_badge: Option<bool>,
pub creator_type: Option<CreatorType>,
pub creator_user_id: Option<u64>,
pub creator_name: Option<String>,
/// Exists instead of lowest_price if the item is non-limited.
pub price: Option<u64>,
/// Exists instead of price if the item is limited.
pub lowest_price: Option<u64>,
pub favorite_count: Option<u64>,
pub premium_pricing: Option<PremiumPricing>,
pub price_status: Option<PriceStatus>,
/// It is unknown as to what type this value is.
/// The farthest it can be tracked by reverse engineering is that the value
/// is fed into a `new Date()` constructor in js.
///
/// Because of this, it is not included in the public struct until
pub off_sale_deadline: Option<serde_json::Value>,
}
did you use a tool to make the structs? i know that one of them does actually use rename all
ah got it
yeah quicktype might be outdated
i think this is the one i regularly use
it does it for practically every language
wdym exactly? i think you can use renameAll(snake_case)
normal case is already camel case compliant
so anything that isn't camel case will just be the same
uhh that shouldn't be happening?
what was your input?
did your input json happen to be PascalCase?
there is a settings box top left
you can change it to PascalCase and it will use that as the defualt
honestly i thought it would just detect what case everything was
and use the appropriate one
#[serde(rename_all = "PascalCase")] should be fine
btw, does roblox really have two different values for the same enumerated value? If so that's definitely on them not you
that's some API horror shit IMO
yeah it seems that way.. why the hell would they have Off Sale and Offsale
i think what you have here already does what you want it to do? or at least i don't know if you can do it any cleaner
ohh
From what i can tell from the roblox API that field can only be Free, OffSale, or NoResellers. is there a reason why you have two ways to deserialize it?
i don't see how that requires you to have multiple deserialization option, but maybe i'm missing something
are people writing the json manually to deserialize it? i don't understand ;p