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
Entity Relations RFC #18
Entity Relations RFC #18
Changes from 13 commits
771682d
8f450d1
a45c111
6868bc5
8cffbe8
49d4f2a
1f40033
96065d4
f090896
adfb72e
94c59d2
5636cec
c4e2af6
23b975b
cf82d3a
8c0171c
df96a01
55dd988
a7aff66
2bbd838
e2ce855
014b33f
4ef058b
ca9d3eb
555a219
2bd4bfe
c88a592
86bfb3e
8727707
b504089
91f52b5
1a121bb
82548cb
40c8ebe
e5481f0
2daaaa2
fd261eb
2b280b1
b384868
0792124
00be482
362ad69
7d63fe8
afe1555
cb95234
9ff30f9
46a9d0e
bf6bd3e
6bc157f
c164e15
c0833fa
ed1694b
bcc3a4d
523cd59
07e47db
17d74d6
1099c39
766fb75
09df9a3
aee4dae
af543da
d1f90c3
8e858c0
4279361
674bd5b
dd82790
7c2cbcf
c9f0e38
23f758d
2bd87d0
2491423
bec0d2c
9f5f62c
a8c2f52
139baec
add64c0
4ac8ab9
7b79ff2
a24d20b
90d9436
d84c1d0
4081fe0
ab0c8bd
b1ce346
85519a3
3e06286
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.
I think the
InFrustum(camera_id)
would be especially understandable here. It's easy to see why it can't be handled by simple tag component here.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.
I worry that multiple camera frustums are bit too obscure of a topic for the average gameplay programmer referring to this RFC. I agree that they're a clear example though!
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.
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.
I'm not sure if this particular point is handled in any way by proposed relations. Afaict cycles are still an issue (in cases where you explicitly don't want them).
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.
The framing could be clearer. These problems are possible to address in a cohesive way at an engine-level by including relations as a feature. Min-relations won't hit all of these though :(
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.
Okay, so it's more of a future prospect. I guess that's fine.
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.
Clarified!
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.
Is &Relation correct? Following relations don't have a reference.
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.
Currently reference is necessary in query directly, but not used in combinators like
With<Relation<T>>
. It is used to determine if access is read-only or read-write. This is similar to how references are used on component types here.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.
I know this isn't exactly the time to bikeshed, but I think we should remove "CoRelation". It's too similar to "corelation", which means totally diffrent thing.
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.
IMO this is better as a separate option. "Co-relation" is the obvious term for readers with a background in advanced mathematics, even if they are a tiny minority.
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.
I mean, I don't have anything strong to say here. In fact i've made a typo there, I wanted to point out similarity to "correlation" :P If you are not convinced to remove or replace this potential name, ignore this comment.
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.
Yeah, this is the main reason I dislike CoRelation and don't think it's adequate :(. Capitalizing it like
CoRelation
is better thanCorelation
for sure, but it doesn't help enough.