LPGhatguy / thunderdome

Arena type inspired by generational-arena
Apache License 2.0
197 stars 15 forks source link

Index<T>? #16

Open toyboot4e opened 3 years ago

toyboot4e commented 3 years ago

Can I hear your thoughts on adding type parameter T to Index?

For example, current Index in thunderdome would look like this:

pub struct AttackCommand {
    pub actor: Index,
    pub dir: Dir,
}

If Index is typed with T, it would look like this:

pub struct AttackCommand {
    pub actor: IndexT<Actor>,
    pub dir: Dir,
}

It depends on preferences, but I thought the latter would make more sense.

What would you think? Thank you.

toyboot4e commented 3 years ago

I tried adding it.

(Edit: implemtended Clone, Copy and Debug manually)

LPGhatguy commented 3 years ago

I like the idea of strongly typed keys, but I'm not sure whether they belong in Thunderdome as-is. We could add a new IndexT<T> type with the existing arena, but we could also add a new submodule that works with only strongly-typed methods.

LPGhatguy commented 3 years ago

~Thought about this some more. Maybe we could add the type parameter but make it default to ()? That way, existing consumers aren't impacted.~

EDIT: This would break a lot of consumers who now need to specify what their Index type is. Default type doesn't help here.

LPGhatguy commented 1 year ago

I thought about this some more. If we wanted to, we could introduce another type parameter on Arena that is the type used for the indices.

Arena<T, I = ()> would have indices of type Index<I>. Existing consumers should be unaffected, and users could opt into an additional smidge of type safety.

toyboot4e commented 1 year ago

LGTM! Today I’m using my own arena and I'm not going to make a PR, but that sounds clever.

tyler274 commented 1 year ago

I thought about this some more. If we wanted to, we could introduce another type parameter on Arena that is the type used for the indices.

Arena<T, I = ()> would have indices of type Index<I>. Existing consumers should be unaffected, and users could opt into an additional smidge of type safety.

I would appreciate this feature a lot actually.

edit: Implemented in https://github.com/LPGhatguy/thunderdome/pull/39