[RFC] System name constants

[azriel91]

Hey, I had to write this out so that I don't forget it.

---

When a bundle adds a system to a dispatcher, it provides a name which is used in system dependency ordering. The system name is effectively API, as external systems that depend on that system specify it as a dependency.

While the number of systems is small, it isn't hard to maintain a few &'static strs. However when there are many systems, if I make a typo, or the name has changed, using &strs defers failure to application startup (runtime) instead of compile time. If we can use constants or a function that returns a derivable system name, it would decrease the maintenance cost for larger applications.

Things to consider:

• Systems are generally not public outside of a crate, so we'd have to expose the name some other way.
• What about Systems with generic parameters, such as AnimationControlSystem<I, T>, where the type parameters are defined by the consumer / application code

Possible useful crates:

• named_type, but would depend on https://github.com/cjhowe7/named_type/issues/5
• typename

Alternatives:

• Do nothing — perhaps the maintenance cost isn't big enough for this effort to be undertaken.

[AnneKitsune]

Currently I'm not sure this is something worth changing. Since you usually declare all your Systems in the main (for small to medium games. big requires states containing other dispatchers, but all systems are still declared at the same place), it is pretty easy to see what goes where, except with bundles where you have to dig in the code to find the name of the Systems. Maybe a doc update including the names in the bundle doc would be helpful though?

For example, this https://github.com/amethyst/amethyst/blob/b24355d36fc728b19eb4f1ccebfe805acc6ffd21/amethyst_core/src/transform/bundle.rs#L12 is missing a "parent_hierarchy_system" entry

[azriel91]

Since you usually declare all your Systems in the main (for small to medium games. big requires states containing other dispatchers, but all systems are still declared at the same place), it is pretty easy to see what goes where, except with bundles where you have to dig in the code to find the name of the Systems.

The part about "except with bundles" doesn't uphold the "all systems are declared at the same place" bit. Consider the scenario:

• Bundle A adds systems A1, A2, A3

• Bundle B adds systems B1, B2

• We need to be able to say B1 depends on A1 and A2.

  This has two possibilities, either crate B knows about crate A, or the function that constructs the dispatcher know about both A and B, and passes in names for each of the systems.

• In either case, it would be nice to not require:

  • crate B, or
  • the function that builds the dispatcher with bundles A and B (and hence systems A1-3 and B1-2)

  to use string names for A1 and A2, but pull them out of the type — similar notion to strongly typed vs stringly typed. i.e. fail at compile time if I make a typo. This also allows the definition of the System to be complete on its own — I'm of the understanding:

  • It is incorrect to add two different Systems with the same name to the same dispatcher.
  • It is incorrect to add two different instances of the same Systems to the same dispatcher, so if two different bundles happen to try and add SomeSystem<u32> it should fail.

Re:updating docs, I know docs should be updated, but it's also easy to miss / forget, whereas when code is updated, the compiler catches it, and I often lean towards that automated catch.

Re:effort, I've recently implemented the system naming using typename in my game, and the effort to implement this isn't too high, though it has some technical decisions that I'm not sure we want in Amethyst (hence this RFC):

• The typename crate provides a Struct::type_name() -> String, instead of -> &'static str
• Generic structs such as crate::MyStruct<A: TypeName, B: TypeName>, when used as MyStruct<Hello> will return "crate::MyStruct<Hello>".to_string(), instead of "my_struct"
• To use it, we just #[derive(TypeName)] on Systems.
• Either take Into<String> or pass in &MySystem::type_name() to the dispatcher builder.
• Systems that may be depended on have to be pub, to allow type_name() to be used

[AnneKitsune]

You actually have to implement this in the shred crate. If it works there, then on the amethyst side we can add it too.

[AnneKitsune]

I thought about it, and it may make sense to change the 'static str to a T: Eq in some cases.

I'm not sure how you could make this work with the engine's bundles though.

[fhaynes]

@azriel91 @jojolepro is this still relevant?

[AnneKitsune]

@torkleyy do you want to take note of this somewhere? Should it be reopened as an issue on the shred or specs repo?

[azriel91]

For now I'm happy to close until we decide to engineer either or both of these:

• Ergonomic error diagnostics: If we sprinkle #[derive(NamedType)] or #[derive(TypeName)] everywhere in shred, specs, shrev and amethyst, we can output the unregistered resources upon a fetch failure on stable Rust. Possibly behind a feature flag of course, in case people don't want to add that compilation dependency.
• Named systems: Right now we pass in &strs, but sometimes that's simpler. See earlier posts in this issue for additional rationale.

[fhaynes]

Moving this to the RFC repo