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

Issue 1330 #1332

Merged
merged 5 commits into from
Sep 11, 2020
Merged

Issue 1330 #1332

merged 5 commits into from
Sep 11, 2020

Conversation

gilbertohasnofb
Copy link
Member

Closes #1330

The idea behind this PR is:

  • any lever that moves in/out or up/down will match the direction of the scroll
    • scroll up: raise flaps
    • scroll up: move throttle and mixture in
    • scroll up: move trim wheel up, lowering the nose
  • for knobs that turn on a different plane (e.g. all gauge knobs), scrolling up means increase of value, just like any GUI out there (e.g. volume knobs on OS's). This will also match other flight simulators as far as I know.
  • I also slowed down the animation of the scroll wheel so that it looks more smooth, it was moving too much for each small step of trim and did not look too good.

@gilbertohasnofb
Copy link
Member Author

@wlbragg @dany93 and everyone else: ready for testing and merging

@dany93
Copy link
Collaborator

dany93 commented Aug 19, 2020

Correct for the inverted directions.

Trim wheel:
I think that the control step is too coarse: try stabilizing a horizontal flight to see.

In Models/c172p.xml, lines 5238 - 5247

        <action>
            <binding>
                <command>property-adjust</command>
                <property>controls/flight/elevator-trim</property>
                <factor>0.001</factor> <!-- 0.01 -->
                <min>-1</min>
                <max>1</max>
                <wrap>0</wrap>
            </binding>
        </action>

The initial <factor> is 0.01.
Changing it for 0.001 is good IMO. (this is the step I have on my joystick binding file)

@wlbragg, can you confirm that I did it the right way?

In this case, displaying three decimal digits (instead of two) on the hover indication would be better too.
I'm not used to that, but

        <hovered>
            <binding>
                <command>set-tooltip</command>
                <label>Elevator trim: %3.3f</label> <!-- %3.2f -->
                <tooltip-id>pitch-trim</tooltip-id>
                <property>controls/flight/elevator-trim</property>
            </binding>
        </hovered>

(with %3.3f instead of %3.2f)
does the trick.

@gilbertohasnofb
Copy link
Member Author

@dany93 just to be clear, I have not changed the trim property itself, just how fast the animation of the trim wheel is. I can of course adjust that in this same PR. @wlbragg can you give us your thoughts?

@dany93
Copy link
Collaborator

dany93 commented Aug 19, 2020

In this case, it was like that before. I had never noticed it because I use my joystick for trim control. And the trim wheel is not easily visible.
Weird that nobody else noticed it....

I give my opinion but I'd prefer that you and @wlbragg are convinced if you do it. Can you try stabilizing in flight?

@legoboyvdlp
Copy link
Member

I agree -- also throttle; setting by 5% seems a bit fast.

@wlbragg
Copy link
Collaborator

wlbragg commented Aug 19, 2020

I like it right where it's at. It seems OK to me. I defiantly wouldn't want to go coarser than it is now but i don't seem to have a hard time tuning it where it is at.
I just tried it at 0.001 and I feel it is too fine and takes to long to make any change. Keep in mind this is with a mouse, It may be that with the JS it is more sensitive and the JS needs to be set finer. But with a mouse it's really slow at .001.
Something else I don't like is the rudder trim. Trying to keep the mouse on the lever and scrolling the wheel doesn't work well. I think the text legend needs to also be hot.

@gilbertohasnofb
Copy link
Member Author

I just tried it at 0.001 and I feel it is too fine and takes to long to make any change.

@wlbragg I have to say I agree with @dany93 on this one. Our current value is 0.01, I tried halving it to 0.005 and I still struggle to precisely trim the aircraft in air. I understand that it takes long to make large changes, but the question is do pilots really need to make such large changes in RL? Actually, is the range of our trim too large perhaps? Regardless, I am definitely up for lowering it substantially, possibly to 0.002.

also throttle; setting by 5% seems a bit fast.

@legoboyvdlp I tested it at 2% and it feels much better. Much better control over mixture and throttle and it still takes a reasonable time to go from open to full.

I can push these changes for you all to test if we are in the same boat.

@dany93
Copy link
Collaborator

dany93 commented Aug 20, 2020

@wlbragg wrote

I just tried it at 0.001 and I feel it is too fine and takes to long to make any change. Keep in mind this is with a mouse, It may be that with the JS it is more sensitive and the JS needs to be set finer. But with a mouse it's really slow at .001.

I have in mind that I rarely have to change the elevator trim by more than 0.1 or 0.2 (0.3 at final approach). Which does not take that long, and is to be made progressively.
Controlling with a mouse or a joystick should not be different if one wants to test the trim. The difference is that, with a mouse, you can stabilize at any value different from 0, no trim needed. With a joystick, the springs hold it at zero when released and you need a trim to stabilize.

With my JS trim at 0.001 step, I can stabilize horizontal flight during more than one or two minutes with no JS intervention.
160 hp engine
(read in the Internal Properties)
2276 RPM
106 kts
elevator trim 0.096
aileron trim 0.019
Stability: vertical speed oscillates +/- 20 ft/mn.

@gilbertohasnofb
Copy link
Member Author

For me, we can have the trim much much finer as Dany is suggesting, even at 0.001 as he says. I will push this modification shortly together with the throttle and mixture with steps of 2% instead of 5% so that everyone can test it.

aileron trim 0.019

What aileron trim? I thought our aircraft did not have an option for that.

Something else I don't like is the rudder trim. Trying to keep the mouse on the lever and scrolling the wheel doesn't work well. I think the text legend needs to also be hot.

We might want to make that one finer too. Also, I do not understand what you mean by hot legend.

@dany93
Copy link
Collaborator

dany93 commented Aug 20, 2020

@gilbertohasnofb wrote

What aileron trim? I thought our aircraft did not have an option for that.

Correct on our aircraft.
But for convenience, I coded aileron, elevator and rudder trim controls by buttons in my Joystick binding file. That's cheating, but Joysticks go back to 0 when you loosen them (springs) and this is much more uncomfortable than in reality. In reality, it is not so strong and you don't feel this kind of "notch" at neutral. Anyway, even in reality, having neutral controls is very comfortable and more accurate. For the aileron (and rudder trim if there is no), this is done by mechanics on the ground, compromise at a given cruise airspeed.

I gave you the aileron trim to help if you wished to try this way. Accessible by setting it in the properties. For a given rpm and airspeed (close to the one I gave), you even do not need changing the aileron trim value.

Also, the possibility to loosen the JS controls and have a stable trajectory is particularly interesting for accurately doing tests such as drag measurements.

@wlbragg
Copy link
Collaborator

wlbragg commented Aug 20, 2020

I'm OK with whatever you all decide. .002 would be my choice over .001. My thoughts on @dany93 argument of trimming only .1 to .3 during a flight kind of supports the value being .01 and not .001. Theoretically wouldn't it take 10 turns or 10 to 1 to turn from .1 to .2 at .01, but 100 turns at .001? With a matching "step" setting of course. That is what I feel on my mouse when the setting is at .001. Are you all using the mouse for this, except @dany93? 100 turn to get a 1% trim is excessive.
Let me try it again, I was in a real hurry when I tried it last time.

I do not understand what you mean by hot legend.

Meaning the "hot" spot is the knob and it appears to scroll off the mouse when turning the mouse wheel and then wont scroll anymore without moving the mouse back over the knob. So if you make an invisible hot spot (that's what I really meant), the size of the texture mesh for the entire trim graphic or close to it. Not a must I don't use it that much and it's not that hard to use. I'm not usually trimming the rudder but maybe once and that would be at a low workload time where I can afford some attention.

@wlbragg
Copy link
Collaborator

wlbragg commented Aug 20, 2020

After reviewing it again and using it to trim the aircraft I am OK with .001 or .002. It is actually way better. The first time I was in a hurry and it appeared it was taking way to long to move it to where it was effecting the aircraft. But from a "sane" starting trim it is actually a fine adjustment you need and being coarse just makes it harder ti get the trim correct.
It's possible we will get some negative feedback on this at first introduction to those that are used to the other setting. But I believe the "finer" grain is better. I had been testing MS2020 and that trim is similar to ours now and not at all easy to set. It is to coarse..

@dany93
Copy link
Collaborator

dany93 commented Aug 20, 2020

@wlbragg wrote

Theoretically wouldn't it take 10 turns or 10 to 1 to turn from .1 to .2 at .01, but 100 turns at .001?

(if I understand well what you wrote...)
Going from 0.1 to 0.2 trim (= 0.1 difference) takes 10 steps at 0.01/step, 100 steps at 0.001/step.
One scroll-wheel notch gives one step (0.01 or 0.001). Which makes 10 or 100 scroll-wheel notches, not 10 or 100 scroll wheel turns.

The best and easiest is to rotate the scroll-wheel and observe the trim values in the sim.

@dany93
Copy link
Collaborator

dany93 commented Aug 20, 2020

@wlbragg wrote

Something else I don't like is the rudder trim

I almost never use the rudder trim setting.
I had to change it once on the P51D for settings of the propeller effects. But in fact, these effects were way too strong, which needed a lot of rudder (or trim) to keep the slip ball centered.
Since that, I usually set a (weak) compromise value in the -set.xml file and that's enough. At climbing, full thrust, one needs the rudder but only temporarily, and at cruise the compromise (fixed) value is sufficient.

@wlbragg
Copy link
Collaborator

wlbragg commented Aug 20, 2020

Right, I wasn't trying that hard to see the relationships, turns, notches, etc., I was more generalizing. I think we're on the same page. I'm good with the changes as is. My bottom line is I can turn the wheel starting at a "sane" beginning value and get it trimmed without having to excessively take my attention away from flying.

I did think of a really nice addition we can make in time. I really hate having to change view to set any setting. The options are a quick key (I don't know if trim has any). I of course can use my JS, but for those on mouse I want to make a GUI popup like the Shuttle uses for many things (canvas) with the actual animated graphical mouse wheel for trimming so you don't have to change view at all yet it will feel like your actually doing it same as looking down at it because you see the graphical visualization of it in a popup.

@gilbertohasnofb
Copy link
Member Author

Ok, so I pushed my changes, please see what you all think. This is what I did:

  • inverted controls
  • throttle and mixture set to move 2% at every scroll instead of 5%
  • trim set to 0.001 from 0.01
  • rudder trim set to 0.002 from 0.01
  • all labels should display enough digits

@wlbragg I see what you mean about the hotspot now, and that is a good idea, but that would require some 3D modelling. If you want to do it, then we should add a hot spot behind the whole rudder trim, not only a cube at the centre, since users can still try to scroll above the handle. But I am fine with how it is as well and we can tackle this in the future at some point.

@dany93
Copy link
Collaborator

dany93 commented Aug 21, 2020

@wlbragg wrote

I did think of a really nice addition..... I really hate having to change view to set any setting..... for those on mouse I want to make a GUI popup...... with the actual animated graphical mouse wheel for trimming

Seems a nice idea. But one still has to see the hot spot to be clicked (e.g. the trim wheel) to click on it. Not easy unless with a large zoom angle or a kind of temporarily zooming.

@dany93
Copy link
Collaborator

dany93 commented Aug 21, 2020

@gilbertohasnofb
Tested.
From my point of view, everything is fine.
(except rudder trim, not tested because I don't know where it is)

Nice work, which has gone farther than you thought. Thank you.

@wlbragg
Copy link
Collaborator

wlbragg commented Aug 21, 2020

Seems a nice idea. But one still has to see the hot spot to be clicked (e.g. the trim wheel) to click on it.

I would use a quiick key to open it.

@gilbertohasnofb
Copy link
Member Author

@dany93 Thanks for testing!

(except rudder trim, not tested because I don't know where it is)

This is enabled in the C172P > Aircraft Menu dialogue.

@wlbragg When you have a chance, would you check the modifications again just so we are sure everyone is happy with the results before merging? Otherwise this is ready to be merged

@dany93
Copy link
Collaborator

dany93 commented Aug 21, 2020

Rudder trim:
I tested it.

It has a very weird action. It makes the aircraft turn, but not like the rudder should do. The slip ball almost does not change, differently from an action on the rudder.
In FG, it should change the rudder trim controls/flight/rudder-trim. Very simple.
Instead, it changes controls/flight/rudder-trim-knob, fcs/yaw-trim-cockpit, and its yawing moment seems added to yaw-trim-sum by a very weird an inappropriate manner. I think it does not act like the rudder (+rudder-trim) should do.
I don't understand why this weird way of adding a rudder trim has been used.

In reality, the rudder trim acts by changing the "neutral" position of the rudder (thanks to the airflow on the rudder-trim tab) when there is no action on the yoke. At the end, the yawing moment is always done by the rudder position (slightly deflected here). Not some other extra "thing".

@wlbragg
Copy link
Collaborator

wlbragg commented Aug 21, 2020

We definitely need to change it then, I don't recall who did the code I know I did the modeling and @gilbertohasnofb did the textures. I may have done the code as well and didn't understand the FDM properly.

@dany93
Copy link
Collaborator

dany93 commented Aug 21, 2020

In the FDM, controls/flight/rudder-trim is added to controls/flight/rudder by the intermediate of the fcs. This sum forms the Yaw Trim Sum.

To make it simple, all that the lever has to do is changing controls/flight/rudder-trim

At first view, it only should remain

            <summer name="Yaw Trim Sum">
                <input>fcs/rudder-cmd-norm-filtered</input>
                <input>fcs/yaw-trim-cmd-norm</input>
                <clipto>
                    <min>-1</min>
                    <max>1</max>
                </clipto>
            </summer>

(see the <summer name="Pitch Trim Sum"> and <summer name="Roll Trim Sum"> to confirm)

Setting 0.13 to the rudder trim should have the same effect as adding 0.13 to the rudder control.

@legoboyvdlp
Copy link
Member

All looks good here 👍

@gilbertohasnofb
Copy link
Member Author

@dany93 Ok, I found three entries where controls/flight/rudder-trim-knob was mentioned in c172p.xml and I changed it to controls/flight/rudder-trim. Could you please take a look if this takes care of the issue with the FDM?

@dany93
Copy link
Collaborator

dany93 commented Aug 22, 2020

@wlbragg controls/flight/rudder-trim
After more thinking, your initial code with rudder-trim-knob did the job, sorry for having been too harsh. However, by an inappropriate manner. Moving the rudder-trim knob without seeing the controls/flight/rudder-trim value change was confusing.
Moreover, the code with controls/flight/rudder-trim is simpler. Not only in accordance with usual way.

@gilbertohasnofb , @wlbragg
It works well like that.
However, some cleaning remains to be done.

  • in c172p.xml, lines 813 - 834, delete the fcs/yaw-trim-cockpit (useless from now) parts.
    (under comment tags below)
            <!-- <fcs_function name="fcs/yaw-trim-cockpit">
                <function>
                    <table>
                        <independentVar lookup="row">/controls/flight/rudder-trim-knob</independentVar>
                        <tableData>
                            -.18    0.20
                            0.00    0.00
                             .18   -0.20
                        </tableData>
                    </table>
                </function>
            </fcs_function> -->

            <summer name="Yaw Trim Sum">
                <input>fcs/rudder-cmd-norm-filtered</input>
                <input>fcs/yaw-trim-cmd-norm</input>
                <!-- <input>fcs/yaw-trim-cockpit</input> -->
                <clipto>
                    <min>-1</min>
                    <max>1</max>
                </clipto>
            </summer>
  • c172p-set.xml, line 595, delete rudder-trim-knob
    <!-- <rudder-trim-knob type="double">0.0</rudder-trim-knob> -->
  • c172p.nas, line 321
    # setprop("/controls/flight/rudder-trim-knob", 0.0);

Despite these deletions,

  • the controls/flight/rudder-trim-knob still appears in the Internal Props,
  • nasal errors still appear (although not sure they are connected with this issue)
    [ALRT]:nasal ERROR: Cannot add listener to tied property /controls[0]/flight[0]/rudder-trim[0]
  • In c172p-set.xml, I had set the rudder trim at 0.02 as an approximate compromise to have the slip ball centered at cruise
    <rudder-trim type="double">0.02</rudder-trim>
    It might be better to have it at 0 at start with the rudder trim option (@wlbragg ?).
    Otherwise, the effect is (was, when I did it?) not only an approximate compromise without rudder trim setting in the cockpit, but also very weak. An possibly depending on 160 or 180 hp...

(Probably not exhaustive....)

@wlbragg
Copy link
Collaborator

wlbragg commented Aug 22, 2020

After more thinking, your initial code with rudder-trim-knob did the job, sorry for having been too harsh.

No Worries!

It might be better to have it at 0 at start with the rudder trim option (@wlbragg ?).

Would most people more than likely have to adjust the knob then at startup? Or would it be best to have the knob position match the .02 at start up? If .02 is a good trim I would feel funny leaving the plane out of trim without some kind of notice, although we don't do that with the elevator trim. The elevator trim is expected to be change thought in most every flight.

@gilbertohasnofb
Copy link
Member Author

@wlbragg @dany93 could you guys take a look at this and push fixes into this branch? I think I know less than both of you about the rudder trim issue.

Would most people more than likely have to adjust the knob then at startup? Or would it be best to have the knob position match the .02 at start up? If .02 is a good trim I would feel funny leaving the plane out of trim without some kind of notice, although we don't do that with the elevator trim. The elevator trim is expected to be change thought in most every flight.
Merge state

The state of the rudder trim should be saved if the option of saving states is on. The idea is that the trim tab will be left in the same position as the last pilot, and it's part of the realism to check for that. But In the case of the rudder trim, we need to reset it to 0.0 whenever the option of using rudder trim in the aircraft dialog is disabled, since the pilot will not have access to it any longer. Makes sense?

@wlbragg
Copy link
Collaborator

wlbragg commented Aug 22, 2020

The state of the rudder trim should be saved if the option of saving states is on.

OR

But In the case of the rudder trim, we need to reset it to 0.0 whenever the option of using rudder trim in the aircraft dialog is disabled

I think you meant .02?

Yeah, I got that, so you think it is OK to set it to .02 when rudder trim is disabled and always back to 0 when rudder trim is enabled? If so then I wonder if we need a separate property for the adjustable rudder trim so it can have its own persistence separate from the auto trim. Then it would work like thi...
Select auto trim and trim is set to .02 always.

Enable manual trim and use its isolated property so it can be persistent and it will be whatever that last value was set at.
OR
Enable manual trim and it is always set to 0.
OR
Enable manual trim and it is always set to 02.

The above is settings are on the fly.

Then we have the "on startup" and of course auto trim will always be at .02.
But manual trim could either be persistent or always set to 0 or always set to start at .02.

These are all the options we have, what do we want?

@dany93
Copy link
Collaborator

dany93 commented Aug 23, 2020

Select auto trim and trim is set to .02 always.

Yes, if possible (we are on the fly). Or (in fact), the value in c172p-set.xml. Otherwise, it will be set at next startup.

Enable manual trim and use its isolated property so it can be persistent and it will be whatever that last value was set at.

Uselessly complicated. If one clicks to enable the manual trim, it can be set to 0. This is a new configuration.

Enable manual trim and it is always set to 0.

Yes

Enable manual trim and it is always set to 02.

No. It can (should) be set with the trim lever. Otherwise, complicated because you would have to read the c172p-set value, which is an approximate compromise, not that important. And useless for manual-trim.

The above is settings are on the fly.

Yes.

Then we have the "on startup" and of course auto trim will always be at .02.

Yes Yes! Or the value in c172p-set.xml

But manual trim could either be persistent or always set to 0 or always set to start at .02.

If no too complicated, the best would be persistent if the manual option is kept from one session to the next.
Otherwise (still if manual option is kept from one session to the next), 0 is the best and most simple.

I noticed that, since my last proposed cleaning of files (or maybe not due to it), disabling the rudder trim (i.e. changing from manual to disabled = auto in flight) resets it to 0. Which is perfectly acceptable because it is only for the current session. Next start with "disabled" (= auto trim) will be with 0.02 (or the value in c172p-set.xml).

@dany93
Copy link
Collaborator

dany93 commented Aug 24, 2020

@wlbragg wrote:

If so then I wonder if we need a separate property for the adjustable rudder trim so it can have its own persistence separate from the auto trim

For what it's worth, kind of template for the principles...
A possible way (??) might be
in c172p-set.xml

    <controls>
        <flight>
            <rudder-trim-preset type="double">0.02</rudder-trim-preset>

In c172p.nas
(if auto trim)
setprop("/controls/flight/rudder-trim", getprop ("/controls/flight/rudder-trim-preset"));
or (if manual, new check or non-persistent at startup)
setprop("/controls/flight/rudder-trim", 0);
or (if manual, persistent, already checked at startup)
setprop("/controls/flight/rudder-trim", "the persistent value");

This way, the FDM is unchanged and as usual. Also, the pre-set value in c172p-set.xml is easy to understand.

@gilbertohasnofb
Copy link
Member Author

@dany93 would you like to push the necessary modifications to the rudder trim so that we can merge this?

@dany93
Copy link
Collaborator

dany93 commented Aug 28, 2020

I have nothing to push.
My previous message was just indications to (maybe, hopefully) help @wlbragg if you want to include this features in the code. But, as I wrote it, I'm not sure of their validity, and you can see part of them are only schematic.
And @wlbragg can have better ideas, he did most of this code. I discovered this feature by reading this issue. Among other things, I have no practice of the persistent values and options.

@gilbertohasnofb
Copy link
Member Author

@wlbragg would you like to work on the rudder trim feature on this PR?

@wlbragg
Copy link
Collaborator

wlbragg commented Aug 28, 2020

Yeah, I'll finish this up. I have my hands full at the moment with out of town guests, but will try to squeeze it out in the next 5-7 days, if that is OK?

@gilbertohasnofb
Copy link
Member Author

Absolutely, there is no rush. Hope you are having a great time with your guests!

@wlbragg
Copy link
Collaborator

wlbragg commented Sep 10, 2020

@dany93

I wasn't understanding closely what the changes were here. But I understand we basically removed rudder-trim-knob as the controlling property for this.

I'm trying to finish these last few PR up.

You posted

Despite these deletions,

the controls/flight/rudder-trim-knob still appears in the Internal Props,

I did a find-in-files search and below are the remaining uses of the "rudder-trim-knob" property.
I assume the sound is OK or have we completely removed the "rudder-trim-knob" property and the sound should be tied to something else?

The setting of the "rudder-trim-knob" property to 0 in the dialog should be removed as well? Actually it will be replaced by the proper trim property which is what now? Is is simple controls/flight/rudder-trim?

Searching 915 files for "rudder-trim-knob"

/mnt/IDrive/FG/Aircraft/Development Aircraft/c172p/c172-sound.xml:
 1296              <mode>in-transit</mode>
 1297              <path>Sounds/rudder-trim.wav</path>
 1298:             <property>/controls/flight/rudder-trim-knob</property>
 1299              <position>
 1300                  <x>-0.3660</x>

/mnt/IDrive/FG/Aircraft/Development Aircraft/c172p/gui/dialogs/aircraft-dialog.xml:
  367                      <script>
  368                          if (!getprop("/sim/model/c172p/ruddertrim-visible")) {
  369:                             setprop("/controls/flight/rudder-trim-knob", 0);
  370                          }
  371                      </script>

3 matches across 3 files

@wlbragg
Copy link
Collaborator

wlbragg commented Sep 10, 2020

nasal errors still appear (although not sure they are connected with this issue)
[ALRT]:nasal ERROR: Cannot add listener to tied property /controls[0]/flight[0]/rudder-trim[0]

I don't think this is anything.

@wlbragg
Copy link
Collaborator

wlbragg commented Sep 10, 2020

I already removed the other code you suggested above and changed the tool tip string formatting

<label>Elevator trim: %3.3f</label> <!-- %3.2f -->

but haven't pushed it yet.

@wlbragg
Copy link
Collaborator

wlbragg commented Sep 10, 2020

Marked for deletion

c172p.nas, line 321
# setprop("/controls/flight/rudder-trim-knob", 0.0);

In reality, I think this will change to the rudder trim persistent property for the manual trim.
The setting of the automatic trim will be done at startup or in the dialog when the change is to use "no ruder trim slide".

In c172p-set.xml, I had set the rudder trim at 0.02 as an approximate compromise to have the slip ball centered at cruise
0.02
It might be better to have it at 0 at start with the rudder trim option (@wlbragg ?).
Otherwise, the effect is (was, when I did it?) not only an approximate compromise without rudder trim setting in the cockpit, but also very weak. An possibly depending on 160 or 180 hp...

I will finish all this up as well and this should be all to close this issue up.

@wlbragg
Copy link
Collaborator

wlbragg commented Sep 10, 2020

@gilbertohasnofb @dany93
I don't think you all understood why I did what i did with rudder-trim-knob and I certainly didn't defend why I did it that way, because I didn't remember why. It was set for the animation of the knob graphics and then mapped to the rudder-trim property. By eliminating it and using the rudder-trim property directly the rudder trim knob no longer works correctly and I have to change the range and scale of that animation. I don't know if that is possible or not. I guess I can convert or scale the rudder-trim range to match the rudder trim knob animation directly?
I just discovered this and ran out of time for today. I will look at it again tonight or tomorrow.

@wlbragg
Copy link
Collaborator

wlbragg commented Sep 10, 2020

I think I figured out how the persistence of the rudder trim will work.
We set it to 0 in the set file.
On load, if ruddertrim-visible is false, (it is auto), it gets changed to .02
If not it remains at 0, ("We set it to 0 in the set file"), unless it was made persistent by "switches_save_state" in which case it will be whatever it was set at when exiting last.
There is a listener tied to the ruddertrim-visible and if it is triggered it sets it to 0 or .02 accordingly.

@wlbragg
Copy link
Collaborator

wlbragg commented Sep 11, 2020

I guess I can convert or scale the rudder-trim range to match the rudder trim knob animation directly?

OK, I was able to fix this in the animation.

A couple more questions about it however.

  1. Should the ruddertrim-visible property be persistent always if you exit the sim with it active or only if you check "Switches Save State"?
  2. Depending on (1) above, should the rudder trim setting be persistent?

Choices for (2) are...
if the ruddertrim-visible property is persistent always if you exit the sim with it active then should the actual rudder trim be persistent always or only if you have also selected "Switches Save State"?

@wlbragg
Copy link
Collaborator

wlbragg commented Sep 11, 2020

OK I think this one is done. I am happy with the persistence logic. It is as follows...
Auto rudder trim is always set to .02 whether starting the sim or changing it in the GUI.
Manual rudder trim visibility choice is always persistent.
If exiting with rudder trim visibility set to true then the rudder trim setting will either be set to 0 at the start of next session or set to the last setting if the "Switches Save State" is also checked.
If the rudder trim visibility GUI choice is changed from auto to manual the rudder trim is always set back to 0.

Please verify before merging...

  1. all @gilbertohasnofb changes for elevator trim are still in this and correct
  2. rudder trim knob change sound is OK
  3. rudder trim is functioning correctly
  4. rudder and elevator trim tool tip formatting correct
  5. rudder trim and elevator trim persistence is as intended

I think that is it, I'm moving on to #1327 and #1335

@gilbertohasnofb
Copy link
Member Author

gilbertohasnofb commented Sep 11, 2020

A couple more questions about it however.

I think the rudder trim option should be persistent between sessions despite the switches save state, since that's an aircraft option similar to having the GPS or bush wheels which, I believe, persist between sessions despite the save state option (which is correct).

If that option is ON, the rudder trim value should be persistent, otherwise it should go back at zero. Also, whenever the rudder trim option is removed, it's value should also be reset (since the user won't have the trim to change it back).

What do you think?

Edit: I see this is exactly what you wrote, so it should all be good. I will test this and merge it now.

@gilbertohasnofb gilbertohasnofb merged commit 996d98d into master Sep 11, 2020
@gilbertohasnofb
Copy link
Member Author

@wlbragg it all works exactly as it should, thanks for this

@wlbragg
Copy link
Collaborator

wlbragg commented Sep 11, 2020

Thanks @gilbertohasnofb !

@wlbragg wlbragg deleted the Issue-1330 branch November 6, 2020 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invert mouse wheel set to true, by default...
4 participants