Skip to content

General - Various small code fixes #11144

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.

Sign up for GitHub

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

Jump to bottom
Merged
PabstMirror merged 8 commits into from
Dec 13, 2025
Merged

General - Various small code fixes #11144

PabstMirror merged 8 commits into from
Dec 13, 2025

Conversation

@PabstMirror
Copy link
Contributor

@PabstMirror PabstMirror commented Nov 22, 2025

@PabstMirror PabstMirror added this to the 3.20.2 milestone Nov 22, 2025
@PabstMirror PabstMirror added the kind/bug-fix Release Notes: **FIXED:** label Nov 22, 2025
LinkIsGrim
LinkIsGrim reviewed Nov 22, 2025
View reviewed changes
@PabstMirror
Copy link
Contributor Author

PabstMirror commented Nov 25, 2025

What is best way to handle

 * Return Value:
 * Success? <BOOL>
 *
 * Example:
 * [bob, true] call ace_medical_fnc_setUnconscious;
 */

// only run this after the settings are initialized
if !(EGVAR(common,settingsInitFinished)) exitWith {
    EGVAR(common,runAtSettingsInitialized) pushBack [FUNC(setUnconscious), _this];
};

this returns number if called early
should it return true, false or something else?

@LinkIsGrim
Copy link
Contributor

LinkIsGrim commented Nov 26, 2025

Typically BIS fncs return nil for such cases IIRC

@PabstMirror PabstMirror modified the milestones: 3.20.2, 3.20.3 Dec 1, 2025
PabstMirror
PabstMirror commented Dec 4, 2025
View reviewed changes
PabstMirror
PabstMirror commented Dec 12, 2025
View reviewed changes
addons/common/functions/fnc_removeSyncedEventHandler.sqf
_data params ["", "", "", "_eventId"];

[_eventId] call CBA_fnc_removeEventHandler;
[_name, _eventId] call CBA_fnc_removeEventHandler;
Copy link
Contributor Author

@PabstMirror PabstMirror Dec 12, 2025

Choose a reason for hiding this comment

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

this function has never worked, we could also just remove it

Copy link
Contributor

@LinkIsGrim LinkIsGrim Dec 13, 2025

Choose a reason for hiding this comment

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

Deprecate, remove in next major version?

Copy link
Contributor Author

@PabstMirror PabstMirror Dec 13, 2025

Choose a reason for hiding this comment

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

I guess it's not doing much harm
I was more worried it could cause problems if people had been calling and it starts working
but I guess we'll find out

johnb432
johnb432 reviewed Dec 12, 2025
View reviewed changes
PabstMirror and others added 3 commits December 13, 2025 12:15
@PabstMirror @johnb432
Update addons/artillerytables/functions/fnc_adjustFire.sqf
078577b 
Co-authored-by: johnb432 <58661205+johnb432@users.noreply.github.com>
@PabstMirror @johnb432
Update addons/interaction/functions/fnc_getGlassDoor.sqf
016c0e8 
Co-authored-by: johnb432 <58661205+johnb432@users.noreply.github.com>
@PabstMirror
Update coding-guidelines.md
0d8271c
PabstMirror
PabstMirror commented Dec 13, 2025
View reviewed changes
docs/wiki/development/coding-guidelines.md
To keep the syntax in the header consistent, we set some rules:
1. Each argument must have a type that is enclosed in brackets `<TYPE>`. The type must be in UPPERCASE.
2. Use one of the following types only: `OBJECT`, `NUMBER`, `STRING`, `BOOL`, `ARRAY`, `CONTROL`, `DISPLAY`, `CONFIG`, `CODE`, `ANY`, `LOGIC`, `SIDE`, `GROUP`, `HASHMAP`, `NAMESPACE`, `LOCATION`, `TEXT`.
2. Use one of the following types only: `OBJECT`, `NUMBER`, `STRING`, `BOOL`, `ARRAY`, `CONTROL`, `DISPLAY`, `CONFIG`, `CODE`, `ANY`, `SIDE`, `GROUP`, `HASHMAP`, `NAMESPACE`, `LOCATION`, `STRUCTUREDTEXT`, `NIL`.
Copy link
Contributor Author

@PabstMirror PabstMirror Dec 13, 2025

Choose a reason for hiding this comment

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

removed logic (just object)
TEXT -> STRUCTUREDTEXT
added nil

LinkIsGrim
LinkIsGrim approved these changes Dec 13, 2025
View reviewed changes
addons/ai/functions/fnc_garrison.sqf
Copy link
Contributor

@LinkIsGrim LinkIsGrim Dec 13, 2025

Choose a reason for hiding this comment

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

Could we deprecate this maybe? I think most people who care are going to use their preferred AI mod's garrison function

Copy link
Contributor Author

@PabstMirror PabstMirror Dec 13, 2025

Choose a reason for hiding this comment

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

ace_zeus_fnc_moduleGarrison uses this
as far as I know it works ok, so I would prefer to keep it

addons/common/functions/fnc_removeSyncedEventHandler.sqf
_data params ["", "", "", "_eventId"];

[_eventId] call CBA_fnc_removeEventHandler;
[_name, _eventId] call CBA_fnc_removeEventHandler;
Copy link
Contributor

@LinkIsGrim LinkIsGrim Dec 13, 2025

Choose a reason for hiding this comment

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

Deprecate, remove in next major version?

@PabstMirror PabstMirror modified the milestones: 3.20.3, 3.20.2 Dec 13, 2025
@PabstMirror PabstMirror merged commit e13339f into master Dec 13, 2025
3 checks passed
@PabstMirror PabstMirror deleted the hFix1 branch December 13, 2025 22:12
@PabstMirror PabstMirror mentioned this pull request Dec 13, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@johnb432 johnb432 johnb432 left review comments

@BrettMayson BrettMayson BrettMayson approved these changes

@LinkIsGrim LinkIsGrim LinkIsGrim approved these changes

Assignees

No one assigned

Labels

kind/bug-fix Release Notes: **FIXED:**

Projects

None yet

Milestone

3.20.2

Development

Successfully merging this pull request may close these issues.

5 participants

@PabstMirror @LinkIsGrim @BrettMayson @johnb432