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

Setting - Added multiline setting parsing support #1581

Merged
merged 4 commits into from
Sep 7, 2023

Conversation

johnb432
Copy link
Contributor

@johnb432 johnb432 commented Jul 1, 2023

When merged this pull request will:

  • Lets CBA_settings_fnc_parse parse settings that are over multiple lines.
  • The word force now needs to be followed by a space, tab, LF or CR in order for it to not be considered a setting.

E.g:
Previously:

force forceforce = true; // Set the setting "force" to "be enforced 2x"

Now:

force forceforce = true; // Sets the setting "forceforce" to "be enforced 1x"

I have tested it, but not extensively.

@jonpas
Copy link
Member

jonpas commented Jul 1, 2023

Wiki page amended.

@jonpas jonpas added this to the 3.15.9 milestone Jul 1, 2023
@johnb432
Copy link
Contributor Author

johnb432 commented Jul 2, 2023

Something that just came to mind is forceUnicode, where is says trim supports the usage of forceUnicode.

I used this string: "[Āā,Ăă,Ҙ,привет]" to test. In my testing, Ҙ wasn't even recognised by Arma (I could not paste it into the console).

To test I added forceUnicode 0; at the start of the function. However it didn't seem to make a difference: Both the old (the one in CBA v.3.15.8) and the new versions of CBA_settings_fnc_parse returned the same thing.

Ultimately, what I'm trying to convey is that I am slightly concerned about non-ASCII chars being a potential problem, but at the same time the small amount of testing I did suggests it's fine.

EDIT: Minor correction: forceUnicode = 0; -> forceUnicode 0;

@rautamiekka
Copy link
Contributor

Something that just came to mind is forceUnicode, where is says trim supports the usage of forceUnicode.

I used this string: "[Āā,Ăă,Ҙ,привет]" to test. In my testing, Ҙ wasn't even recognised by Arma (I could not paste it into the console).

To test I added forceUnicode = 0; at the start of the function. However it didn't seem to make a difference: Both the old (the one in CBA v.3.15.8) and the new versions of CBA_settings_fnc_parse returned the same thing.

Ultimately, what I'm trying to convey is that I am slightly concerned about non-ASCII chars being a potential problem, but at the same time the small amount of testing I did suggests it's fine.

Did you actually have a forceUnicode = 0; or a forceUnicode 0; ?

@johnb432
Copy link
Contributor Author

johnb432 commented Jul 2, 2023

Did you actually have a forceUnicode = 0; or a forceUnicode 0; ?

No, that was a typo on my part, it was forceUnicode 0;.

@johnb432
Copy link
Contributor Author

johnb432 commented Aug 3, 2023

Did you actually have a forceUnicode = 0; or a forceUnicode 0; ?

No, that was a typo on my part, it was forceUnicode 0;.

Confirming that trim does not require forceUnicode.

Copy link
Member

@jonpas jonpas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ready?

@johnb432
Copy link
Contributor Author

Ready?

Testing by others would be nice.

@Mike-MF
Copy link
Contributor

Mike-MF commented Sep 7, 2023

Tested this with

ace_common_checkPBOsWhitelist = "[
    'ace_noactionmenu',
    'Blastcore_MainCore',
    'Blastcore_VEP',
    'WarFXPE',
    'ocap_main',
    'ocap_extension',
    'ocap_recorder'
]";

Came up in-game as
force ace_common_checkPBOsWhitelist = "['ace_noactionmenu','Blastcore_MainCore','Blastcore_VEP','WarFXPE','ocap_main','ocap_extension','ocap_recorder']";

it works.

@jonpas
Copy link
Member

jonpas commented Sep 7, 2023

it works.

In addition, it successfully parsed https://github.com/Theseus-Aegis/Mods/blob/96ce4f4a1c20029ad722a4cd992a314b461e6f7e/addons/cba_settings/cba_settings.sqf in its entirety as well (tested single-line and multi-line ace_common_checkPBOsWhitelist).

@jonpas jonpas merged commit 9ac1bd7 into CBATeam:master Sep 7, 2023
4 checks passed
@johnb432 johnb432 deleted the multiline-cba-settings-support branch August 14, 2024 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants