Peternator7
Add #[strum(flatten)] to EnumIter
Hi again, I can't re-open #188, so I created a new issue
I still want this functionality 😅 .
repost from original issue.. My personal use case for EnumIter is using it for tests where I cover every possible enum variant. And with nested enums, it becomes a bit weird, as instead of getting every possible variant, EnumIter uses default values for non-trivial enum variants.
I understand that doing that will be hard without breaking backward compatibility and probably very tricky for the general case, so I'm proposing an attribute like #[strum(flatten)] inspired by serde's flatten.
#[derive(strum::EnumIter)]
enum Uwu {
Much,
Such,
}
#[derive(strum::EnumIter)]
enum React {
Wow,
#[strum(flatten)]
Reaction(Uwu),
}
#[test]
fn test_flatten() {
let results = React::iter().collect::<Vec<_>>();
assert_eq!(results, vec![React::Wow, React::Reaction(Uwu::Much), React::Reaction(Uwu::Such)]);
}
*... end of repost from original issue * Related discussions:
- https://github.com/Peternator7/strum/discussions/265
- https://github.com/Peternator7/strum/discussions/266
I have a working implementation (at least on paper) that would do the trick.
While I did all that, I realized that both my feature and original could be implemented in a simpler way.
I looked at the code for the current EnumIter, and it makes sense, and it works, etc.
But ... it seems like the current implementation could be rewritten just with this, and what I want could be adjusted to inserting a few chain() calls for each #[strum(flatten)].
impl IntoIterator for Vibe {
type Item = Vibe;
type IntoIter = std::vec::IntoIter<Vibe>;
fn into_iter(self) -> Self::IntoIter {
vec![#(#name::#ident #params),*].into_iter()
}
}
Is there a reason why we can't use this? Is it runtime/compilation performance? I'm fine with either way, but this is really much simpler, imo.
UPD1: I guess size_hint and ExactSizeIterator might be an issue for flatten, but we can disable them
UPD2: ok, we can't ...
UPD3: ok, I know how to implement ExactSizeIterator
Not the maintainer, this is a drive-by comment. Feel free to dismiss if this isn't useful:
Meta Feedback on the issue and PR
I've just been looking at strum recently to learn more about the macro ecosystem. I also maintain https://codetriage.com/ and through that work had some feedback if it's helpful.I read through your issues: I'm unclear on what is the exact problem you're solving. Looking at the linked issue I think that's what the maintainer is saying too, though less directly:
unless you still have a need
I see three thumbs-ups on your original issue, but I don't have a good picture of what end goal you (or they) are trying to accomplish.
I see some examples in your tests of your PR where you demonstrate what you want. I took those and re-framed for an example of how you could demonstrate your desired usage https://gist.github.com/schneems/0a2513d94ac6d1134236e8dcbab35886 in a bug report format.
However, that's still a little abstract. Some more details about your real world need might help find alternative ways to accomplish it.
Another thing that can help is scanning through existing open issues and trying to find concrete problems you feel your PR could help solve and linking them. Basically trying to rally some support. It could help if you find any, or if not ... you'll gain a better understanding of the types of problems people are having.
I also recommend reviewing some "closed and merged" PRs as positive examples of what makes a compelling case to the maintainer. Reviewing some "close and not merged" PRs can give you some examples of what to avoid.
I hope this extremely unsolicited advice helps.
I read through your issues: I'm unclear on what is the exact problem you're solving. Looking at the linked issue I think that's what the maintainer is saying too, though less directly
Well, multiple problems.
In general, if EnumIter exists, it should handle enum iterations, and it falls a bit short here.
In our game, we have a Body enum, an enum of all possible entities.
We split them into categories, mostly for animation purposes, as each category has different animation requirements, but also to help with maintainability.
Why would you iterate over Body enum? Well, I guess for the same reasons why would you iterate over any enum. Generate some methods for its creation, generate fuzzy-like tests to cover all possible inputs, etc.
And it's a bit sad that the moment we try to split our enum into different categories, strum gives up.
I'm not trying to gain a free T-shirt here. If it's deemed an undesirable feature, that's fine too, worst case we'll fork strum or switch to some other solution.
My assumption, though, is that if people are using EnumIter, they'd find it super useful being able to use it on a larger set of types.
For now, I created a set of declarative macros, enum_iter! and struct_iter!, which work with nested types and produce the desired results for us, all possible values this type can have.
In addition to that, it has some niceties like giving you all variants for "simple" enums into a const array (for some reason, strum decided to go with const slices, but I assume it's just because const arrays weren't available back then).
Some more concrete examples. Body::iter() implemented with enum_iter! and struct_iter! combo.
Testing that NPCs don't have invalid assets attached to them with Body::iter().
// Testing different species
//
// Things that will be caught - invalid assets paths for
// creating default main hand tool or equipment without config
#[test]
fn test_loadout_species() {
for body in Body::iter() {
std::mem::drop(LoadoutBuilder::from_default(&body))
}
}
Without it.
#[test]
fn test_loadout_species() {
macro_rules! test_species {
// base case
($species:tt : $body:tt) => {
let mut rng = thread_rng();
for s in comp::$species::ALL_SPECIES.iter() {
let body = comp::$species::Body::random_with(&mut rng, s);
let female_body = comp::$species::Body {
body_type: comp::$species::BodyType::Female,
..body
};
let male_body = comp::$species::Body {
body_type: comp::$species::BodyType::Male,
..body
};
std::mem::drop(LoadoutBuilder::from_default(
&Body::$body(female_body),
));
std::mem::drop(LoadoutBuilder::from_default(
&Body::$body(male_body),
));
}
};
// recursive call
($base:tt : $body:tt, $($species:tt : $nextbody:tt),+ $(,)?) => {
test_species!($base: $body);
test_species!($($species: $nextbody),+);
}
}
// See `[AllBodies](crate::comp::body::AllBodies)`
test_species!(
humanoid: Humanoid,
quadruped_small: QuadrupedSmall,
quadruped_medium: QuadrupedMedium,
quadruped_low: QuadrupedLow,
quadruped_small: QuadrupedSmall,
bird_medium: BirdMedium,
bird_large: BirdLarge,
fish_small: FishSmall,
fish_medium: FishMedium,
biped_small: BipedSmall,
biped_large: BipedLarge,
theropod: Theropod,
dragon: Dragon,
golem: Golem,
arthropod: Arthropod,
);
}
Generating entities for autocompletion in commands.
pub static ref ENTITIES: Vec<String> = {
let npc_names = &*npc::NPC_NAMES.read();
// HashSets for deduplication of male, female, etc
let mut categories = HashSet::new();
let mut species = HashSet::new();
for body in comp::Body::iter() {
// plugin doesn't seem to be spawnable, yet
if matches!(body, comp::Body::Plugin(_)) {
continue;
}
if let Some(meta) = npc_names.get_species_meta(&body) {
categories.insert(npc_names[&body].keyword.clone());
species.insert(meta.keyword.clone());
}
}
let mut strings = Vec::new();
strings.extend(categories);
strings.extend(species);
strings
};
Without it.
pub static ref ENTITIES: Vec<String> = {
let npc_names = &*npc::NPC_NAMES.read();
let mut souls = Vec::new();
macro_rules! push_souls {
($species:tt) => {
for s in comp::$species::ALL_SPECIES.iter() {
souls.push(npc_names.$species.species[s].keyword.clone())
}
};
($base:tt, $($species:tt),+ $(,)?) => {
push_souls!($base);
push_souls!($($species),+);
}
}
for npc in npc::ALL_NPCS.iter() {
souls.push(npc_names[*npc].keyword.clone())
}
// See `[AllBodies](crate::comp::body::AllBodies)`
push_souls!(
humanoid,
quadruped_small,
quadruped_medium,
quadruped_low,
bird_medium,
bird_large,
fish_small,
fish_medium,
biped_small,
biped_large,
theropod,
dragon,
golem,
arthropod,
crustacean,
);
souls
};
Testing proper localization.
Without body::iter, we can only test NPC configs and even then not very reliably.
#[test]
fn test_npc_names() {
let mut no_names = HashSet::new();
let mut no_i18n = HashSet::new();
let mut rng = rand::thread_rng();
let localization = LocalizationHandle::load_expect("en").read();
let entity_configs =
try_all_entity_configs().expect("Failed to access entity configs directory");
for config_asset in entity_configs {
// TODO: figure out way to test events?
let event = None;
// We have no way to request all possibilities, so cycle a few times
// to hopefully cover all genders and such
for _ in 0..10 {
let entity =
EntityInfo::at(Vec3::zero()).with_asset_expect(&config_asset, &mut rng, event);
let name = entity.name;
let Some(name) = name else {
no_names.insert(config_asset.to_owned());
continue;
};
let Content::Attr(key, attr) = name else {
panic!("name is expected to be Attr, please fix the test");
};
if localization.try_attr(&key, &attr).is_none() {
no_i18n.insert((key, attr));
};
}
}
if !no_names.is_empty() {
panic!(
"Following configs have neither custom name nor default: {:#?}",
no_names
);
}
if !no_i18n.is_empty() {
panic!("Following entities don't have proper i18n: {:#?}", no_i18n);
}
}
}
With it, we could just iterate over every possible body and get what we need.
Our game is a big project with a lot of people contributing to it, and we want to be sure that it functions correctly and catch all errors if possible.
And my unsolicited advice: I'm not sure if people should be contributing to open source to become better developers. I'm an open source maintainer myself, and I'd rather have people contributing to the project because they care about it and want to make it better.
And that's why I filed this issue and this PR. I use strum and I want it to be better and fit my needs and hopefully the needs of other people.
P.S. Sorry for turning this issue thread into a battleground. I overreacted. I thought I got a comment from an AI spambot.
Thanks for the details! Sounded like it was open source is it this? https://github.com/veloren/veloren/blob/7d74238009cdbaa6cb96c1957058a8da6f3e3ee2/common/src/lod.rs#L3
Thanks for the details! Sounded like it was open source is it this? https://github.com/veloren/veloren/blob/7d74238009cdbaa6cb96c1957058a8da6f3e3ee2/common/src/lod.rs#L3
I'm not familiar with this file in particular, but yes, it's the right project and it's the right derive, heh. You can check out the MR I made for the macros I mentioned. https://gitlab.com/veloren/veloren/-/merge_requests/4843
For my use case of recursive iterations, I have a nested enum that has a method kindof like this:
impl MyEnum {
fn name_normalized(&self) -> &'static str {
match self { ... }
}
}
Then, given an initial String, I want to loop through every possible variation of MyEnum and check if it matches the given String. To get an iterator of every variant I have manually included the recursive cases, but it would be much more convenient and correct to use #[strum(flatten)] (or my personaly opinion #[strum(recursive)] to be more explicit but I understand the flatten from serde) as adding new variants will not require a code change elsewhere
An alternative approach for accomplishing this (If the maintainer is concerned with over-complexity) could be as follows:
The general approach is to allow recursion, but put the onus on the developer to implement the chain without the risk of the iterator size increasing in an unintended geometric explosion. The user defines a function that describes how to chain enum variants that have associated types (much like serde's default attribute.)
pub enum Hero {
Knight
Wizard
Thief
}
pub enum UIButton {
Attack
Flee
#[strum(chain = "select_hero_chain")]
SelectHero(Hero)
}
pub fn select_hero_chain() -> Iter<UserInterfaceButton>{
UIButton.iter().map(|hero| UserInterfaceButton::SelectHero(hero))
}
fn main() {
for button in UIButton.iter() {
println!("{button}");
}
}
// output:
// Attack
// Flee
// SelectHero(Knight)
// SelectHero(Wizard)
// SelectHero(Thief)
The benefit of this is that you can implement the chain functions with strum's existing functionality, at the cost of a fair amount of boiler plate for highly complex enums.
I think this is possible to implement, if OPs approach is considered out of scope.
