MobiFlight / MobiFlight-Connector

MobiFlight is an open source project that allows you to create your own home cockpit for your favorite flight simulator in a flexible, affordable and extremely user-friendly way.
https://mobiflight.com
MIT License
234 stars 104 forks source link

Misbehaviour when Placeholder in a Formula uses a "Char" that is part of a command. #362

Closed MobiFlight-Admin closed 2 years ago

MobiFlight-Admin commented 3 years ago

Problem summary

If the choosen symbol for a Placeholder ( as Expression Syntax) is also used in a “function command” that occured in a Error!

Steps to reproduce

Expected result

Placeholder “f” is defined and Syntax is correct. So it should work

Actual result

ERROR…… Simply in case Mobiflight “swap” now every “f” in the syntax into the value of the used Placeholder Config Refference.
It ALSO swap the “f” from the “if” command…..
So lets Say Config1 have a Value of “5” Then our executed Code is pretty sure i5(5=1,999,$)

Same is confirmed if we use e.g. the Command “Round” in the Transform Formula….
Then “R” “o” “u” “n” “d” are no longer useable as Placeholder Symbols here.

Further details

If possible we should “exclude” the Ncalc Funtions like “if” “in” “Round” and so on as standaloane Words that are not effected by the Placeholder Settings…..
If not possible ( or need a hudge rework of our logic) we should include this Workaround in the FAQ and within a short Video/Tutorial …… The use need to be informed what happen here and how to avoid this !

[Issue created by Pizman: 2021-05-12] [Last updated on bitbucket: 2021-05-25]

[Comment created by DocMoebiuz: 2021-05-25] The only thing I can spontaneously think of is, to make the placehoder logic more complicated, and use [ ] (brackets) or { } (curly braces) around them. And yet, this will also possibly yield a conflict in case I wanted to use these in my LCD output.

But at the end of the day, this feature is an expert feature and if people want to use it, they are smart enough to prevent a conflict in the first place.

[Comment created by Pizman: 2021-05-15] I just talk to Sebastian…. For now this is not priority….. We go the way to TELL the people how to handle this.
I agree to him….. this is a special case and every minute to solve this is lost time to build something else that is more usefull!

About your Comments @Jaime Leon and @Tuomas

1. LCD….. Here we know this fact. This in not new. We recommend this since the first days we implement that tool.
For LCD use Symbols ONLY ….. And Avoid of “:” and “-” if you like to use them as label Text.

2. Tuomas Formulas.
TAKE CARE !!! This is not only “hard to parse” This is fullly wrong and your just lucky it works here.
\% and & are mathematical Operators !! (Modulo and AND)
So if you use a Formula like “$%5-A” will work….. But sure “$%5-%” will not cause now %Modulo is no longer a Operator, its a Value now!

Thats why we make also from beginning the Rule “Use only characters for Placeholder in formulas….. NOT +-*/%&$ or so.
We “just” not think about the fact that “i” and “f” is used in the word “if” , too.

Capital Chars will work fine and are individual to the small ones…. You can use “A“and “a” as TWO different Placeholders in one Config !
BUT
Same Problem if you use e.g. “Round” as command…. then the Capital “R” is blocked…. the small “r” would work here.


So my final note….. “Won´t Solve” BUT A Note in Tutorial/FAQ to inform the User to NOT use Chars/Symbols that are a part of the Formula or a part of the TEXT in qa Display !

[Comment created by tigert: 2021-05-13] Capital letters probably work by luck?

This is a bit unfortunate, as those special chars cause brain tumors and hair loss when trying to write them into formulas, as they are very hard to parse. I find A and B easier to understand.

if(% > &, 1, 0)

or

if (A > B, 1, 0)

But I think the placeholders should just be “sanitized”, like replacing them with some ncalc-unique strings (placeholder1, placeholder2 etc) instead of just doing a blind search and replace for the whole formula?

[Comment created by jaime709: 2021-05-13] I found a similar error in lcd display configurations when using placeholders letters that were also used as part of the text of the display. MF replaced those text letters with the value of the config.

Users need to be warned that if they use alphabet characters as placeholder characters, they must make sure not to use those characters that are used as part of the text or in this case, part of the formula string.

Its better to stick with the special characters like “@#%&” as placeholders or for example, use only upper case letters for your text and lower chase letters for your placeholders.

DocMoebiuz commented 2 years ago

I tend to close this with "won't fix" because how much of a problem is it actually? Once you figured it out you can take care and make sure there are no conflicts.

Especially for the LCD Display it is not easy to do, because we want to have the preview in the UI. Any extra escape character like [A] or {A} would occupy more space in the display preview.

And UPPERCASE letters should work most of the times, but maybe only not on the LCD Display.


Once I rework the UI i might come up with a better way for this - but with WinForms it is not easy and in my eyes not worth the effort.