General - Make type syntax in header more consistent #10681

Timi007 · 2025-01-20T13:50:28Z

When merged this pull request will:

Make type syntax in header more consitent
Make all types UPPERCASE
Append lowercase s for plural of type
Use or for multiple options
Unify types (e.g., BOOLEAN -> BOOL, POSITION -> ARRAY)
Unify optional argument syntax
Fix some spelling
Replace <OPTIONAL> with (default: ...)
Move (default: ...) after <TYPE>
Improve validation of argument type for documentation

IMPORTANT

If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
Development Guidelines are read, understood and applied.
Title of this PR uses our standard template Component - Add|Fix|Improve|Change|Make|Remove {changes}.

Discussion

Nested types

Currently, there are two syntax versions of how the types inside an array are represented:

ARRAY<NUMBER>, ARRAY<ARRAY<OBJECT, NUMBER>>
ARRAY of ARRAYs of OBJECTs and NUMBERs
Which syntax should be used?

Enclosure

Furthermore, should each type be enclosed in their own <TYPE> when multiple types are an options?

<CODE or STRING>
<CODE> or <STRING>

Plural

Should types be pluralized? If so, how?

No plural: <ARRAY of NUMBER>
Plural, lowercase s: <ARRAY of NUMBERs>
Plural, uppercase S: <ARRAY of NUMBERS>

Andx667 · 2025-01-21T16:48:32Z

My thoughts on the discussion:

Nested types
ARRAY<NUMBER>, ARRAY<ARRAY<OBJECT, NUMBER>> seems precise and unambiguous to me.

Enclosure
<CODE> or <STRING> is my favorite here. It's consistent with the other types because its always <type> not < something or something>

Plural
I'm kinda torn on this. Keeping with the consistency argument from before I would choose no plural. But I feel that the added s make it's a lot more clear what is meant.

No plural: <ARRAY of NUMBER>

Plural, uppercase S: <ARRAY of NUMBERS>

johnb432

Thanks for doing this, I'm glad someone had the courage to do it.

docs/wiki/framework/interactionMenu-framework.md

johnb432 · 2025-02-16T07:55:17Z

addons/tagging/functions/fnc_createTag.sqf

@@ -6,11 +6,12 @@
 * Arguments:
 * 0: Position ASL <ARRAY>
 * 1: Vector dir and up <ARRAY>
- * 2: Colour of the tag (valid colours are black, red, green and blue or full path to custom texture) <STRING>
+ * 2: Color of the tag (valid colors are black, red, green and blue or full path to custom texture) <STRING>


Imo, the spelling changes should be reverted, we use both American and British spelling in ACE.

I have a code spell checker enabled in VS Code that is set to American English. So I briefly went over all the warnings of the spell checker. Why not just specify and set it to either American English or British English? This way, we can maybe add the Code Spell Checker to the VS Code recommended extensions and have fewer overall spelling mistakes.

I do agree with you. However, not everyone uses VS Code (such as myself), but that's a relatively small issue.
We need to discuss it internally to find out which English is preferred - I just didn't want to alienate anyone by choosing a specific version, but the benefits outweigh the drawbacks.

should be possible to add words to dictionary
maybe in the .vcode/settings.json?

American English is the standard that we have chosen to go with.

addons/common/functions/fnc_getWeaponState.sqf

addons/artillerytables/functions/fnc_rangeTableUpdate.sqf

johnb432 · 2025-02-16T09:25:47Z

Discussion

[...]

For me:

ARRAY<NUMBER>, ARRAY<ARRAY<OBJECT, NUMBER>>
<CODE or STRING>
Plural, lowercase s: <ARRAY of NUMBERs>

addons/common/functions/fnc_unloadUnitWeapon.sqf

addons/common/functions/fnc_syncedEvent.sqf

Co-authored-by: johnb432 <[email protected]>

TheCandianVendingMachine · 2025-03-04T17:52:09Z

Would you be willing to add this standardization to our coding guidelines wiki page? Probably in section 3.2; enumerating the allowed types, specifying how to do optional arguments, etc.. Currently the page has some examples, but it isn't spanning, and a concrete ruleset would probably clear things up (I know I would like this).

TheCandianVendingMachine

sorry for bikeshedding a bit. imo, we should specify subtypes of posASL/AGL/ATL/2d but I am fine without

TheCandianVendingMachine · 2025-03-05T22:11:50Z

addons/ai/functions/fnc_garrison.sqf

@@ -4,7 +4,7 @@
 * Garrison function used to garrison AI inside buildings.
 *
 * Arguments:
- * 0: The building(s) nearest this position are used <POSITION>
+ * 0: The building(s) nearest this position are used <ARRAY>


wouldnt ARRAY of NUMBERs be more consistent?

Now that I think about it, I would interpret <ARRAY of NUMBERs> as a list consisting of any amount of numbers. <ARRAY> would be generic. Technically, we would need to write <ARRAY<NUMBER, NUMBER, NUMBER>> for a 3D position or an array containing exactly 3 numbers. I don't know what the best solution here would be.

I think position is better than array, the biwiki uses position, and even specifies which kind (ASL, ATL)

Suggested change

* 0: The building(s) nearest this position are used <ARRAY>

* 0: The building(s) nearest this position are used <POSITION>

addons/common/functions/fnc_errorMessage.sqf

addons/cargo/functions/fnc_getDelayItem.sqf

Timi007 · 2025-03-05T22:43:31Z

sorry for bikeshedding a bit. imo, we should specify subtypes of posASL/AGL/ATL/2d but I am fine without

IMO, the subtype should be specified in the argument description. But I'm open to accept using the Position subtypes instead of <ARRAY>.

johnb432 · 2025-03-06T05:44:10Z

sorry for bikeshedding a bit. imo, we should specify subtypes of posASL/AGL/ATL/2d but I am fine without

IMO, the subtype should be specified in the argument description. But I'm open to accept using the Position subtypes instead of <ARRAY>.

I also think they should be part of the description.

TheCandianVendingMachine · 2025-03-18T21:18:06Z

It seems like most people who answered are for the position types, so I think changing it to them would be ideal. Not blocking, but if it doesn't make much labour I think it's worth while

PabstMirror · 2025-03-22T17:07:55Z

addons/zeus/functions/fnc_moduleGarrison.sqf

@@ -5,7 +5,7 @@
 *
 * Arguments:
 * 0: Module logic <OBJECT>
- * 1: Position of the module <POSITION>
+ * 1: Position of the module <ARRAY>


Suggested change

* 1: Position of the module <ARRAY>

* 1: Position of the module <POSITION>

PabstMirror · 2025-03-22T17:08:05Z

addons/explosives/functions/fnc_placeExplosive.sqf

@@ -5,7 +5,7 @@
 *
 * Arguments:
 * 0: Unit <OBJECT>
- * 1: Position to place explosive <POSITION>
+ * 1: Position to place explosive <ARRAY>


Suggested change

* 1: Position to place explosive <ARRAY>

* 1: Position to place explosive <POSITION>

PabstMirror · 2025-03-22T17:08:42Z

addons/ai/functions/fnc_garrisonMove.sqf

- *    0: Unit needing to be placed <UNIT>
- *    1: Position the unit need to be placed at <POSITION>
+ *    0: Unit needing to be placed <OBJECT>
+ *    1: Position the unit need to be placed at <ARRAY>


Suggested change

* 1: Position the unit need to be placed at <ARRAY>

* 1: Position the unit need to be placed at <POSITION>

I think these are the only 4 changes to revert and then we can merge it

Timi007 · 2025-03-25T13:49:04Z

If we introduce now a position type, how should it be written? I wish GitHub had a poll feature. Could you please vote with reactions? Multiple votes allowed.

❤️ Just <POSITION> and the subtype e.g., ATL should be specified in the description
🚀 <POSITION3D>, <POSITION2D>, <POSITIONAGL>, <POSITIONASL>, <POSITIONATL>, <POSITIONWORLD>, <POSITIONRELATIVE>
👀 <POS3D>, <POS2D>, <POSAGL>, <POSASL>, <POSATL>, <POSWORLD>, <POSREL>

DartRuffian · 2025-03-25T14:29:34Z

Feels weird to have POSITION in the brackets, since it's not a type. Fwiw, I've always done: 0: Position format ASL <ARRAY> in my own projects. The type is still an array, making it POSITION doesn't make sense IMO. Same logic as doing 0: Unit <OBJECT> and not 0: Something <UNIT>.

Timi007 · 2025-03-25T14:50:22Z

Feels weird to have POSITION in the brackets, since it's not a type. Fwiw, I've always done: 0: Position format ASL <ARRAY> in my own projects. The type is still an array, making it POSITION doesn't make sense IMO. Same logic as doing 0: Unit <OBJECT> and not 0: Something <UNIT>.

We had this discussion a few comments above.

Since I'm unsure what the majority now wants, here's another poll. Multiple votes allowed.

👎 Position arguments should have type <ARRAY>. The details about the position subtype (e.g., ATL) should be in the description.
👍 Use own type for positions (see other poll)

Timi007 added 13 commits January 20, 2025 13:11

Fix spelling and casing of types

0efa6e3

Fix spelling and casing

5b866f3

Fix syntax of multi-types and plural of types

52a96b7

POSITION -> ARRAY

a1f0762

Remove implicit nil

3bed2b5

Fix type

64842dc

Remove optional type

5399677

Rename BOOLEAN to BOOL

caab52b

Add helper script

63dbd77

Add docs

dec4457

Remove optional

97b370b

Unify optional argument syntax

1baecd6

Fix spelling of public functions

7e7e7b9

BaerMitUmlaut changed the title ~~General - Make type syntax in header more consitent~~ General - Make type syntax in header more consistent Feb 10, 2025

BaerMitUmlaut added the ignore-changelog Release Notes: Excluded label Feb 10, 2025

johnb432 reviewed Feb 16, 2025

View reviewed changes

johnb432 added 3 commits February 16, 2025 09:09

Merge branch 'master' into pr/10681

e7f895f

Fix for multiple types

c85b3be

Update fnc_getDelayItem.sqf

69acb54

johnb432 added the kind/documentation label Feb 16, 2025

Timi007 added 7 commits February 16, 2025 15:12

Apply review findings

1df2c5f

Fix more findings

c9d0cb5

Fix type

f24a310

Move default value after TYPE

24e5f53

Do not allow for | in types and allow nested types using < >

98a652f

Fix spelling

3d7a5d0

Improve validation of argument type for documentation

d120067

johnb432 reviewed Feb 21, 2025

View reviewed changes

addons/common/functions/fnc_unloadUnitWeapon.sqf Outdated Show resolved Hide resolved

johnb432 reviewed Feb 24, 2025

View reviewed changes

addons/common/functions/fnc_syncedEvent.sqf Outdated Show resolved Hide resolved

johnb432 and others added 2 commits February 24, 2025 19:08

Merge branch 'master' into pr/10681

0e5e354

Apply suggestions from code review

663f783

Co-authored-by: johnb432 <[email protected]>

Standardize header syntax

605a1f7

TheCandianVendingMachine requested changes Mar 5, 2025

View reviewed changes

Merge branch 'master' into pr/10681

6fb55d8

PabstMirror reviewed Mar 22, 2025

View reviewed changes

PabstMirror added this to the 3.19.3 milestone Mar 22, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

General - Make type syntax in header more consistent #10681

General - Make type syntax in header more consistent #10681

Timi007 commented Jan 20, 2025 •

edited

Loading

Andx667 commented Jan 21, 2025 •

edited

Loading

johnb432 left a comment

johnb432 Feb 16, 2025

Timi007 Feb 16, 2025

johnb432 Feb 21, 2025

PabstMirror Feb 21, 2025

johnb432 Feb 24, 2025

johnb432 commented Feb 16, 2025

Discussion

TheCandianVendingMachine commented Mar 4, 2025

TheCandianVendingMachine left a comment

TheCandianVendingMachine Mar 5, 2025

Timi007 Mar 5, 2025

BrettMayson Mar 17, 2025

PabstMirror Mar 22, 2025

Timi007 commented Mar 5, 2025 •

edited

Loading

johnb432 commented Mar 6, 2025

TheCandianVendingMachine commented Mar 18, 2025

PabstMirror Mar 22, 2025

PabstMirror Mar 22, 2025

PabstMirror Mar 22, 2025

PabstMirror Mar 22, 2025

Timi007 commented Mar 25, 2025

DartRuffian commented Mar 25, 2025

Timi007 commented Mar 25, 2025 •

edited

Loading

	* 0: The building(s) nearest this position are used <ARRAY>
	* 0: The building(s) nearest this position are used <POSITION>

	* 1: Position of the module <ARRAY>
	* 1: Position of the module <POSITION>

	* 1: Position to place explosive <ARRAY>
	* 1: Position to place explosive <POSITION>

	* 1: Position the unit need to be placed at <ARRAY>
	* 1: Position the unit need to be placed at <POSITION>

General - Make type syntax in header more consistent #10681

Are you sure you want to change the base?

General - Make type syntax in header more consistent #10681

Conversation

Timi007 commented Jan 20, 2025 • edited Loading

IMPORTANT

Discussion

Nested types

Enclosure

Plural

Andx667 commented Jan 21, 2025 • edited Loading

johnb432 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnb432 commented Feb 16, 2025

Discussion

TheCandianVendingMachine commented Mar 4, 2025

TheCandianVendingMachine left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Timi007 commented Mar 5, 2025 • edited Loading

johnb432 commented Mar 6, 2025

TheCandianVendingMachine commented Mar 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Timi007 commented Mar 25, 2025

DartRuffian commented Mar 25, 2025

Timi007 commented Mar 25, 2025 • edited Loading

Timi007 commented Jan 20, 2025 •

edited

Loading

Andx667 commented Jan 21, 2025 •

edited

Loading

Timi007 commented Mar 5, 2025 •

edited

Loading

Timi007 commented Mar 25, 2025 •

edited

Loading