Identifiers: Explicit Generics VS Auto-generated + bindings

(Joël Lupien) #1

Context

When having to identify anything, there are three (or more) ways to do it. In amethyst, we tend to never know which one to use, so this conversation will change that.

Let’s start with a super general use case.
We have a HashMap as a resource. It looks like this:

type Mapper<K> = HashMap<K, Entity>;
world.add_resource(Mapper::<K>::new());

Let’s go with the possible solutions:

Hardcoding

type Mapper = HashMap<String, Entity>;

world.read_resource::<Mapper>().get(String::from("Key1")).unwrap();

Pros

  • Easy to use

Cons

  • Memory and performance intensive
  • Have to hardcode strings throughout the program when you want to get a value you know the key of.

Generics

type Mapper<K> = HashMap<K, Entity>;

world.get_resource::<Mapper<MyEnum>>().get(MyEnum::Key1).unwrap();

Pros

  • Can be any type that is defined by the user.
  • Can still use strings for simplicity.
  • Forces the user to define a key for each value.
  • Strong and clear typing. Using a custom enum as the key ensures the code where you retrieve the value is clear.

Cons

  • Generic propagation is a pain: Each time you fetch the resource you need to use Mapper instead of Mapper. (You can fix this with type aliases)
  • Forces the user to define a key for each value.
  • Higher cognitive load (what type did I define the other day as the key for the list of entities?).
  • Chance of panic of usage if you use a different type than the one you defined earlier (see previous point).

world.add_resource(Mapper::<String>::new())

world.get_resource::<Mapper<i32>>() -> panic!

Auto-Generated id + binding

type Mapper = HashMap<u32, Entity>;

world.get_resource::<Mapper>().get(MyEnum::Key1.into());

enum MyEnum {...}
impl Into<u32> for MyEnum {big match}

Pros

  • Almost as simple as the String version at the usage point.
  • Can use auto-generated id if you don’t care about using the key. (but why a hashmap then?)

Cons

  • Hard to setup the enum properly (if you want to do that).
  • No meaning to what the key 8345 is supposed to be.

Discussion

Did I miss anything?
Do you have alternative solutions?
Which option do you prefer and why?

0 Likes

(Théo Degioanni) #2

Can static references be used as keys in a hashmap?

0 Likes

(Joël Lupien) #3

I think so.

0 Likes

(Théo Degioanni) #4

Then static references should be preferred over String to save memory.

0 Likes

(Hilmar Wiegand) #5

We’ve just had this discussion over what to use for Widget IDs, so I’d like to add my thoughts on the issue here.

First a short TL;DR:

  • I think we should offer a generic solution for user created entities or widgets, but have the identifier type hardcoded for builtin systems.
  • As for the type, I think u32 or u64 (depending on the domain) would be best, since they are cheap and can be aliased with enums.
  • There should be options to auto-generate ids as well as to provide your own IDs for things you create.

I think the first big issue with complete customisability is that we place the burden to choose on the user. We’ve been trying to make jumping into the engine easier for the average user, and forcing the user to a) define his own id types and b) define every identifier he wants to use is unnecessarily complicated. Defining your own ID types is something that you get to when you’re pretty far into the architectural process and already thinking about how it will scale and work with the rest of your game, not when you’re just trying to get a few buttons and labels on the screen.

I would be fine with having an abstract impl (impl Widget for Button<T> { type Id = T; }) and then providing a default implementation (type SimpleButton = Button<u32>;), but I think only providing generics is not an option and would destroy ergonomics - The possibility to auto generate ids is a must for quick prototyping imo.

The second thing is that most of our lists/maps/data collections live in the World. They are defined and retrieved by their type. Providing generics opens the door for type mismatches, as jojo already mentioned. I think it’s important to highlight that this does not just introduce the additional load of having to look up what ID type you used for your widgets or entities, but also a problem with external libraries:
Imagine there are libraries adding widget collections (like material or bootstrap components in the web world). These would have to clearly define the type for its IDs. If it differs from your own types, you would be forced to accept the library’s conventions, and if you have 2 libraries with conflicting type choices you would be forced to separate their data structures.

Generally, I don’t see what the problem would be with using u32/u64s for IDs - it’s worked fine so far for specs =P I think it’s important that for widgets/entities that can be “named” by the user, we always provide a way for the user to choose their own ID so that he’s not forced to refer to things by their auto generated ID. An example for this would be the following when using the button builder:

MyButtons {
    PlayButton = 1,
}
let some_button = ButtonBuilder::new().build(); // Auto generated id
let play_button = ButtonBuilder::new().with_id(MyButtons::PlayButton);

The only problem I could see with this is that without the source, it would be hard to refer to items by their “given names” in the code, because the editor would have to know what enum value the user assigned. I’m sure we can find a solution for that when it comes up though.

As a last thought, I think it’s still a good idea to provide the user with options. When he’s defining his own entities and widgets he should be able to override the ID type and choose whatever he want, but if he doesn’t, in my opinion he should never even come into contact with it, optimally.

0 Likes

(Joël Lupien) #6

You can just use String everywhere. We are not forcing anyone to define enums.

You could also decide to not register the widget inside of the hashmap register in this case. You won’t know what the key corresponds to, so you wouldn’t know how to edit the button’s values for example.

Can’t they propagate the generic down to the user? Check how amethyst_controls propagates amethyst_input’s generics.

I do agree that having to write ::<TYPE>:: everywhere is annoying, but my personnal opinion on the subject is with the side of generics.

I look forward to know what everyone else thinks of this, its really interesting.

0 Likes

(Azriel Hoh) #7

“Yes”; lateral to both, I prefer safety and troubleshootability. All of these have a usability related focus, let’s see what this means:

  • Simplicity

    “Let me get something up and running without too much code.”

    The phrase “too much code” can be expanded as “defining types” (which might have to implement certain traits).

    This can be addressed by:

    • Using an easy to instantiate type (e.g. String, str, u64)
    • Providing an easy to instantiate type as part of Amethyst (strongly typed, user doesn’t have to write their own)
    • Code generation via proc macro
  • Customizability

    “I want to define my own set of valid values”

    This allows the user to provide their own type. So, traits and generics. Allowing users to provide an enum makes it easier for them to identify that ButtonGameStart relates to their game start button, instead of having to use something from Amethyst.

  • Safety

    “I want to know that invalid values are caught be the compiler”

    Different perspective, but similar implementation to customizability. Strings and numeric types don’t provide compile time safety – e.g. typo, or changing the ID for something but forgetting to change it everywhere.

  • Troubleshootability

    Safety covers a lot of this, but you could still provide an invalid value (e.g. copy pasting ButtonGameStart and forgetting to change that ID to ButtonDemo).

    By using a custom strong type, the search space for errorneous values is greatly reduced – don’t have to wonder “is it a bug in Amethyst” for UI events, or is it “my system handling” etc.

This is partially covered by the above:

0 Likes

(Hilmar Wiegand) #8

You still need to define that you’ll use Strings. A new user wouldn’t know why he needs to provide the ID type, that’s just mental overhead imo.

I think ideally, creating widgets is only supported through their respective builder, which would always insert them into the appropriate widgets resource, so you can still iterate over them. The builder would also return the ID when you build the widget.

They probably could - but for ease of use, not everyone will. I think it’s safer to provide a strong convention here, instead of relying on every library creator to choose the same way to implement things.

0 Likes

(Joël Lupien) #9

Forcing strings everywhere isn’t realistic imo. I included them for completion’s sake.

We want to know if being forced to choose a the id type is too much overhead for the user to be worth the benefits.

Solution 1

ie: InputBundle::<String, String>::new()

and

enum MyBindings {
Forward,
Backward,

}

Usage: for input in inputs { match input }

or

Solution 2

InputBundle::new()

and

enum MyBindings {
Forward,
Backward,

}

impl From<u64 for MyBindings {
… match input -> MyBindings::variant
}

Usage: for input in inputs { match MyBindings::from(input) }

The config file will contain u32s instead of MyBindings variants here.
0: ((Key::W))

Question: Which do you prefer?

0 Likes

(Lucio Franco) #10

not sure I totally understand but from a quick look it seems that with the second option you would have to use numbers to represent certain actions? That seems a bit hard to grok by looking just at the file no?

0 Likes

(Hilmar Wiegand) #11

To be honest I don’t really understand the example you’re making, at least I don’t really get why you couldn’t have enum variants in the config file instead of u32s, there’s always the option of using C-Style enums where each variant has a u32 value.

I agree that this is something we should avoid. However, as I said before, in the version I’m proposing, that would never happen as the user can always either use enums or have his ids auto-generated.

Edit: To be honest, I think the best way would be to have the ID types be completely generic like you suggest, but then offer defaults so that the user doesn’t come into contact with them unless he specifically tries to customize them. However the place where we run into problems with this is when we want to implement auto generation, make working with prefabs seamless and have prebuilt components/widgets/whatever that need to make assumptions about ids and their types.
So, I’m not generally opposed to them, just putting that out there.

1 Like

(Joël Lupien) #12

Last time I checked you couldn’t have an enum represented as a u32. Only u8 would work with #[repr].

You can’t have the enums in the config file with your proposition because the deserializer needs to know the concrete type -> generic. (Maybe if you use repr that is an exception, but I never tried that).

0 Likes

(Hilmar Wiegand) #13

Seems to work fine for me. Not sure how that would work with Prefabs and so on though. However, I do concede that users could run into problems if they try to define multiple enums that describe objects in the same collection, in which case they would always have to update the number values which sounds horrible to me.

As I said in my edit above, I’m really not opposed to the idea of having generics in general. My main problem is that the average user will just attach the types without having any idea what they do and what benefit he could gain from changing them - that, and the auto generation issue. If you have anything that could alleviate those 2 problems (there’s also the “different collections for the same object”-issue, but that’s not a showstopper for me), I would be much more willing to accept your proposal.

0 Likes

(Joël Lupien) #14

Not knowing what a generic is for can be fixed with very good documentation. (like very very good :stuck_out_tongue:)

However, automatic generation isn’t fixable.
However, if you have no pre-defined key (or autogenerated ones), you won’t be able to use the hashmap properly, except if you intend to iterate over all values.

0 Likes

(Hilmar Wiegand) #15

Auto generated values can still be totally usable:

// builder returns the auto generated id
self.some_widget_id = WidgetBuilder::new().build();

// later...
let my_widget = widgets[some_widget_id];
// do something with my_widget

Maybe I’m just so skeptical because I’ve never seen this pattern anywhere, I don’t know…

Edit: Just realized that I have seen this pattern; in many database ORMs you define what your ID is going to be (or what is going to be indexed on) and obviously you define type as well as field for that. Don’t know how transferable that is to this situation though. Definitely a tough question. Can we look at how other game engines solve it?

0 Likes

(Joël Lupien) #16

Ah that makes sense too.

0 Likes

(Hilmar Wiegand) #17

Actually, auto generation might be fixable. We could have a trait that implements generate_random_id or so, and is already implemented for all unsigned number types and String, and ultimately we could probably even provide a derive for that. That would make it difficult to use enums for an id though.

0 Likes