Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
stable reflect type name #5805
stable reflect type name #5805
Changes from 20 commits
c9ad9a5
bab4ae5
06e9258
312659a
6982351
317372f
30a663f
3a8958a
a710c17
006c8e8
a7e8dd6
8eeb06d
140e009
6fba583
4f17b0c
c135dae
11889a1
fe2ff0b
e7fc222
ca5e875
0563cf9
be2b3d5
8cd535a
8af97fc
eb1e2f6
80b22d8
9682cec
d9d3afa
329f248
e6a3ce8
4722a19
aecba77
4fd9a53
b2be7e9
9ea7f9a
b697bc3
a6654c1
34f57b7
26460dd
f4a6655
45eca48
e2e3a02
3f2ee5d
ba4da30
d8987f4
d155529
0c25180
b0a6604
9ef1454
83e3500
b1411ab
86ab916
5e87d13
7295583
85739b8
c9ec3e8
ef4986e
4feef51
6f750c6
563cb75
9c3a034
dbd4bde
f67f1af
3aa805a
188b2f3
08e2c8f
a7aa4de
184d335
fa9bdb4
9dda494
3668407
b449fbe
4d42f12
a4c93cf
2afbd85
fab49b1
c914af0
86e12d2
cb84e1e
60ff2b7
e4efa1b
02aabfa
afda7b3
aaef516
ddf4726
0b0cf93
88d2082
4f9d953
c28c3b4
a46a028
5303da3
5a70e6f
de07bd5
858a6c2
504495b
226dad2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heres an idea (which I haven't yet fully sorted out my opinion on) : now that we're allowing people to define "canonical" names, maybe we should replace TypeUuid with that? Not something to decide or implement in this pr, but a good thought experiment.
It would give the people the option to fall back to the default type name if they don't want the extra boilerplate:
Library authors would have the option to define a shorter / "stable to module refactors" type name like:
The biggest wins are:
The question we should ask is "if its good enough for components, why isn't it good enough for Assets"?
Some downsides are:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, requiring the crate name in TypeName does largely alleviate problem (3). Weirdness when pulling in two crates with the same names and type paths is a corner case I think I'm willing to accept.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about two incompatible versions of the same crate? Even if they aren't used in the same version of the game, you could still have assets using the old version lying around. With uuid's that is easy to fix by generating a new uuid every time there is a breaking change, but for type names you would have to rename the type in code (and all usages) and make sure you don't accidentally every reuse the name again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't given this a ton of thought yet, but I think asset versioning should be done in the AssetLoader trait, not by changing the identity. Having a stable identity across versions feels like the right way to establish continuity / link loader versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kind of just riffing on my previous comment about "replacing TypeUuid with TypeName", but I'm not a huge fan of adding "yet another" required derive in places like this.