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

Common - Improve file checking error messages #10448

Merged
merged 6 commits into from
Nov 15, 2024
Merged

Conversation

LinkIsGrim
Copy link
Contributor

@LinkIsGrim LinkIsGrim commented Oct 23, 2024

When merged this pull request will:

  • Let me call people stupid when they don't read.

Example formatting:
image

image

Cases for errors are:

Client has additional ace addons, and compats are in additional addons // CDLC/RHS/etc loaded on client but not on server
Client has additional ace addons, and compats are not in additional addons (e.g. server has no medical loaded) // ace from different places

Version mismatch between server and client // what it says on the tin

Client has outdated addons across multiple ACE sources // conflicting install
Client has outdated addons across a single ACE source // corrupted install or bad repack

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}.

@LinkIsGrim LinkIsGrim added the kind/enhancement Release Notes: **IMPROVED:** label Oct 23, 2024
@LinkIsGrim LinkIsGrim added this to the 3.18.2 milestone Oct 23, 2024
@TheCandianVendingMachine
Copy link
Contributor

imo the fix should come first. users won't read the full error message, and we can maybe filter out more bogus issues if the order is changed

@LinkIsGrim LinkIsGrim changed the base branch from master to 40mmBrass November 2, 2024 22:52
@LinkIsGrim LinkIsGrim changed the base branch from 40mmBrass to master November 2, 2024 22:52
@LinkIsGrim LinkIsGrim changed the base branch from master to mgstuff November 2, 2024 22:54
@LinkIsGrim LinkIsGrim changed the base branch from mgstuff to master November 2, 2024 22:54
@LinkIsGrim
Copy link
Contributor Author

LinkIsGrim commented Nov 2, 2024

Had to unbreak git, no changes made in force push

Comment on lines +254 to +258
private _errorBuilder = switch (true) do {
case _versionMismatch: {_fnc_diagnose_versionMismatch};
case _addonMismatch: {_fnc_diagnose_addonMismatch};
};
(call _errorBuilder) params ["_title", "_reasonMsg", "_fixMsg", "_infoMsg", "_infoMsgLog"];
Copy link
Contributor

Choose a reason for hiding this comment

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

just a possibility

Suggested change
private _errorBuilder = switch (true) do {
case _versionMismatch: {_fnc_diagnose_versionMismatch};
case _addonMismatch: {_fnc_diagnose_addonMismatch};
};
(call _errorBuilder) params ["_title", "_reasonMsg", "_fixMsg", "_infoMsg", "_infoMsgLog"];
(switch (true) do {
case _versionMismatch: _fnc_diagnose_versionMismatch;
case _addonMismatch: _fnc_diagnose_addonMismatch;
}) params ["_title", "_reasonMsg", "_fixMsg", "_infoMsg", "_infoMsgLog"];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not the biggest fan of it

@LinkIsGrim
Copy link
Contributor Author

I've asked two unrelated test subjects who don't even play Arma to take a look and they said it seems good and could probably fix it themselves (at least better than the old ones).

Copy link
Contributor

@johnb432 johnb432 left a comment

Choose a reason for hiding this comment

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

From a logic point of view, LGTM

@LinkIsGrim LinkIsGrim merged commit d8b3cda into master Nov 15, 2024
3 checks passed
@LinkIsGrim LinkIsGrim deleted the linkIsReallyAnnoyed branch November 15, 2024 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants