Skip to content
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

fix(a380x/fms): fix DH to ensure value inferior or equal 1 #9120

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

thomasshrm
Copy link
Contributor

@thomasshrm thomasshrm commented Oct 22, 2024

Partially Fixes 9114

Summary of Changes

Adding a check to ensure the value can't be superior to 1

Screenshots (if necessary)

References

https://private-user-images.githubusercontent.com/58574351/378502108-66cea740-d823-4c28-8f8b-9c4dbf7a02af.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mjk2MTMyNDUsIm5iZiI6MTcyOTYxMjk0NSwicGF0aCI6Ii81ODU3NDM1MS8zNzg1MDIxMDgtNjZjZWE3NDAtZDgyMy00YzI4LThmOGItOWM0ZGJmN2EwMmFmLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEwMjIlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMDIyVDE2MDIyNVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTlhY2Q1YTZkZWJhMzUyMDgyZDY3MjZmNDhlZDU2ODYzZTQ3YTI2ZmQxNWQ1NTYwNWVlMzZhMmNiMDk1MTA5NmEmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.CJWBl_JJON4t_9HWIkaXrvLnqJGCXKnpZxjHTTawpBI

Additional context

Discord username (if different from GitHub): tommytknuckles

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.

  1. Make sure you are signed in to GitHub
  2. Click on the Checks tab on the PR
  3. On the left side, find and click on the PR Build tab
  4. Click on either flybywire-aircraft-a320-neo or flybywire-aircraft-a380-842 download link at the bottom of the page

Trying to resolve issue flybywiresim#9114
@thomasshrm thomasshrm changed the title Fixing DH > 1 fix a380x: Fixing DH > 1 Oct 22, 2024
@thomasshrm thomasshrm changed the title fix a380x: Fixing DH > 1 fix a380x: fixing DH to ensure value can't be superior to 1 Oct 22, 2024
@thomasshrm thomasshrm changed the title fix a380x: fixing DH to ensure value can't be superior to 1 fix(a380x/fms): fixing DH to ensure value can't be superior to 1 Oct 22, 2024
@thomasshrm thomasshrm changed the title fix(a380x/fms): fixing DH to ensure value can't be superior to 1 fix(a380x/fms): fix DH to ensure value inferior or equal 1 Oct 22, 2024
@ExampleWasTaken
Copy link
Contributor

This only partially addresses #9114 if at all.

@thomasshrm the image you tried to provide is not publicly accessible.

@thomasshrm
Copy link
Contributor Author

This only partially addresses #9114 if at all.

@thomasshrm the image you tried to provide is not publicly accessible.

Hi, sorry for the mistake
It's the same image as the issue image detailing values for DH

@beheh beheh added the A380X Related to the A380X aircraft label Oct 25, 2024
@tracernz tracernz added this to the v0.13.0 milestone Oct 26, 2024
2hwk
2hwk previously approved these changes Oct 29, 2024
@2hwk 2hwk dismissed their stale review October 29, 2024 05:47

outdated

2hwk
2hwk previously approved these changes Nov 2, 2024
@2hwk 2hwk added QA Tier 1 QA A380 Only QA only for A380 required and removed Needs Code Review labels Nov 2, 2024
@2hwk
Copy link
Member

2hwk commented Nov 2, 2024

Please add testing instructions

@@ -2524,6 +2524,8 @@ export class MfdFmsPerf extends FmsPage<MfdFmsPerfProps> {
SimVar.SetSimVarValue('L:AIRLINER_DECISION_HEIGHT', 'feet', -1);
} else if (v === null) {
SimVar.SetSimVarValue('L:AIRLINER_DECISION_HEIGHT', 'feet', -2);
} else if (v > 1) {
SimVar.SetSimVarValue('L:AIRLINER_DECISION_HEIGHT', 'feet', -1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not do what was intended at all, and will actually totally break the minima.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, we must not merge the PR in the current state

@2hwk 2hwk dismissed their stale review November 2, 2024 02:26

revert

@2hwk 2hwk added Waiting For Response This issue or PR needs a response from the author and removed Do Not Merge labels Nov 2, 2024
@2hwk 2hwk removed the QA A380 Only QA only for A380 required label Nov 14, 2024
@2hwk 2hwk removed this from the v0.13.0 milestone Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A380X Related to the A380X aircraft Do Not Merge QA Tier 1 Waiting For Response This issue or PR needs a response from the author
Projects
Status: 🔴 Code Review: In progress
Development

Successfully merging this pull request may close these issues.

8 participants