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: revert cockpit decal textures, add speed placard decal #3571

Merged
merged 10 commits into from Feb 21, 2021

Conversation

ImenesFBW
Copy link
Contributor

Summary of Changes

  • Updated cockpit text emissive and albedo. Added parenthesis to the albedo and reverted the outdated decals that were merged with the texture rework.
  • Reenabled the speed placard decal.

No changelog included since this is mostly a revert of changes that were already committed.

Screenshots

ere

References

This PR reintroduces the changes made with previous cockpit decal PRs that were removed when the texture rework from @MoreRightRudder got merged.

It also adds the speed placard decal that was blocked before since ours was redone in textures. This is a temporary change as @DarkOfNova will be reworking the speed placard and making it a decal. There are small issues with placard right now. If you zoom in you can see L overlaps the screw and the numbers and letters aren't perfect since this is using the default decal layout. But as I said this will shortly be changed to a separate decal.

Additional context

Closes issues caused by #3431.

Discord username: Imenes#8739

Testing instructions

Besides the speed placard, make sure you check that all the decals (qnh units, throttle detent markers, landing gear arrow,...etc) that went missing with the texture rework are back in place and are up to date to textures before the rework was merged..

How to download the PR for QA

Every new commit to this PR will cause a new A32NX artifact to be created and uploaded.
The build script will have already been run with the latest changes, so no need to rerun it once you download the zip.

  1. Make sure you are signed in to GitHub
  2. Click on the Checks tab on the PR
  3. On the right side, click on the Artifacts drop down and click the A32NX link

Copy link
Member

@tyler58546 tyler58546 left a comment

Choose a reason for hiding this comment

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

LGTM

@NonstopFlying
Copy link

Category: QA Trial
Name: NonstopFlying #6771
Date of testing: Current date 17/02/2021
Version of the sim: 1.13.16.0
PR Tested: #3571
Tier of testing: 1
Changes to observe:

revert of cockpit decals and textures that were removed from via the update

Testing technique:
Spawned in at Leeds and looked at the different decals
Steps you've made to test the Pull Request:

  1. Spawned at Leeds
  2. Looked at Throttle
  3. Placard
  4. Throttle
  5. QNH and other decals

Media:
placard
Throttle
FMC

Overall Rating: Good/To be improved
Good but like you said just need to replace that L so it's not overlapping the screw, other than that it's good

@sidnov
Copy link
Member

sidnov commented Feb 17, 2021

Category: QA Tester
Name: sidnov#8337
Date of testing: 17.2.2021
Version of the sim: 1.13.16.0
PR Tested: #3571
Tier of testing: 1

Changes to observe: revert cockpit decal tex, speed placard decal

Testing technique: Tryout

Steps you've made to test the Pull Request:

  1. Spawned on runway at VABB
  2. Checked the speed placard
  3. Checked Throttle indent
  4. QNH and other decals

Media:

image
image
image
image

Overall Rating: Good

Conclusions of the testing: The mentioned decals were checked. Good to go!

@marcman86
Copy link
Contributor

Name: marcman86#4907
Date of testing: 17.2.2021
Version of the sim: 1.13.16.0
PR Tested: #3571
Tier of testing: 1
Changes to observe: Decals are back
Testing technique: Tryout

Steps made to test the Pull Request:

1.) Spawned C&D at EDDP

Decals visible:

  • Speedplate
  • Throttle detent
  • QNH
  • Alt knob increment pin
  • Blue panel markers
  • chair adjustment knobs

Issues (if related to this PR): Blue panel markers are missing

Overall Rating: -

Conclusions of the testing: When the blue markers are no (missing) decals, the PR is fine.

Hope this helps :-)

@ImenesFBW
Copy link
Contributor Author

The L overlap and blue markings are out of the scope of this PR, since they will be added in as separate decals by another PR in the future by @tyler58546 and @DarkOfNova

@ChruutvoLuzi
Copy link
Contributor

ChruutvoLuzi commented Feb 17, 2021

Category: QA Tester
Name: ChruutvoLuzi | SD-PB.ch#8902
Date of testing: 17.2.2021
Version of the sim: 1.13.16.0
PR Tested: #3571
Tier of testing: n/a
Changes to observe: reimplement decals

Testing technique: TryOut
Steps you've made to test the Pull Request:

  1. Spawned at EGGP
  2. Turned on the Light
  3. Checked Speed Placard
  4. Checked FCU
  5. Checked gear arrow
  6. checked throttle

Media:
Microsoft Flight Simulator Screenshot 2021 02 17 - 20 51 00 18
Microsoft Flight Simulator Screenshot 2021 02 17 - 20 51 30 69
Microsoft Flight Simulator Screenshot 2021 02 17 - 21 03 40 17

Issues (If Any): none You can see the units shimmering slightly on the chronometer but not when looking at them

Overall Rating: very good To be improved
Conclusions of the testing: adds alot more to the already great cockpit. no issues found apart the known ones EDIT: i found, while looking at the screenshots above, that the units on the chronometer are slightly shining through. If thats not an issue of this PR, ignore. Otherwise, everything looks good

(flickering)

@ImenesFBW
Copy link
Contributor Author

Taking a look at what is happening with the chrono.

@ImenesFBW
Copy link
Contributor Author

ImenesFBW commented Feb 17, 2021

@ChruutvoLuzi can you take a look at the same conditions in the development version. I seem to see problems there as well. I think it might be the metalness or roughness of the new texture that is overlaping it.

image

EDIT:

Yeah seems like the issue is not introduced by this PR should be the same in dev, we might need to create an issue for this so Mike can sort it out with the chrono textures when someone else confirms it is the same in dev.

Another pic from dev:

image

@hotshotp hotshotp added Testing Started Testing for this PR has started and tweaking / fixing is underway and removed Ready to Test labels Feb 18, 2021
@ImenesFBW
Copy link
Contributor Author

This is good to merge in my opinion. All the QA problems found are either not related to this PR (chrono) or are beyond the scope of this PR (blue markings and overlapping L) and will be added in a separate PR.

@kberg93
Copy link

kberg93 commented Feb 19, 2021

Getting CTD with this PR installed, no issue with other PR's or without! (just a heads up)
Tried reinstalling with 3 different downloads now, all CTD before main menu under loading.

and also a ref for the "L" mentioned on QAs above:
image
(i saw Imenes comment now but ill leave pic for later if wanted)

@kberg93
Copy link

kberg93 commented Feb 19, 2021

Now it doesnt CTD before main menu but it doesnt show up.

FlightSimulator_OBGh7EiMq8
FlightSimulator_gyAoYsmAHq

@marcman86
Copy link
Contributor

Tested: CTD while MSFS loading screen for me.

@ImenesFBW
Copy link
Contributor Author

@kberg93 and @marcman86 thanks for notifying me!

There was an issue with the rebase that should now be solved. If you get the time please redownload and give it another try.

@kberg93
Copy link

kberg93 commented Feb 19, 2021

@kberg93 and @marcman86 thanks for notifying me!

There was an issue with the rebase that should now be solved. If you get the time please redownload and give it another try.

Will do tomorrow if it isnt merged yet. 😄

@marcman86
Copy link
Contributor

Name: marcman86#4907
Date of testing: 20.2.2021
Version of the sim: 1.13.16.0
PR Tested: #3571
Tier of testing: 1
Changes to observe: Decals everywhere
Testing technique: Tryout

Steps made to test the Pull Request:

1.) Spawned C&D to EDDP 08R
2.) Decals work
3.) Clock with daylight (Image1)
4.) Clock on flashlight (Image2)
5.) Clock on int. light (Image3)

Media:

85
Image1

86
Image2

87
Image3

Issues: None (when clock is to another PR)

Overall Rating: Good!

Conclusions of the testing: Works. Decals like before in my first test.

Hope this helps :-)

@kberg93
Copy link

kberg93 commented Feb 20, 2021

Category: QA Tester
Name: Kberg#0666
Date of testing: Current date 02/20/2021
Version of the sim: 1.13.16.0
PR Tested: #3571
Tier of testing: 1
Changes to observe:
Revert cockpit decal textures, add speed placard decal.

Testing technique: Look around.

Steps you've made to test the Pull Request:

  1. Spawn at airport.
  2. Look around the cockpit.

Flight notes (When done a full flight): Parked.

Media: Insert screenshots, recordings, or anything relevant to the review here.

Issues (If Any): None related to this PR.

Overall Rating: Good.
Conclusions of the testing: All decals i know is visible, and this PR does whats intended.

@tyler58546 tyler58546 added Tested and removed Testing Started Testing for this PR has started and tweaking / fixing is underway labels Feb 21, 2021
@tyler58546 tyler58546 merged commit 9cebcdd into flybywiresim:master Feb 21, 2021
@Watsi01
Copy link
Contributor

Watsi01 commented Feb 22, 2021

Just for me: why is this reverted? Cause it worked for me. At least I didn't find any issues.

@ImenesFBW
Copy link
Contributor Author

Just for me: why is this reverted? Cause it worked for me. At least I didn't find any issues.

It was merged not reverted.

@Watsi01
Copy link
Contributor

Watsi01 commented Feb 22, 2021

Thanks. It sounded to me that this is a revert.

@ImenesFBW
Copy link
Contributor Author

Thanks. It sounded to me that this is a revert.

No worries the revert in the title is meant as reverting the older cockpit decals that were merged with the cockpit texture rework. That one was merged with the older decals so it took away some fixes that we made to decals. Hope that makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet