Open alice-i-cecile opened 3 years ago
DJMcNab
commented
3 years ago I agree with this, although I think we should also include each sub crate as an item in the prelude too, and ensure everything is available from the roots of those crates
That is, make PbrBundle accessible as pbr::PbrBundle for example
cart
commented
3 years ago I'm definitely open to this. Lets discuss as many alternatives as possible and try to get as many people to weigh in as possible. This is the kind of change I only want to make once, so I'm happy to move slowly on this :smile:
@DJMcNab's idea is interesting. It does reduce the total amount of imports at the top, but such a pattern is basically encouraging people to prefix the module in front of every type they use (rather than importing the module and/or type at the top of a file). I'm personally an "import at the top" kind of person (with inline module paths to resolve conflicts and one-offs), but I'm curious what others think. I'm also a little worried that it would encourage imports in the style of bevy::prelude::pbr::PbrBundle instead of bevy::pbr::PbrBundle (the latter of which is clearly nicer).
DJMcNab
commented
3 years ago I don't know which of those paths rust-analyzer would auto-import - it's worth experimenting. My suspicion is that it would do bevy::pbr::PbrBundle - at least I'd hope it would if we did the import in prelude as pub use crate::pbr;
alice-i-cecile
commented
3 years ago Another subtle user-induced bug caused by the huge glob imports: silently shadowing the stage enum when defining your own stage module to track your stage names. See this thread in #help.
Nilirad
commented
3 years ago Another subtle user-induced bug caused by the huge glob imports: silently shadowing the
stageenum when defining your ownstagemodule to track your stage names. See this thread in #help.
Yes, happened also to me on bevy_prototype_lyon. I got it resolved when I started importing only the stuff I needed. That change made naming stuff way better.
concave-sphere
commented
3 years ago Regarding https://github.com/bevyengine/bevy/issues/1247#issuecomment-761140205:
I'm personally an "import at the top" kind of person (with inline module paths to resolve conflicts and one-offs), but I'm curious what others think.
Oh, have I got an opinion! :smile:
Brief background to explain where I'm coming from. Professionally (i.e, not here) I've spent the last few years working in Go. Prior to that, I worked for about half a decade in Java. And before that I was working primarily in C++, which is closest to Rust out of these three, but didn't have a strongly established culture around naming (that may have changed, I haven't been following the various C++XX releases).
The reason I bring this up is that Java and Go have very different approaches to imports, and I have a decent amount of experience with both of them, so I think I have grounds for comparison.
In Java, there is no mechanism to import a package name -- you have to either provide a fully qualified name, based on DNS style roots (com.example.my.long.package.MyClassName), or you have to import the name into your local namespace and reference it direction (MyClassName). Because Java package names are generally extremely long, Java developers usually try to make their names globally unique. And due to the influence of design patterns in the Java community, classes often implement one or more pattern and stack up names describing the patterns they implement. Thus, the infamous MyClassNameVisitorFactoryBuilder.
Java also has a mechanism for star-imports (importing everything in a package into scope). At my workplace this was banned, and I believe this is common practice outside very rough and ready prototyping. The basic problem is that a star import makes it very hard for other engineers to understand where a name is coming from, or to predict which names will lead to collisions, or which names are available to their code.
Go takes a very different approach to namespacing from Java. Go import declarations act on a package.
By default, the name of the package is the last part of its fully qualified name. For instance, import "foo/bar/baz/mypackage creates an identifier named mypackage. You can rename a package when importing it, such as import mp "foo/bar/baz/mypackage", which creates an identifier named mp. There is no mechanism for importing a single name into scope from a package.
The Go community (at least, the parts of it I've interacted with) also has clearly established conventions around how to use their import mechanism. The references I usually cite to explain this are this blog post and this section of Effective Go. But the key point is that when designing APIs in Go, the name of the package is part of the name of the item, and the way your users will experience the name. This snippet from Effective Go shows how different it is from Java:
Another short example is
once.Do;once.Do(setup)reads well and would not be improved by writingonce.DoOrWaitUntilDone(setup). Long names don't automatically make things more readable.
Another way to look at this is that talking about a Go identifier without discussing its package is like, eg, talking about a method without discussing the object to which it belongs. The iter method is not very specifically named, but it's only ever encountered by users as a call on a specific object. There's never a question of which iter is in use.
Go also has an equivalent to star-import, import .. As in Java, its use is strongly discouraged outside a few very specific scenarios (primarily when, for technical reasons, you need to put tests in a different package from the tested code). There's also a variant import _ which, as in Rust, imports the package without actually defining any names. These are both very unusual and I won't discuss them further.
Another Go specific technique (possibly specific to my workplace or even team) is the use of totally meaningless module imports, like mwln. This is generally when importing names that have been autogenerated from cross language interface specs where the declared names are already unique. So modulewithlongnames gets imported as mwln -- when writing code, you have to remember the abbreviation, but mwln.VeryLongAndUnambiguousName is just as readable as modulewithlongnames.VeryLongAndUnambiguousName. Arguably this is a workaround for the lack of name level imports, but it works fine.
I've only been writing Rust unprofessionally for about four months now, so I can't speak to its longstanding traditions, whatever they may be; I haven't found an equivalent of Effective Go or Effective Java for it. The module system, at least in terms of its handling to namespaces, is closest to C++: you can import both names and modules. Imports don't have to happen at any particular place in a file, either: you can use for a whole file, or for a single statement block, or anything in between.
This is good, in that it gives you the freedom to do whatever you want, but it's bad, in that you have to decide what the best thing is.
The Go recommendations regarding package names were surprising to me when I first encountered them, and I think they're surprising to most engineers who have previously worked in "more verbose" languages. When we bring on a new team member who isn't familiar with Go, they pretty much always are taken aback by how names are picked, and I have to walk through the rationales with them. But they work, and they work really well. There are parts of the Go language and community practices I don't like, but IMO they really nailed this aspect.
Why do I like them? Module level imports are a great balance between (1) polluting your namespace with dozens of names, and (2) fully qualifying everything in the world. They make it clear which names are foreign without reading the list of imports. And if you aggressively strip out redundant parts of the names inside the module, they don't add boilerplate -- you just move parts of the name from the right side of the qualifier to the left. Finally, module level imports are helpful in that they remind readers about the structure and organization of the library they're coming from, making it easier to find related names and docs.
Module level imports are sometimes a bit harder to design -- picking good names that are unambiguous without being redundant is a bit of an art. But following the general principle that code is read far more often than it's written, this is the right place to put the burden, rather than on the person trying to read a potentially unfamiliar code base.
What I'm not sure about is whether this reasoning is applicable to Rust. As I said above, I'm fairly new to Rust and I don't have a sense of "common practice" yet. Unlike in Java, Rust programmers have the option of importing whole modules. But unlike in Go, they also have the option of importing type names. This feels simpler, and I worry that users of a crate might do it reflexively, without considering the impact on their experience. But it's possible that with clear guidance and docs, this won't happen.
My pet project has a library that I've attempted to design as-if I was going to release it (who knows, maybe I will someday). I experimented with a couple of naming conventions, ranging from full qualification to full Java MassivelyUniqueVeryLongNames, and settled on a fairly Go style, where names are designed to be imported by their module -- with the exception that types named after the module are pulled directly into the namespace (so square::Square is imported as square). The front page of my Rustdoc has a section explaining the intended import style.
As a user of my crate, I find this really pleasant: I get to import as few names as possible, but the resulting code is still very readable and unambiguous. I have a prelude, but it only consists of anonymous trait imports (note: I would not recommend this for Bevy, which is a different kind of library -- e.g, the Query DSL is really important to have in scope).
But since I don't have any users besides myself, it's possible that I've just designed something that fits the quirks of my brain and no one else can understand it. :smile:
Sample code from my rustdoc:
use gridded::prelude::*;
use gridded::grid::square::{self, Square};
let sq = Square::new(10, 10);
let sq_right: square::Edge = sq.right();
let geom = square::Geom::relative_to_grid_origin();
// Invoke the trait method `outward_unit_normal`:
assert_eq!(
glam::vec2(1.0, 0.0),
geom.outward_unit_normal(sq_right));Note: I've lately started compressing paths by re-exporting types named after the package "one level up". e.g, the code above could have imported gridded::grid::{square, Square}. This is convenient, but I'm not sure the loss of organization is worth it, considering that a simple import statement can pull the nested name in (as above).
In my use of Bevy, I went back and forth.
I started out importing prelude::*, but found that introduced way too much namespace pollution, and was confusing when learning the library because I didn't know where names were coming from in order to look up related types and documentation (side note: my 11-year-old son actually brought this up unprompted when I asked him about his attempts to learn Bevy: "I never know where to look for the docs for anything" ... I think he hasn't found the "search" button in Rustdoc yet).
I then switched to fully qualified imports for everything, but this was way too much. bevy::ecs::system::Query<(&Tp1, Tp2), bevy::ecs::query::With<Tp3>>. Yikes.
My current standard Bevy header is:
use bevy::ecs::prelude::*; // This has about the right stuff in it for the Query DSL
use bevy::ecs;and this works great. I only have one ECS in scope, so ecs:: is lightweight and unambiguous, and most of the other Bevy names only get used a couple times per file and/or are near the tops of their modules, so I prefer qualified references to them. bevy::asset::Handle is another one that I may make an exception for, but I'm not sure I want Handle in my root namespace. I might use bevy::asset instead.
The current Bevy prelude is very large, and there's not much rhyme or reason to it. Stripping it down to a more minimal set reduces the difficulties with name space pollution (and anonymous imports), and keeps it more in line with standard Rust philosophy.
Proposed prelude list:
Related changes: