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

refactor: aircraft type check #8500

Closed
wants to merge 8 commits into from
Closed

Conversation

Revyn112
Copy link
Contributor

Fixes #8499

Summary of Changes

This pull request introduces an AircraftType enum to the common module, fundamentally altering the mechanism for aircraft type verification. Previously, this process depended on matching the SimVar Title, a method that proved unreliable due to its reliance on specific search terms which needs to be added by the Livery Creator. This could result in the incorrect loading of build_info.json. With the proposed changes, upon reaching the Ready state, a new SimVar, "L:A32NX_AIRCRAFT_TYPE", will be assigned a value from the AircraftType enum corresponding to the A32NX. This will enable a more reliable comparison against the enum values to determine the appropriate build_info.json prefix (e.g., a32nx, a380x). Furthermore, this PR ensures consistency across the codebase by replacing static string comparisons for airframe checks with enum comparisons. This approach not only improves the current implementation but also allows for the easy integration of additional airframes in the future.

Screenshots (if necessary)

References

Additional context

Discord username (if different from GitHub):

Testing instructions

  1. Load up the aircraft.
  2. Check if the EFB Payload Page work and shows the A32NX Layout.
  3. Check if the EFB Service Page work and shows the A32NX Layout.
  4. Check if the EFB Flight Widget show the correct A32NX Information.
  5. Check if the EFB Overview Page show the correct A32NX Information.
  6. Check if the Throttle Config for the A32NX is correct and working.

How to download the PR for QA

Every new commit to this PR will cause a new A32NX artifact 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, click on the bottom PR tab
  4. Click on the A32NX download link at the bottom of the page

@Revyn112
Copy link
Contributor Author

Revyn112 commented Feb 19, 2024

still needs a decision if the lvar should be set based on the url param 'airframe' or be hard-coded directly in the extra-host module.

Copy link
Member

@frankkopp frankkopp left a comment

Choose a reason for hiding this comment

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

I like this solution better - but still the overall question how the team wants to have this done is still open

} else {
break;
case AircraftType.Unknown:
default:
aircraft = 'other';
Copy link
Member

Choose a reason for hiding this comment

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

I personally would add a console warning as this should not happen in normal circumstances.

@@ -149,7 +149,7 @@ pixel_size = 0,0
texture = NO_TEXTURE
background_color = 0,0,0

htmlgauge00 = A32NX/ExtrasHost/extras-host.html,0,0,1,1
htmlgauge00 = A32NX/ExtrasHost/extras-host.html?Airframe=A320_251N,0,0,1,1
Copy link
Member

Choose a reason for hiding this comment

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

Can't find the A380X panel.cfg change - you would also have to do this there.

@2hwk
Copy link
Member

2hwk commented Mar 14, 2024

Note that is a bit stale but not forgotten, we'll be getting back to this after fsweekend

EDIT: For the build_info.json prefix, I think it would make more sense to define this in the .env, this makes sense for this since the build_info.json is generated at build time, so I don't see why this should be controlled via a simvar whether this value is a string or an enum. Unless there is something I am missing entirely.

@2hwk
Copy link
Member

2hwk commented Mar 19, 2024

Panel.xml documentation:

Panel.xml docs:
https://docs.flightsimulator.com/html/Content_Configuration/SimObjects/Aircraft_SimO/Instruments/panel_xml.htm
Loaded into baseinstrument here:
https://github.com/microsoft/msfs-avionics-mirror/blob/9d8f57ad0ace02a59eab55907bd711e3912806eb/src/msfstypes/pages/vcockpit/instruments/shared/baseinstrument.d.ts#L13

See F/A-18 behaviors/model package for code sample

This is for my own reference, as we will be investigating if we can replace panel.cfg with a panel.xml solution.

EDIT: For now we will stick to panel.cfg and transition to defining this prefix as an environment variable (.env).

@2hwk
Copy link
Member

2hwk commented Mar 25, 2024

@Revyn112 I have merged the repo's remote master branch into my local branch, but as your source branch of this PR is your fork's master branch, I don't wish to force push to your master branch.

Let me know if you would like to cherry pick this change, check out commit 735e79ef from my fork (2c-pr-8500-cont)

I'm be using this as the basis for any future rewrite, stay tuned.

@2hwk 2hwk added the Waiting For Response This issue or PR needs a response from the author label Mar 31, 2024
@2hwk
Copy link
Member

2hwk commented Mar 31, 2024

Still waiting for response from PR owner, if not as this is quite a critical issue for everyone, I will be opening a new PR that is sourced from my 2c-pr-8500-cont branch on my fork. (I do not wish to force push to master on @Revyn112 's branch)

@2hwk
Copy link
Member

2hwk commented Apr 6, 2024

Closing in favour of #8599 which continues on from the last commit.

This should also fix 8499 and result in an easier time to deconflict in the VFS, build_info.json as well as helping with other flypad related configuration.

Note that the AIRCRAFT_PROJECT_PREFIX and AIRCRAFT_VARIANT environ will be mandatory going forwards.

@2hwk 2hwk closed this Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Code Review Waiting For Response This issue or PR needs a response from the author Waiting on Core Decision
Projects
Status: ❌ Cancelled/Unfinished
Development

Successfully merging this pull request may close these issues.

Build_Info.json not loaded within EFB because of non-reliable Aircraft Check
4 participants