Add an optimizer library to score tracks #203
Conversation
christophe-taret-zenika
force-pushed
the
feat/use-optimizer-in-xc-planning
branch
2 times, most recently
from
577a05e
to
3e47dae
Compare
christophe-taret-zenika
force-pushed
the
feat/use-optimizer-in-xc-planning
branch
from
87d7be2
to
94322bb
Compare
|
Tell me if it's ok for you. |
Yep the best is to pause worked based on this until it gets merged. I should be able to review tomorrow. Thanks a lot! |
|
@vicb do you have any idea why the tests fails on optimizer lib? I can not reproduce the problem locally (a nicer way to say WOMM ;-) ) |
christophe-taret-zenika
force-pushed
the
feat/use-optimizer-in-xc-planning
branch
4 times, most recently
from
aa443ca
to
a7d7eb2
Compare
vicb
changed the title
christophe-taret-zenika
force-pushed
the
feat/use-optimizer-in-xc-planning
branch
from
c09e791
to
46e2fd0
Compare
|
The issue with the hack for 3d is:
So I'm -1 on using this commit. |
I agree that this hack, as almost every hack, has drawbacks. It permit to have something that works for the user vs an application that can't ne used.
|
Switching to 3D mode does not reload However when It turns out that the changes done by What your hack does is that it fully reload the page (instead of just swapping the 2D component for the 3D component). In this case the
It's a problem with any library polluting the global namespace.
|
vicb
force-pushed
the
feat/use-optimizer-in-xc-planning
branch
from
46e2fd0
to
ba76ae2
Compare
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 95 files out of 152 files are above the max files limit of 50. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughThe recent changes encompass a wide range of updates, including refactoring scoring logic, introducing new methods and types, updating configurations, and enhancing documentation. Key updates include modifications to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Outside diff range and nitpick comments (4)
apps/fxc-front/src/app/redux/planner-slice.ts (1)
Line range hint
11-25: The use ofLeagueCodefor theleagueproperty enhances type safety. Consider using strict equality for comparisons.- if (route.length == 0) { + if (route.length === 0) {Tools
Biome
[error] 3-4: All these imports are only used as types.
[error] 4-5: All these imports are only used as types.
apps/fxc-front/src/app/components/ui/about-modal.ts (1)
Line range hint
75-75: Consider specifying a type for the window object to improve type safety when accessing custom properties.- (window as any).klaro?.show(); + (window as { klaro: { show: () => void } }).klaro?.show();apps/fxc-front/src/app/components/2d/path-element.ts (2)
Line range hint
110-110: Replace==with===to avoid type coercion issues.- if (this.encodedRoute.length == 0) { + if (this.encodedRoute.length === 0) {Using
===ensures that both the type and value are checked, which is safer and prevents potential bugs due to type coercion.Also applies to: 175-175, 206-206, 208-208, 242-242, 276-276
Tools
Biome
[error] 17-18: Some named imports are only used as types.
[error] 18-19: All these imports are only used as types.
Line range hint
195-267: Optimize theoptimizeandcomputeScoremethods for better performance and clarity.Consider refactoring these methods to separate concerns more clearly and improve readability. For instance, breaking down the
optimizemethod into smaller, more focused methods could make the code easier to understand and maintain.Tools
Biome
[error] 276-276: Use === instead of ==.
== is only allowed when comparing againstnull
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (29)
- CONTRIBUTING.md (1 hunks)
- apps/fxc-front/src/app/components/2d/path-element.ts (6 hunks)
- apps/fxc-front/src/app/components/2d/planner-element.ts (1 hunks)
- apps/fxc-front/src/app/components/ui/about-modal.ts (1 hunks)
- apps/fxc-front/src/app/components/ui/main-menu.ts (1 hunks)
- apps/fxc-front/src/app/components/ui/pref-modal.ts (2 hunks)
- apps/fxc-front/src/app/logic/score/league/leagues.ts (1 hunks)
- apps/fxc-front/src/app/logic/score/scorer.ts (1 hunks)
- apps/fxc-front/src/app/redux/planner-slice.ts (3 hunks)
- apps/fxc-front/vite.config.ts (1 hunks)
- docker/.env (1 hunks)
- docker/docker-compose.yml (1 hunks)
- libs/optimizer/.babelrc (1 hunks)
- libs/optimizer/.eslintrc.json (1 hunks)
- libs/optimizer/README.md (1 hunks)
- libs/optimizer/jest.config.ts (1 hunks)
- libs/optimizer/project.json (1 hunks)
- libs/optimizer/src/index.ts (1 hunks)
- libs/optimizer/src/lib/optimizer.spec.ts (1 hunks)
- libs/optimizer/src/lib/optimizer.ts (1 hunks)
- libs/optimizer/src/lib/scoring-rules.ts (1 hunks)
- libs/optimizer/src/lib/utils/create-segments.ts (1 hunks)
- libs/optimizer/src/lib/utils/merge-tracks.ts (1 hunks)
- libs/optimizer/tsconfig.json (1 hunks)
- libs/optimizer/tsconfig.lib.json (1 hunks)
- libs/optimizer/tsconfig.spec.json (1 hunks)
- nx.json (1 hunks)
- package.json (4 hunks)
- tsconfig.base.json (1 hunks)
Files skipped from review due to trivial changes (11)
- docker/.env
- docker/docker-compose.yml
- libs/optimizer/.babelrc
- libs/optimizer/.eslintrc.json
- libs/optimizer/jest.config.ts
- libs/optimizer/project.json
- libs/optimizer/tsconfig.json
- libs/optimizer/tsconfig.lib.json
- libs/optimizer/tsconfig.spec.json
- package.json
- tsconfig.base.json
Additional context used
Biome
libs/optimizer/src/lib/utils/merge-tracks.ts
[error] 15-15: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 1-1: All these imports are only used as types.
libs/optimizer/src/lib/utils/create-segments.ts
[error] 1-1: All these imports are only used as types.
apps/fxc-front/src/app/logic/score/league/leagues.ts
[error] 1-1: All these imports are only used as types.
apps/fxc-front/src/app/redux/planner-slice.ts
[error] 60-60: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 1-1: Some named imports are only used as types.
[error] 3-4: All these imports are only used as types.
[error] 4-5: All these imports are only used as types.
apps/fxc-front/vite.config.ts
[error] 11-11: Unexpected any. Specify a different type.
apps/fxc-front/src/app/components/ui/about-modal.ts
[error] 75-75: Unexpected any. Specify a different type.
[error] 1-1: Some named imports are only used as types.
libs/optimizer/src/lib/scoring-rules.ts
[error] 21-21: The computed expression can be simplified without the use of a string literal.
[error] 105-105: The computed expression can be simplified without the use of a string literal.
[error] 111-111: The computed expression can be simplified without the use of a string literal.
apps/fxc-front/src/app/components/ui/pref-modal.ts
[error] 24-26: Prefer for...of instead of forEach.
[error] 1-1: Some named imports are only used as types.
[error] 8-9: Some named imports are only used as types.
libs/optimizer/src/lib/optimizer.ts
[error] 91-91: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 130-130: Forbidden non-null assertion.
[error] 146-146: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 170-170: Unexpected any. Specify a different type.
[error] 201-201: Unexpected any. Specify a different type.
[error] 208-208: Unexpected any. Specify a different type.
[error] 257-257: Forbidden non-null assertion.
[error] 2-3: Some named imports are only used as types.
[error] 3-4: All these imports are only used as types.
[error] 4-5: Some named imports are only used as types.
[error] 122-122: Reassigning a function parameter is confusing.
apps/fxc-front/src/app/components/2d/planner-element.ts
[error] 208-208: The assignment should not be in an expression.
[error] 220-220: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 1-1: Some named imports are only used as types.
[error] 6-7: All these imports are only used as types.
[error] 9-10: Some named imports are only used as types.
apps/fxc-front/src/app/components/2d/path-element.ts
[error] 80-82: Prefer for...of instead of forEach.
[error] 110-110: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 116-118: Prefer for...of instead of forEach.
[error] 175-175: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 206-206: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 206-206: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 208-208: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 242-242: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 242-242: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 276-276: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 314-314: The assignment should not be in an expression.
[error] 3-4: All these imports are only used as types.
[error] 4-5: Some named imports are only used as types.
[error] 17-18: Some named imports are only used as types.
[error] 18-19: All these imports are only used as types.
libs/optimizer/src/lib/optimizer.spec.ts
[error] 8-180: Prefer for...of instead of forEach.
[error] 23-178: Prefer for...of instead of forEach.
[error] 1-1: Some named imports are only used as types.
[error] 1-2: Some named imports are only used as types.
apps/fxc-front/src/app/components/ui/main-menu.ts
[error] 255-255: Prefer for...of instead of forEach.
[error] 306-306: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 557-557: Unexpected any. Specify a different type.
[error] 564-564: Unexpected any. Specify a different type.
[error] 653-653: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 683-683: Use === instead of ==.
== is only allowed when comparing againstnull
[error] 1-2: Some named imports are only used as types.
[error] 2-3: Some named imports are only used as types.
[error] 6-7: All these imports are only used as types.
[error] 12-13: Some named imports are only used as types.
[error] 16-25: Some named imports are only used as types.
[error] 28-29: Some named imports are only used as types.
LanguageTool
libs/optimizer/README.md
[uncategorized] ~9-~9: Possible missing article found.
Context: ...ptimizer.ts#optimizefunction computes score of a given track given by aScoringTra...
[grammar] ~9-~9: Did you mean the possessive pronoun “its”?
Context: ...oringTrackfor a given league known by it'sLeagueCode. Theoptimize` function ...
[uncategorized] ~13-~13: “an” (indefinite article before a vowel sound) seems less likely than “and” (in addition to, following this).
Context: ...equestdescribing containing the track an some options. It returns anIterator<...CONTRIBUTING.md
[grammar] ~15-~15: There seems to be a noun/verb agreement error. Did you mean “adds” or “added”?
Context: ...## Project setup - runnpm install- add default keys definitions - `cp apps/f...
Markdownlint
CONTRIBUTING.md
21-21: null
Emphasis used instead of a heading
25-25: null
Emphasis used instead of a heading
29-29: null
Emphasis used instead of a heading
Additional comments not posted (24)
libs/optimizer/src/index.ts (3)
1-1: Export ofgetOptimizerandCircuitTypelooks good.
2-2: Type exports are correctly defined and useful for external integrations.
3-3: Export of scoring rules related types and constants is well-implemented.apps/fxc-front/src/app/logic/score/scorer.ts (3)
1-1: Import ofCircuitTypeis correctly utilized in theScoreclass.
4-8: Property definitions in theScoreclass are clear and appropriately typed.
12-17: Constructor implementation is robust, using modern JavaScript features to handle defaults effectively.apps/fxc-front/src/app/logic/score/league/leagues.ts (1)
3-46: League definitions and utility functions are well-implemented and provide a robust way to handle different leagues and their scoring rules.CONTRIBUTING.md (3)
3-10: The list of required tools is comprehensive and well-detailed.
33-46: The installation instructions for the Datastore emulator are clear and well-detailed.
49-51: The provided commands are useful for developers to ensure code quality before PR submission.nx.json (1)
25-29: The configuration for TypeScript under "@nx/js:tsc" is correctly set up and consistent with other configurations.apps/fxc-front/src/app/components/ui/about-modal.ts (1)
23-24: The addition of new contributors is correctly implemented.apps/fxc-front/src/app/components/ui/pref-modal.ts (1)
11-11: LGTM! Import change aligns with the PR's objectives to handle league configurations.libs/optimizer/src/lib/optimizer.ts (1)
1-1: Good reminder to replace console logs with a proper logging library in the future.apps/fxc-front/src/app/components/2d/planner-element.ts (1)
142-142: LGTM! UsingdistanceMaligns with the PR's objectives to handle units more accurately.apps/fxc-front/src/app/components/2d/path-element.ts (1)
48-48: Ensure consistent use of state management.The use of
@state()forleagueis consistent with other state properties in this component, which is good for maintainability.apps/fxc-front/src/app/components/ui/main-menu.ts (8)
Line range hint
1-1: The class structure and logic withinMainMenuappear well-organized and maintainable.
Line range hint
1-1: TheAirspaceItemsclass is well-implemented with appropriate use of LitElement's directives for conditional rendering.
Line range hint
1-1: TheSkywaysItemsclass is well-implemented with appropriate use of LitElement's directives for conditional rendering.
Line range hint
1-1: TheViewItemsclass is well-implemented with appropriate use of LitElement's directives for conditional rendering.
Line range hint
1-1: TheFullScreenItemsclass is well-implemented with appropriate use of LitElement's directives for conditional rendering.
Line range hint
1-1: TheTrackItemsclass is well-implemented with appropriate use of LitElement's directives for conditional rendering.
Line range hint
1-1: TheLiveTrackItemsclass is well-implemented with appropriate use of LitElement's directives for conditional rendering.
Line range hint
1-1: Verify the impact ofmaybeHideSidePaneon other parts of the application.
|
I have pushed more changes to the PR.
I have also added more info to the Hopefully we can merge this tomorrow! |
|
@coderabbitai generate interesting stats about this repository and render them as a table. |
|
Tip For best results, initiate chat on the files or code changes. @vicb, here are some interesting statistics about the repository:
These metrics provide a snapshot of the repository's current state, including contributions, codebase size, and ongoing issues. Analysis chainScripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Count the number of authors in the codebase.
git shortlog -sn --all | wc -l
# Count the total number of commits.
git rev-list --all --count
# Count the number of lines of code in the codebase.
tokei --output json
# Count the number of open issues using GitHub CLI.
gh issue list --repo vicb/flyxc --state open --json number --jq 'length'
Length of output: 76545 |
Summary by CodeRabbit
New Features
flyingtofin the About modal.computeScoremethod for optimized route scoring.optimizefunction in the optimizer library for enhanced performance.Bug Fixes
PlannerElementfromthis.score.distancetothis.score.distanceM.Refactor
PathElementfor better readability and efficiency.handleUploadmethod for clarity.Documentation
CONTRIBUTING.md.Chores
package.jsonto their latest versions.