Address code review: encapsulation, dedup, and stale docs#5
Merged
Conversation
- ConfigurationsModel: expose the synodic/event schedule as TReadOnlyProperty backed by a single DerivedProperty instead of manually-set writable Properties recomputed from 5 call sites; dedupe the inner/outer branch logic and the locate-event scan; replace slew/countdown sentinel fields with an explicit idle/slewing/countingDown state. - Remove the duplicate/dead PRESET_RADII constant (ConfigurationsPlanet.ts already owned this data). - CelestialBodyNode: use optionize instead of hand-rolled option defaulting; make the ModelViewTransform2 a reactive Property so it and ConfigurationsElongationIndicator redraw themselves consistently instead of the screen view mutating a shared mvt in place and manually re-syncing every consumer afterward. - PtolemaicModel: dedupe deferent/equant geometry, drop the unused pass-through sunLongitudeProperty, wrap sunAngle/anomaly with mod2pi for consistency with ConfigurationsModel. - Fix a magic-number/constant mismatch in PtolemaicPathTrail's buffer cap. - Rewrite doc/model.md and doc/implementation-notes.md, which were still the unedited TemplateSingleSim placeholder content despite both screens being complete; delete the now-obsolete root plan.md, folding its physics reference material into doc/model.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XELqGcAumaJoByJXv6g9dt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
backed by a single DerivedProperty instead of manually-set writable
Properties recomputed from 5 call sites; dedupe the inner/outer branch
logic and the locate-event scan; replace slew/countdown sentinel fields
with an explicit idle/slewing/countingDown state.
already owned this data).
make the ModelViewTransform2 a reactive Property so it and
ConfigurationsElongationIndicator redraw themselves consistently instead
of the screen view mutating a shared mvt in place and manually re-syncing
every consumer afterward.
pass-through sunLongitudeProperty, wrap sunAngle/anomaly with mod2pi for
consistency with ConfigurationsModel.
unedited TemplateSingleSim placeholder content despite both screens being
complete; delete the now-obsolete root plan.md, folding its physics
reference material into doc/model.md.
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01XELqGcAumaJoByJXv6g9dt