-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Add a debug-only alternative to Name
#21509
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
base: main
Are you sure you want to change the base?
Conversation
…led. Also removed old TODO.
It may be worth using Which would turn the example into this
|
If I might be permitted to engage in a bit of minor bikeshedding, another possible name for this could be
|
Agreed. I was worried about brevity, but now I think that's a red herring. If someone's compiled out debug tags then they probably meant to compile out debug logs as well. So it's an exceptional situation and ok to be a bit verbose. Something else I've realised is that I'm not sure how inspectors and BRP should handle compiled out debug tags. Currently it serializes the redacted message, but in that case an inspector should probably display nothing. I might need to change the string to be in an Option, so it can serialize as None when compiled out. |
Bikeshedding welcome! I agree with your points, and that "tag" is not ideal. But I feel fairly strongly that the name should include Maybe |
I'm fine with |
I'd prefer to hold off on this sort of thing until we have upstream inspector / editor tools (and BSN), which will allow us to better understand and solve the problem in context. As it stands I'm not yet convinced that Name is not sufficient. |
@cart I'm having a really hard time seeing why you aren't seeing what I am seeing :) Let's take the case of names in skeletal meshes As I understand it, Moreover, the producer of the bone names is not the same as the consumer. The bone names are determined by (a) the artist, and (b) the developer who wrote the asset importer. Both of these people have, in effect, promised the consumer (the person who is searching for bones by name) that bone names will be non-conflicting. It's an implied contract. There are many other uses of names which require similar implied contracts between different creative roles: the consumer depends on the fact that the producer of the names are following certain rules. This implied contract isn't something that the producer and consumer agree to on an individual basis - there's no handshake meeting. Instead, it is generally understood by practitioners within the industry that certain rules must be followed. It's table stakes for being a participant in the ecosystem. These kinds of contractual policies generally go hand-in-hand with a namespace. What's the difference between domains in the ".org" TLD and the ".gov" TLD? At the purely technical level there is none. However, what these TLDs represent are different policies: what kinds of organizations or institutions are permitted to register names in that space. Let me repeat this: "namespaces are policies". Back to our skeletal mesh example. One might say, "OK, so within the restricted context of armatures, we need to have a particular policy of how names are assigned, but outside that, it's anything goes." But that's confusing: it means that the developer needs to learn different rules for different contexts. Consider than in HTML you have both "id" and "class". Both are used as identifiers and both can be used as filter terms. However, "id" has an explicit policy of uniqueness. Even though it's not enforced by the browser, every web developer knows how ids are supposed to be used. Not only do the tutorials say this, but the web APIs strongly encourage uniqueness. There's no guesswork about which contexts require unique ids and which do not. The rules are simple and universal: if you want non-unique names, then use a different attribute (like "class"). Bevy, on the other hand, has a policy of "no policy". Do whatever you want with What this means, then, is that as a consumer of I mentioned that I had made a mistake of using If I had known that, I would never have created so many duplicates. But I didn't know this, because there's no policy. |
I would argue that there is a policy, just not the one that you want. The policy is:
This "policy" largely stems from the following constraints:
I think the current state of things is a reasonable constraint solve. I would be most receptive to the following alternatives:
Just embracing a "policy" of "please make these unique or you're doing bad things" doesn't feel particularly trust-able as a user. |
Objective
Add a component that's similar to
Name
, but intended only for debugging. The user can choose to (mostly) compile it out in production builds. The goal is to avoid the ambiguities and other issues that come from usingName
as a debugging tool.Why a draft?
The PR is unfinished. I suspect it might be controversial, so I'm filing an early version to test the waters. The remaining work is trying out an alternative implementation (based on
DebugName
), sorting out the feature structure, and documentation.Background
The
Name
component is an ad-hoc way of identifying entities. Sometimes it's used as load bearing data - an app might have logic that expects certain entities to have certain names. Sometimes it's used as a debugging tool - a way for the user to recognize entities in an inspector or editor.The dual purpose of
Name
can be problematic. For a detailed breakdown, see this proposal. The gist is:Name
components intended for debugging can end up as load bearing, which causes issues like Examples that modify gltf materials are broken #19322.Name
components in production builds.Solution
This PR adds a new
DebugTag
component:The key differences from
Name
are:Name
includes a hash for fast comparison).debug_tag
feature is not enabled then it becomes an empty struct.Debug
. It does not implementDisplay
,Deref
, or comparisons.There's also a
debug_tag!
macro which helps compile out awkward cases.DebugTag::new(format!(...))
will not be compiled out, butdebug_tag!(format!(...))
will be. As far as I can tell, the common case ofDebugTag::new("example")
is compiled out.The PR also updates the
observer_propagation
example. This might actually be wrong - the point of that example is to print out log messages, so maybe they should stay asName
- but it makes for a nice demo. The final PR should decide which examples to update, and whether that should be a separate PR.FAQ
Why not call it
DebugName
?One issue is that there's already a
DebugName
inbevy_utils
. It's quite similar toDebugTag
, but it's not a component and doesn't depend onbevy_ecs
.A second issue is that I feel
DebugName
is too close toName
, and perhaps a bit misleading - "name" usually means the primary method of identifying something. "Tag" feels a bit more secondary and optional, and it's nice and short.Alternatives:
DebugDescription
orDescription
.DebugAnnotation
orAnnotation
.Flecs
has multiple options with different roles: "brief", "detail" and "link".DebugComment
component that's presented in a multi-line editor.DebugTag
is a name/sentence, whileDebugComment
is paragraph.What's to stop someone misusing
DebugTag
as a load bearing name?The string inside
DebugTag
is only exposed via theDebug
trait and serialization. So the user has to work pretty hard to misuse it - even a simple comparison needs something likeif format!("{tag1:?") == format!("{tag2:?}")
.Why not implement
Display
?Abundance of caution. On the other hand, implementing
Display
would be convenient and backwards compatible withName
.What does
Debug
print when thedebug_tag
feature is disabled?It prints
[REDACTED]
, so theobserver_propagation
example log looks like:I like this as it's brief and hard to confuse with real output. It's also a bit whimsical, like someone's censored a top secret document.
DebugName
takes a different approach and printsEnable the debug feature to see the name
. I find this verbose and awkward - the example above would become:But it is nice that the user can work out how to enable the feature.
Maybe printing
[DEBUG_TAG]
would be a compromise - if the user searches for "debug_tag" then they'll probably come across the documentation.How does this affect inspectors and editors?
I'd guess that inspectors should treat
Name
andDebugTag
as equivalent, and display them together if the entity has both. Or maybe that's too verbose and one should take priority.For editors, I'm guessing they should distinguish between
Name
andDebugTag
to reduce confusion.Name
would be the primary focus, andDebugTag
would be secondary or displayed in a tooltip.DebugTag
isn't really compiled out - it becomes an empty struct. Why not compile it out entirely?Ergonomics. Compiling it out completely would be safer, but also require users to apply a lot of
#[cfg(feature = "debug_tag")]
.I haven't tested efficiency. I'm assuming the ECS handles ZST components as well as it can, but two otherwise identical entities will have different archetypes if only one has a
DebugTag
. Maybe there's a way for the ECS to optimise this further based on the feature flag.Another option might be to make
DebugTag
aQueryData
instead of a component, so the actual component is hidden from the user and fully compiled out. But I suspect there might be complications or confusing behavior around adding the component. Maybe someone more familiar with the ECS could advise.Couldn't
DebugTag
be implemented asstruct DebugTag(DebugName)
?Perhaps - the implementations are currently very similar. Although
DebugName
seems specifically geared towards type names, so maybe there's a risk they diverge in the future. I don't feel strongly either way.Why add a new
debug_tag
feature? Couldn't it reusedebug
?Probably, yeah. I'm mainly being cautious for now. I also recall some discussion of whether
debug
is a bit too vague.This would become moot if the implementation is changed to use
DebugName
, since that's gated ondebug
.Shouldn't
NameOrEntity
supportDebugTag
?Probably. I haven't worked this through. Like inspectors, there's the question of what to print if there's both a
Name
andDebugTag
.What might come after this PR?
There's a few places where the engine currently adds
Name
components.bevy_input
adds a name to gamepad entities (gamepad_connection_system
).bevy_gltf
adds a name to most of its entities.primitive_name
).These should be changed to
DebugTag
or typed names. MaybeName
support could be kept as an option to ease migration, particularly for glTF.Testing