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.

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

Check callout validity #380

Closed
wants to merge 3 commits into from
Closed

Check callout validity #380

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e060e00
Check callout validity
rekterakathom Feb 18, 2024
ff4ab4f
Check validity when caching
rekterakathom Feb 18, 2024
4aaff2c
Switch to regex, add mp3
rekterakathom Apr 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions addons/main/functions/fnc_doCallout.sqf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,18 @@ if (isNil "_cachedSounds") then {
_cachedSounds = getArray (_protocolConfig >> _callout);
private _deleted = false;
{
private _sound = _x;
if (_sound == "") then {
private _sound = toLowerANSI _x; // playSound3D is case-insensitive

// File extension must exist for playSound3D to work
if (_sound == "" || !(".ogg" in _sound || ".wss" in _sound || ".wav" in _sound)) then {
Copy link
Contributor

Choose a reason for hiding this comment

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

Considering in is rather slow on strings, RegEx might be faster (it's been faster before).

Alternatively, assuming there can't be anything after the extension for the sound to be considered legal, doing an end-of-string select may be even faster than a full-string RegEx.

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 think overengineering will result in a slowdown.

String to benchmark with (just smashed my keyboard):
_sound = "a3479tghuaighreuyigvhbierlagheriap9tg745ra3heugjreivbugav7843fgrhbg864753g453hqb76453q9g54rgbh6q793g84gfcy43q.wav"

Benchmarks:
!(".ogg" in _sound || ".wss" in _sound || ".wav" in _sound); ~0.0013ms
(_sound regexFind ["(.*?)\....", 0]) isEqualTo [] ~0.0030ms

End-of-string select (not even doing the compare)
_end = _sound select [((count _sound) - 4)] ~0.0013ms

An argument could be made for the RegEx, because even though it is slower, it will match any file extension. But I don't believe the engine even supports that many formats, mp3 maybe? But mp3 could just be added to the list of checks if it is supported.

Copy link
Contributor

Choose a reason for hiding this comment

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

Try regexMatch instead of regexFind if you don't need the match itself, and if you check just the extension it can be done by the .+?\.(?:ogg|wss|wav)$/io pattern (or .+\.(?:ogg|wss|wav)$/io if the former is somehow slower, or even more weirdly .++\.(?:ogg|wss|wav)$/io).

I admit I can't test it right now and select's docs make me suspicious, but it should support end-of-string/-array selections through negative indexes since 'Arma 3 v2.14':

private _end = _sound select [-4]

Copy link
Owner

Choose a reason for hiding this comment

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

As I recall, all callout sounds in vanilla are in .ogg

Copy link
Collaborator

Choose a reason for hiding this comment

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

if we want to go after that, the fastest here is to check if a . is in the file path, though I don't think the speed is needed.
However, the question is if we maybe check if a file with the "supported" file endings exists so that we don't "filter" out still possible valid callouts and increase mod support.

Copy link
Contributor

Choose a reason for hiding this comment

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

True.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An alternative method would just be to run fileExists on it. That would guarantee that everything is valid, but it might be an order of magnitude slower.

I'd appreciate it if somebody could run the benchmarks too for the current method, the regexMatch method and the fileExists method. I think regexMatch might be an improvement over the current one (even with a slight performance regression), as it's a much more compact statement and it's easier to add more filetypes (maybe mp3).

_sound = objNull;
_deleted = true;
} else {
if (_sound select [0, 1] != "\") then {
_sound = (getArray (configFile >> "CfgVoice" >> _speaker >> "directories") select 0) + _sound;
};
};

_cachedSounds set [_forEachIndex, _sound];
} forEach _cachedSounds;

Expand Down
Loading