search

c172p-team / c172p

A high detailed version of the Cessna 172P aircraft for FlightGear
GNU General Public License v2.0
82 stars 43 forks source link

Exiting sim saves persistent variant data which loads on next startup regardless of variant selected #1435

Closed wlbragg closed 1 year ago

wlbragg commented 1 year ago

@wlbragg I got this several times at launching (10 or more, with some changes in the method). The aircraft appears like this (for the first one, I guess that I checked "Spawn at seaport if available" after the sim start) c172p-bug-launch2 c172-bug-amphib-launch3

At least one way to get it: ("Save state of aircraft between sessions" is unchecked)

The 3D model is the default one in accordance with --aircraft=c172p, but the FDM contact reference is the amphibious one. As displayed in the GUI.

"Go Now" seems to be the way to repeatedly get the bug.

Not blocking, can easily be recovered (in the sim) by selecting "Default" if above ground, or by selecting "Float" if above water.

Akin to this observation (this message) @dany93 wrote

Once, from ground to ground relocation the amphib floats appeared in a first step drowned in the ground, with "Default" option checked in the GUI. However, back to correct after selecting the Amphib option in the GUI.

wlbragg commented 1 year ago

The issue is in mooring.nas and the persistent data system.

It is used for a couple different user selected actions.

It has a listener that fires on /sim/signals/fdm-initialized to check for controls/mooring/automatic and controls/mooring/allowed. If those flags are set to true the aircraft is configured for water landings and takeoffs, which means the property fdm/jsbsim/bushkit is set to either 3 or 4 and there is an associated water port to the startup ICAO. If this is all true the fdm/jsbsim/bushkit property is cached (aircraft.data.add("/fdm/jsbsim/bushkit");) and the fgcommand("reposition") command is issued so the aircraft restarts and you are started at the water port.

When using the Go Now button the exact same conditions are met and the same actions are taken.

The problem is arising because in both cases the property fdm/jsbsim/bushkit is cashed by the mooring.nas code. On the next load if you select any variant other than the last session, the cached data fdm/jsbsim/bushkit overwrites the current variant the user selected and the variant set files instruction. So the variant loading sets up for one configuration and the cached data is set for something else. It's causing conflicting instructions.

~There is no easy answer for this~. If you allow the saved fdm/jsbsim/bushkit to determine the loading then the variant you select means nothing. But if you select a variant, currently in the above situations, the cached data is overriding the users intent.

wlbragg commented 1 year ago

@dany93 OK, I think I got this fixed. It appears it was fairly easy to correct the issue. On the first test using the controls/mooring/automatic flag set to true, the startup failed to go to the water port and left me sitting on a paved runway on broken floats. I think it was a fluke, maybe cache needed to be cleared. After that one misfire it worked correctly every time.

PR https://github.com/c172p-team/c172p/pull/1436

dany93 commented 1 year ago

@wlbragg Partially fixed, correct for this procedure (= my last one, previously giving the bug)

  • Launch with --aircraft=c172p
  • Select amphibious,
  • "Go Now"
  • Quit
  • Again, Launch with --aircraft=c172p

But with this procedure (Including "Go to airport"):

"Go to Airport": done from KSFO (water) to another airport (LFOH) or back to KSFO (tarmac). Gives amphib on the tarmac partially drown in the ground.

c172-bug-gotoairport

wlbragg commented 1 year ago

I didn't test that, this may be more difficult.

wlbragg commented 1 year ago

@dany93

OK, I think this is pretty well solved. The exception is the fg1000 variants. They actually work the same way and are repositioning and reinit correctly, but the frame rate appeared to drop drastically on either a reinit or reposition of the fg1000, regardless of the undercarriage. I think maybe it is not clearing the original instance of the fg1000 and thus loading it 1, 2, or more times, each time you reinit/reposition it is duplicating itself. Maybe you can verify that noted behaviour. I only tried it a couple times and that was the impression I got.

So if you have a chance you can test all you did before and verify that those parts are corrected. In the meantime I will take a look and see if there is a way to clear out the existing fg1000 system if a reinit/reposition occurs. If indeed that is what is going on.

dany93 commented 1 year ago

@wlbragg wrote

So if you have a chance you can test all you did before and verify that those parts are corrected

Tested. My procedure above with "Go to Airport" works (the aircraft appears on the tarmac, correct on its wheels with the amphib). Otherwise, the procedure with "Go Now", "Quit" and launch again (which worked) still works.

I will test the FG1000 with relocation.

Done and confirmed:

No strong effect on RAM (6 to 6.4 GB). But strong effect on CPU usage at each relocation (60%, then 85%, then 100% on one of the CPU threads). Nasal code?

Please tell me for the "merge". I think this is enough for the current issue (unless we find something else...). The FG1000 is another subject.

wlbragg commented 1 year ago

@dany93

I found the issue. It is the fix for the original fg1000 cache issue. The delay listener, "setlistener("/sim/signals/fdm-initialized", func {" is duplicating the electrical of fg1000 every time there is a re-init/reload, or something like that, as far as I can tell. I'm going to have to change it.

FYI: the c182t does not exhibit the same breakage on a re-init (go to airport) at the c172sp. So I should be able to see how they configured the fg1000 load order timing and electrical so as not to cause the issue we had.

Would it be appropriate to reopen the https://github.com/c172p-team/c172p/issues/1421 issue to fix this fg1000 problem and go ahead and merge this work?

dany93 commented 1 year ago

@wlbragg I think that merging this work under this issue title would be better. It is solved, it was not a FG1000 issue.

After that, either reopening the https://github.com/c172p-team/c172p/issues/1421 issue or opening a new issue. At your preference, if you feel the work to do now can go under the https://github.com/c172p-team/c172p/issues/1421 title. [EDIT] On second thought, reopening the https://github.com/c172p-team/c172p/issues/1421 issue seems sensible.

wlbragg commented 1 year ago

I'm going to create a new issue and PR for the fg1000. I think I already fixed it. I think it will be cleaner to do it as a new issue. The https://github.com/c172p-team/c172p/issues/1421 thread was quite long and complex.

I'll have it posted and the PR pushed shortly after I test myself. I can't be confident that I get the old cache bug as sometimes I didn't have it. So you will need to verify that I didn't reintroduce the old issue.

wlbragg commented 1 year ago

@dany93

In case of any confusion, I am done with all of it. Both PR's should be ready to go. Push order would be the PR associated with this issue https://github.com/c172p-team/c172p/pull/1436 then https://github.com/c172p-team/c172p/pull/1438

wlbragg commented 1 year ago

This should have closed automatically, I formatted the auto-close on PR #1436 incorrectly.

It reads Fixes issue #1435 Should have read Fixes #1435