Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why use strings as labels? #103

Closed
Plecra opened this issue Aug 10, 2020 · 12 comments
Closed

Why use strings as labels? #103

Plecra opened this issue Aug 10, 2020 · 12 comments
Labels
A-ECS Entities, components, systems, and events

Comments

@Plecra
Copy link
Contributor

Plecra commented Aug 10, 2020

Why have you opted to use strings as labels in Bevy's APIs? It seems strictly less expressive than using ZSTs for the same purpose.

struct DoThings;

App::build()
    .add_stage_after(Update, DoThings)
    .add_system_to_stage(DoThings, something.into_system())
  • Type checking
    • It's not huge, but tracking down bugs stemming from little typos is annoying
  • Scoping
    • Types can be completely private to specific modules, allowing an implementation to pop implementation details into the ECS without needing to worry about it being observable. (This is just an extension of the same principle in Rust's sound/correct APIs)
  • Documentation
    • This creates a clear location for the developer to document the purpose of the label, which can be found easily with editor definition jumps
@cart
Copy link
Member

cart commented Aug 11, 2020

This is a reasonable proposal. I'll need to give it some thought (and maybe some experimentation to see how it affects ergonomics). But the points you make are all valid and I don't think it would cause any "usability regressions".

@tbillington
Copy link
Contributor

Just my 2c, making a const str then using that would solve your type checking & documentation points, and possibly also scoping but I haven't thought about it too much.

@Ratysz
Copy link
Contributor

Ratysz commented Aug 11, 2020

This is similar to specs using strings to specify relative execution order between systems; Legion's way of avoiding doing that were stages (which is think is not the best way of handling this at all, but that's a topic for another time), similar to those used in Bevy now.

The way I approached this in yaks is similar to that of specs, but doesn't rely on strings - instead, the user provides the type they are going to use as handles to diffirentiate systems; it can still be strings, but it can also be a user-defined enum, or anything else that is Sized + Eq + Hash + Debug. The handles are discarded on build (implying all systems and any dependencies between them have been defined) and an opaque usize newtype is used internally from there. Moreover, a couple of API tricks allow the compiler to infer a default dummy type if the user never specifies a handle (implying that no systems have execution order dependencies), so the API is not polluted by type signatures that do something the user doesn't need.

It might be possible to use the same tricks here, with effectively no API change from the perspective of a user - other than providing them with the option to use something other than strings to differentiate between stages.

@kazimuth
Copy link

There are some problems with serializing type IDs over time / across crates, see #32.

I think the const str approach is alright, those actually exist for the default stages. It's simple and straightforward to understand, and the user doesn't have to deal with wacky reflection issues / generics, which can be a big barrier for inexperienced rust users. Imo a good compromise would be strings + documentation of the "use a const string" best practice.

@Plecra
Copy link
Contributor Author

Plecra commented Aug 11, 2020

I'd probably consider the scoping the most valuable part of this. Allowing the reader to understand a system locally using the privacy of the types could enable better abstractions, by providing guarantees about the Bevy's state within the private stages.

More API considerations would need to be made to get the most out of this kind of privacy of course. I suppose I'm saying it's a trade-off, and types-as-ids have interesting properties for developers

As I understand it, the ECS stages would never need to be serialized, so that shouldn't be an issue. Am I mistaken?

@cart
Copy link
Member

cart commented Aug 11, 2020

Lots of good conversation here. Something that I haven't called out yet: stages must have human readable string names for the (eventual) editor schedule visualization tools. We could still have that with typed stages, but we would need them to implement a trait that returns a name.

@zicklag
Copy link
Member

zicklag commented Aug 11, 2020

Maybe you could just allow anything that's Sized + Eq + Hash + Display. That way you cover the need to be able to see it in an editor, etc. and you allow for the privacy gains that @Plecra brings up if you use ZSTs.

@Ratysz
Copy link
Contributor

Ratysz commented Aug 12, 2020

What about a combined approach, type ID + the hash? This allows using ZSTs as well as sized types (which can also be module-local), allowing scoping and things like strings. Name-providing trait would still be necessary, but possibly can be blanket implemented on many of the applicable types.

(Side note: the Debug bound from my previous post is actually a fortunate implementation detail - that's how dummy handles are differentiated from proper handles by the compiler. The name-providing trait would fill this role perfectly.)

Edit re: side note. There won't be any need in dummy handle tricks if stages are keyed by literally (TypeId, u64), the latter part being the hash. I should try that in my lib...

@karroffel karroffel added the A-ECS Entities, components, systems, and events label Aug 12, 2020
@alice-i-cecile
Copy link
Member

There was some detailed discussion and analysis on this in #1021 starting here.

@cart's tentative conclusion was "any key + type constraints + derives" seems to be the best solution, but this is worth considering a bit further I think.

In addition, we're running into a virtually identical issue with system labels, relevant to #1144. We will probably want to mirror the solution we settle on to system labels as well.

@TheRawMeatball
Copy link
Member

I implemented a version of this here: https://github.com/TheRawMeatball/bevy/tree/zst-labels

@Ratysz
Copy link
Contributor

Ratysz commented Jan 19, 2021

In the interest of having options conscisely listed in one place, here's an example (and implementation driving it) of that "any key + type constraints + derives" pattern (which I've been referring to as "heterogeneous key trick"). A "real" implementation would also include a derive macro for StageLabel. Here is the accompanying message with some explanation.

@alice-i-cecile
Copy link
Member

Fixed in 0.5.

hymm referenced this issue in hymm/bevy Jan 28, 2023
add a system to run the enter schedule of the initial state
tychedelia pushed a commit to tychedelia/bevy that referenced this issue Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events
Projects
None yet
Development

No branches or pull requests

9 participants