Fire - Add ability to customize screams when on fire

[DartRuffian]

When merged this pull request will:

• Add ability to customize on fire screams for different units

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

[DartRuffian]

Docs could probably use some better wording

[johnb432]

I don't like that this is a purely scripted function: mods need to absolutely call functions to make use of this functionality, when a config property would be much better suited for mods.
Imo a config property should either outright replace the scripted adding or, at the very least, complement the existing. I don't really see the point of the scripted adding - is it for mission makers?

[DartRuffian]

I went for a function over config because I couldn't think of a good way to do the lookups without having a hashmap where most of the entries just have the same value. Not that it matters a ton, it just felt nicer imo

And mission makers being able to customize stuff is always nice

[johnb432]

without having a hashmap where most of the entries just have the same value

I'll try to come up with something to add onto this PR.

[johnb432]

I'll try to come up with something to add onto this PR.

I'm tempted to say: scrap the functions altogether, inform users about the hashmap but don't mark it as API and add a config property to CAManBase, an array containing the screams.

I don't think such a niche feature needs proper API, apart from the config properties, which only exists to make it easy for mods to implement.
The absence of proper API would allow mission makers (or others) to edit the hashmap as they see fit. As of now, the function you provide doesn't update the scream array for units that inherit from the class you provide. E.g. you want to update screams for all units, so editing CAManBase, but the existing hashmap keys won't have their respective values updated.

I'd like to hear other opinions though before changes are made to the PR.

[DartRuffian]

Works for me

My reasoning for not inheriting screams was just different voices

[johnb432]

My reasoning for not inheriting screams was just different voices

At that point we'd be better off using voices instead of unit types to determine the screams, no?
The issue with that could be the no voice from ace - not sure if units having that voice should scream or not.

[rautamiekka]

The issue with that could be the no voice from ace - not sure if units having that voice should scream or not.

They should, otherwise NoVoice would add yet another problem (I personally use NoVoice on Player units who'll use ACRE2 for voice chat, and units like isolated guards, but very often to decrease confusion added by other units like civilians speaking, too) that'll require balancing between not being confused by AI speaking and someone getting burned without knowing, which is unacceptable.

[DartRuffian]

Reasons rautamiekka pointed out were why I just did it class based

[DartRuffian]

I'd like to hear other opinions though before changes are made to the PR.

I'd like to get this finished up, was there anything else needing deciding? Also just to make sure I understood correctly, you're saying to remove fnc_addScreamSounds and fnc_getScreams completely and just have the hashmap of ["class": [screams]] correct?

Then fnc_burnReaction would just be something like:

private _scream = selectRandom (GVAR(screams) getOrDefault [typeOf _unit, GVAR(screams) get "CAManBase"]);

And instead of manually specifying ACE's sounds in XEH_preInit it'd just be:

GVAR(screams) = createHashMapFromArray [["CAManBase", getArray (configFile >> "CfgVehicles" >> "CAManBase" >> QGVAR(screams))]];

If I'm understanding correctly

[johnb432]

If I'm understanding correctly

It would be something similar to that, yes. However, this would not easily permit inheritance. Still considering another solution.

[johnb432]

There are several options for better incorporating inheritance:

1. private _sound = selectRandom (getArray (configOf _unit >> QGVAR(screams)));
   We add the GVAR(screams) entry to CAManBase and assume that all units that scream inherit from CAManBase (I think that's guaranteed by ace_fire, so it should be fine). Specific classes would be changed by mod makers.

2. private _sound = selectRandom (GVAR(screams) getOrDefault [typeOf _unit, getArray (configOf _unit >> QGVAR(screams))]);
   We'd also add GVAR(screams) entry to CAManBase. The benefit of the hashmap is that mission makers can edit the sounds, but I don't really see the point of it, as I doubt mission makers are going to want to change this specifically without making a mod patch directly.

3. Use CfgVoice instead, so it could be based off of voices instead. Add a default to the Base class, so that all voices should have it. Downside is that numerous no-voice-voices from 3rd party mods don't inherit from Base (ACE' could be easily accounted for, but others less so).

[LinkIsGrim]

Why not a config lookup?

EDIT: Reviewed before reading discussion.