Repairing flag set to true during "Go to Airport" relocation

[wlbragg]

There is a bug where the flag /fdm/jsbsim/damage/repairing is set to true during a GUI instigated airport relocation.
What that does is set the property /controls/gear/brake-parking to true. This requires a cycling of the "Repair" function or bush gear change or manually setting the repairing flag to false.
The function that is responsible for repairing flag to be set to true is in Systems/ground-effects.xml

<logic>
        <name>Parking Brake And Chock</name>
        <input>
            <or>
                <!-- The actual parking brake controlled by the pilot -->
                <property>/sim/model/c172p/brake-parking</property>

                <property>/sim/model/c172p/securing/chock-visible</property>

                <!-- When changing the bushkit, hold position so that new
                     initial length of tiedowns can be safely computed.
                -->
                <property>/fdm/jsbsim/damage/repairing</property>
            </or>
        </input>
        <output>
            <property>/controls/gear/brake-parking</property>
        </output>
    </logic>

The cause may be linked to a singleshot timer in Nasal/physics.nas that is set upon bush gear change out. This function works fine on a normal startup but not on a reset such as happens with a Airport relocation.

var set_bushkit = func (bushkit) {
    setprop("/fdm/jsbsim/damage/repairing", 1);
    reset_all_damage();
    bushkit_changed_timer.restart(bushkit_change_timeout);
};

# This timer object is used to enable damage again a short time after
# changing to the last bush kit option.
var bushkit_changed_timer = maketimer(bushkit_change_timeout, func {
    setprop("/fdm/jsbsim/damage/repairing", 0);
    setprop("/fdm/jsbsim/damage/traversing", 0);
});
bushkit_changed_timer.singleShot = 1;

# Update the 3D model when changing bush kit
setlistener("/fdm/jsbsim/bushkit", func (n) {
    set_bushkit(n.getValue());
}, 1, 0);

Further investigation is needed to determine what is causing this repairing flag to be set and by what function for sure.

Test if Airport relocation triggered by tutorial or mooring relocation trigger the same issue.

Tested all gear change outs and the only one this bug happens with is the default gear.
In other words if you are using any other gear than default, this bug does not happen.
Something I noticed that may be part of the issue. When doing an airport relocation, the gear change out is reverting to default. Part of the problem may be because all gear set files call the c172p-set.xml file. c172p-set.xml ile sets bushkit = 0. But I think the order of operation is important here.
For example the flow looks to be...
c172p-bush26-set.xml set file calls c172p-set.xml then executes the rest of its xml

<PropertyList include="c172p-set.xml">
    <sim>
        <variant-of>c172p</variant-of>
        <description>Cessna 172P (160 hp) 26" Bush Tires</description>
        <previews>
            <preview>
                <type>exterior</type>
                <path>Previews/c172p-preview11.jpg</path>
                <splash type="bool">true</splash>
            </preview>
        </previews>
        <model>
            <variant type="int">1</variant>
        </model>
    </sim>
    <controls>
        <engines>
            <active-engine type="int">0</active-engine>
        </engines>
    </controls>
    <fdm>
        <jsbsim>
            <bushkit type="int">1</bushkit>
        </jsbsim>
    </fdm>
</PropertyList>

More investigation, ah, because any variant calls c172p-set.xml the second bushkit change out is triggering the set-bushkit function which in turns executes the repairing= true property and the timer to set it back to false after 8 seconds. This whole system was designed for safely changing gear changes and avoiding damage during the initialization.
So it appears the first execution of set-bushkit function triggered by a listener is not happening.
Also I think we may need to change c172p-set.xml to have the same abbreviated instructions as the rest of the variant sets and all of them should include a new "master" or "main" file called c172p-main.xml. This is how the j3cub is setup.

[wlbragg]

I tested making the change to have the c172p-set.xml an abbreviated version of a set file like all the other set files. They all include c172p-main.
There were two properties specifically that were a potential issue the old way...
fdm/jsbsim/bushkit
controls/engines/active-engine
Both of these properties values are dependent on which variant your in.
The old way they both are set to default load and overwritten by the variant set.
The new way they are set by the variant set exclusively.

This change cleans up some of the bugginess of not getting the last selected load out on a restart.

Odd though this change had no effect on the repairing flag issue. Still set to true on airport reload if in the default load out but works correctly when in any other variant.

[wlbragg]

Two other notable issues during an airport relocation.

2. If your in an exterior view when you do the relocation it messes up that view on reload and puts you interior looking at the pilot seat.

3. On airport reload the gear are not picking up the correct z-index setting.

Original issue

1. Repairing flag set to true during "Go to Airport" relocation

All testing so far done with 2019.2.0. I'll try 2020.1.2

Time passes...

Tested on 2020.1.2 same issues.

Issue 3 is because the presets are using information passed from the last selected variant and not the last change out done using the GUI. Probably a aircraft saved data issue and the mechanics of the reinit VS new session loading.

Issue 1, both mooring relocation and tutorial relocation work correctly, in amphibious mode at least. As in repairing flag is set correctly.

[gilbertohasnofb]

Thanks for catching this bug!

Issue 1, both mooring relocation and tutorial relocation work correctly, in amphibious mode at least. As in repairing flag is set correctly.

If you solved the issue, do open a PR and I'll merge it.

As for the other two issues, are those fixable from our side? I always thought that the airport relocation option was quite buggy in FG in general...

[wlbragg]

As for the other two issues, are those fixable from our side?

@zakalawe has been working on getting the bugs out of it for awhile now and I think issue 3 may be fixable on our side. It's the variants VS saved data flow that is at issue. It may be a matter of totally understanding the difference between the two and programming accordingly.

Issue 2 I think is something new and I need to make sure it is a bug with other aircraft and not just the c172p. It is more likely a core issue after the resent view changes.

[wlbragg]

@legoboyvdlp @dany93 @gilbertohasnofb

I'm at a bit of a crossroads here, this issue seems to be gone in the fg1000 variant branch. At some point I could use some assistance verifying some corrected issues, testing and merging some of this work.

[legoboyvdlp]

Hi there, thanks for the ping.

For a start I'll try and reproduce this one on master.

[legoboyvdlp]

A simple reposition on master did indeed show the issue.

So it doesn't happen in tutorials. But I think this may be because tutorials cycle a damage repair: you can audibly hear the repair sound.

Unfortunately on a reposition in the FG1000 branch, default gear configuration both with and without the FG1000 variant, it also went to damage=true. The view didn't change on the first reposition but did on the second. I think this issue might not actually be solved.

[wlbragg]

Thanks, I forgot to check the default gear specifically which was the main culprit originally. OK, I'll dig into this one and get it resolved eventually, then ask for another review.

[ecraven]

Using 2020.3.6 AppImage, this is a problem in the tutorials. Both Taxiing and Take-Off do not work, because you cannot release the parking brakes. Manually setting /fdm/jsbsim/damage/repairing to false works. Thanks to @colin on IRC for explaining this to me.