dragonfruitnetwork / dragon6-api

Rainbow Six Siege Stats API for .NET
Apache License 2.0
12 stars 6 forks source link

Split PvP and PvE stats into 2 separate requests #135

Closed aspriddell closed 3 years ago

aspriddell commented 3 years ago

While it is a breaking change, it's a good idea because in most cases we don't need all the stats.

byBlurr commented 3 years ago

Was this not done yet? Is it StatsRequest.cs that you want to be split into separate requests?

How would you like it to be separated? Keep 'StatsRequest' as casual and add 'RankedStatsRequest' for the ranked stats?

aspriddell commented 3 years ago

well, basically the only thing that actually changes is the keys, for example https://github.com/dragonfruitnetwork/dragon6-api/blob/f0050b108dd70b4f6f29beb5479882ad9344fe83/DragonFruit.Six.API/Data/Strings/Weapon.cs#L8 pvp gets changed to pve and the same would need to be applied to the deserializer. as far as i'm concerned, it's not really worth doing unless somebody really wants terrorist hunt stats

(not sure if you knew, but the new stats site api is available here. it uses parts of this library but the response format is a bit more predictable. there's also only one request type too)

byBlurr commented 3 years ago

Just now realised I misread the title, that's my bad...

So as of right now its only PvP stats (which makes sense) so this would be just making the PvE stats available? Taking weapon stats as an example... Add the PvE strings to Data/Strings/Weapons.cs and then create WeaponStatsDeserializer, WeaponStats and WeaponStatsRequest for PvE. Keeping them entirely separate (Except the strings). Then finally bringing it together with a GetPveWeaponStats() in WeaponStatsExtensions.cs

Am I getting this right? If not let me know what I am missing or if I have got it all wrong 😂

aspriddell commented 3 years ago

in dragon6 (app) it uses this: image

so there are already some pve strings there, but as i've discovered a lot of them are looking like they work both ways. Not too sure if there's a best-case here, as we can get away with a single request right now, but it could end up splitting into two requests with 2 sets of very similar responses

the ideal situation would be so the string base is only mentioned once. but then again it's terrorist hunt, so i'm not sure if we want to invest too much into this...

byBlurr commented 3 years ago

I believe the way I said before prevents it from being breaking and would be less work, as like you said its terrorist hunt. I feel no bad can come from it, as it makes more data available. Then at a later date, it could be reworked to be more like above

aspriddell commented 3 years ago

I believe the way I said before prevents it from being breaking and would be less work, as like you said its terrorist hunt. I feel no bad can come from it, as it makes more data available. Then at a later date, it could be reworked to be more like above

as the stats and formats for this are now set in stone, i don't see why not

byBlurr commented 3 years ago

Which stats are relevant here? Besides weapons? General stats already have training right?

(Edited because I think Operator is different to Weapons, let me check)

aspriddell commented 3 years ago

think so

byBlurr commented 3 years ago

Ok, once I have done the namespaces refactor, I will assign myself to this issue. Looks like its the Weapons and Operator stats

byBlurr commented 3 years ago

How would you prefer them to be named? Personally I think PickedTraining @aspriddell

image

aspriddell commented 3 years ago

top one makes more sense imo