-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat(a380x/mfd): Add DATA/WAYPOINT page #9786
base: master
Are you sure you want to change the base?
feat(a380x/mfd): Add DATA/WAYPOINT page #9786
Conversation
getAllStoredWaypoints(): PilotWaypoint[] { | ||
if (this.storedWaypoints === undefined) { | ||
return []; | ||
} else { | ||
return this.storedWaypoints; | ||
} | ||
} |
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.
getAllStoredWaypoints(): PilotWaypoint[] { | |
if (this.storedWaypoints === undefined) { | |
return []; | |
} else { | |
return this.storedWaypoints; | |
} | |
} | |
getAllStoredWaypoints(): PilotWaypoint[] { | |
return this.storedWaypoints ?? []; | |
} |
Could be better to just return undefined though rather than allocating an array every call when it's empty.
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.
Hmmm, I dislike returning undefined for elements like this. This shouldn't be called that often so I feel like the allocation is not going to be that bad (this may be C/Rust brain thinking where an allocation for that slice would be tiny)
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.
Then why not have this.storedWaypoints always an array instead? If this is being called every time the page updates it's allocating quite a lot of unneeded objects across a flight. It's death by a thousand cuts with GC pauses = microstutter.
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.
Then why not have this.storedWaypoints always an array instead
Just checked, it already is (Line 61). So this function doesn't need any nullish coaliescing at all, it can just return the array
Summary of Changes
Add the waypoint page under the data header
Screenshots (if necessary)
References
Additional context
Discord username (if different from GitHub):
Testing instructions
How to download the PR for QA
Every new commit to this PR will cause new A32NX and A380X artifacts to be created, built, and uploaded.