flybywiresim / aircraft

The A32NX & A380X Project are community driven open source projects to create free Airbus aircraft in Microsoft Flight Simulator that are as close to reality as possible.
https://flybywiresim.com
GNU General Public License v3.0
5.14k stars 1.12k forks source link

[TRACKING] Reactive systems #1525

Open devsnek opened 4 years ago

devsnek commented 4 years ago

Currently we poll (check every update cycle) on lots of things, like the positions of switches in the cockpit. As an example, a pilot is not moving ADIRS knobs every 1/60th of a second, so there's no need to check its position that often. Moving to event (reactive) code where possible (using h-events and such) would help improve performance.

davidwalschots commented 4 years ago

Using e.g. RxJS would make a lot of the code clearer. Right now I'm seeing a lot of variables being set and then used somewhere 400 lines later. That by itself would already be a good reason for using a different programming model. There's a lack of "flow" to understanding the code in that sense. All I see is events! An example. Lower ECAM Bleed:

Current:

let currentLeftPackState = SimVar.GetSimVarValue("L:A32NX_AIRCOND_PACK1_TOGGLE", "bool");
let currentRightPackState = SimVar.GetSimVarValue("L:A32NX_AIRCOND_PACK2_TOGGLE", "bool");

// Various unrelated lines

// Original code also sets bothPacksOn, but ignored for the example.
if (currentLeftPackState && currentRightPackState) {
    this.singlePackOn = false;
} else if (currentLeftPackState || currentRightPackState) {
    this.singlePackOn = true;
} else {
    this.singlePackOn = false;
}

// 60 lines later and only usage
//sets pack flow to high if TOGA applied/ single pack on / bleed provided by apu alone
if (this.thrustTOGAApplied || this.singlePackOn || (!currentEngineBleedState[0] && !currentEngineBleedState[1] && this.apuProvidesBleed)) {
    currentPackFlow = 2;
}

RxJs:

const leftPackState$ = OurMagicSimVarToObservableMagic.GetObservableOf("L:A32NX_AIRCOND_PACK1_TOGGLE", "bool");
const rightPackState$ = OurMagicSimVarToObservableMagic.GetObservableOf("L:A32NX_AIRCOND_PACK2_TOGGLE", "bool");

const isSinglePackOn$ = combineLatest(leftPackState$, rightPackState$).pipe(
    map(([leftPackState, rightPackState]) => {
        return leftPackState + rightPackState === 1; 
    })
);

const packFlow$ = // ... etc. This goes on a while until we actually need to subscribe to do UI changes.
devsnek commented 4 years ago

@davidwalschots that seems interesting but is unrelated to this issue. this issue is specifically about changing polling GetSimVarValue calls into onInteractionEvent h-events.

derl30n commented 3 years ago

I am a little late, just curious because the problem above seems to be easy fixable with a simple comparison of both Booleans, or did I missed something?

this.singlePackOn = currentLeftPackState !== currentRightPackState;
davidwalschots commented 3 years ago

Yes. The boolean expression can be simplified. The example selected is not the best one. As I learn more about the project, I also learn that a lot of this code should move away from JS anyway. So in that sense my suggestion was not useful.

derl30n commented 3 years ago

Yes absolutely our code is poorly optimized (one can even say our code isn't optimized at all ^^) so anyways we need to figure stuff out ;).