StudioInfinity/dyna3
2
0
Fork 1
Code Issues 44 Pull requests Projects 2 Releases 2 Wiki Activity Actions

Generalize constraints to observables #48

Merged
glen merged 25 commits from Vectornaut/dyna3:observables_on_main into main 2025-03-10 23:43:25 +00:00
Conversation 69 Commits 25 Files changed 7 +2 -2
1 changed files with 2 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 874c823dbe - Show all commits

4
app-proto/src/assembly.rs
View file

@ -31,8 +31,8 @@ pub struct Element {
pub color: ElementColor,
pub representation: Signal<DVector<f64>>,
glen marked this conversation as resolved Outdated
glen commented 2025-02-25 23:45:02 +00:00
Outdated
Review
Copy link

Is it worth commenting that this is intended to be the set of all regulators that have this Element as a subject? Or is that entirely clear? Similarly, I think it should be noted somewhere, perhaps as a comment here, the mechanism/responsibility for maintaining that information. When a regulator is inserted into an Assembly, is it the assembly's responsibility to tell each of the subjects? I imagine that's the design, but it would be good to have it written down somewhere.

Is it worth commenting that this is intended to be the set of all regulators that have this Element as a subject? Or is that entirely clear? Similarly, I think it should be noted somewhere, perhaps as a comment here, the mechanism/responsibility for maintaining that information. When a regulator is inserted into an Assembly, is it the assembly's responsibility to tell each of the subjects? I imagine that's the design, but it would be good to have it written down somewhere.
glen commented 2025-02-26 01:31:52 +00:00
Outdated
Review
Copy link

I mean, of course by now I have gotten to that section of the code and seen that what I surmised is exactly what's happening, but I think the obligations of the various structures should be made clear not just in the implementing code in case some later coder comes along and doesn't realize that if the Assembly modifies regulators in some way, it's the Assembly's responsibility to update this information on the elements...

I mean, of course by now I have gotten to that section of the code and seen that what I surmised is exactly what's happening, but I think the obligations of the various structures should be made clear not just in the implementing code in case some later coder comes along and doesn't realize that if the Assembly modifies regulators in some way, it's the Assembly's responsibility to update this information on the elements...
Vectornaut commented 2025-02-27 08:32:14 +00:00
Outdated
Review
Copy link

I've added a comment to this effect (7cbd926). Let me know if I should make it more detailed.

I've added a comment to this effect (7cbd926). Let me know if I should make it more detailed.
glen commented 2025-03-01 01:28:49 +00:00
Outdated
Review
Copy link

Detail level was fine. I just tweaked grammar/explicitness level. Resolving.

Detail level was fine. I just tweaked grammar/explicitness level. Resolving.
// the regulators that affect this element. the ambient assembly is
// responsible for keeping this set up to date
// All regulators with this element as a subject. The assembly owning
// this element is responsible for keeping this set up to date.
pub regulators: Signal<BTreeSet<RegulatorKey>>,
// a serial number, assigned by `Element::new`, that uniquely identifies