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
Introducing model descriptions and instances #242
Introducing model descriptions and instances #242
Changes from 79 commits
ce017cb
f00141d
6167ec1
03cace2
0010b9f
243b2b4
8386209
9f30004
1215fa9
1c25265
7adb5c6
ce9f345
97f8dbf
537ff76
b3f232d
c284a64
8cf25e7
9c96e32
f6124a2
e84dc58
6d0a7e8
63fb8b9
443a3fe
ecb9bb7
5a8ef05
c896ea8
f7a24be
69ea1df
9963a8f
a944432
94ccc5c
4b413a2
d95be3b
42f25c2
4729bef
afddc82
bb13ee0
8ffcfe6
1d8aa7d
af5a2d5
c6eebf4
e2b365c
f0fa12c
ad5ba6e
d331a79
5786bae
7b01c91
d450cd7
83f3d85
de9b60e
90a60ba
acb39b9
1c2cdcf
057e186
94b54a9
6ad0ea7
8c19eed
5aed94f
3a6ad1d
cc29375
01fca32
4252c38
bbc373c
d97232d
766fd62
48c94e9
347d546
73076af
6663e1a
8cc2670
d6b643a
2fab67d
57436ed
a2e2ef5
7ef079c
5e5e16d
eb49604
4816d02
1bb13d0
812c183
a850590
5ce02a1
661ecb9
f527790
8c09224
c8e1fb2
19483c4
1478654
6067fef
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
In the current setup, we iterate over
&mut model_instances
which does not have this filter, meaning it will include all model instances in the world every update cycle. Moreover, this query will only include model descriptions whoseHovered
orSelected
status has changed during the latest cycle, so this line will not fetch anything if neither of these components changed for the model description.I have a feeling this filter was meant to go on
model_instances
, not onmodel_descriptions
.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.
My understanding of this implementation is to have the model instances highlighted if their affiliated description is selected/hovered over, rather than the other way round. The current app supports that, such that a change in selection/hovering for a description (queried with the
Changed<>
filter) will trigger an update to all affiliated instances being highlighted (query of all instances in each cycle).From what I see this behavior also allows relevant task locations to be highlighted when we iterate over every model.
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.
You're absolutely right, I forgot that I had specifically asked Reuben to implement this so that hovering and selecting the group widget would have this effect.
I might suggest renaming the function to
update_visual_cues_for_model_group
to make that purpose more obvious, but it's a minor concern.