search

hohav / peppi

Rust parser for Slippi SSBM replay files
MIT License
37 stars 9 forks source link

Add Costume type to model #9

Closed NickCondron closed 2 years ago

NickCondron commented 2 years ago

Color names based on ssbwiki.com, but I made sure Red, Green, and Blue existed for every character. ~I plan to add colloquial names if #8 gets merged.~

I removed derive implementations of Default/Deserialize for the Player type because my change caused conflicts. It doesn't seem like those traits made much sense in the first place.

~It might make sense to move the default, red, green, blue functions to the External type, but I left it how it is for now.~

hohav commented 2 years ago

Looks great. Thanks!

How do you anticipate red() & co being used? I assume for teams, but to do what exactly?

NickCondron commented 2 years ago

I honestly didn't have a solid use case in mind. I was having fun with macros and it seemed like a simple addition. I understand wanting to keep the api streamlined, so I'm not sentimental about those functions.

Maybe a niche use case would be searching for games with red characters but you don't care which character in particular.

hohav commented 2 years ago

I honestly didn't have a solid use case in mind. I was having fun with macros and it seemed like a simple addition. I understand wanting to keep the api streamlined, so I'm not sentimental about those functions.

I'd rather leave them out for now, in that case. We can always add them if there's demand, but removing them would break backwards compatibility.

NickCondron commented 2 years ago

Ok I removed them for now. I plan on using this costume feature. If I find that the red/green/blue functions would be useful for me I will bug you to add them back in at that point.

NickCondron commented 2 years ago

Rebased and squashed

NickCondron commented 2 years ago

One last thing: could you replace spaces with tabs? Sorry I haven't gotten around to configuring rustfmt yet.

I'm not seeing any space indents on this branch

hohav commented 2 years ago

One last thing: could you replace spaces with tabs? Sorry I haven't gotten around to configuring rustfmt yet.

I'm not seeing any space indents on this branch

Oh my bad, Firefox was misleading me. Sorry for the noise.