Add support for spellcast command's -x flag for all spells

[meszaros-lajos-gyorgy]

Currently only 4 spells (Heal, DetectTrap, Armor, LowerArmor) check whether the SPELLCAST_FLAG_NOSOUND flag is set. This addition adds support for all the sounds of all the spells.

[fredlllll]

this bit flag magic could be moved to a macro or function instead. makes it easier to understand

bool emitsSound = GET_EMITS_SOUND(m_flags);

also removes code repetition

[bsxf-47]

I think a member function would look much cleaner than a macro. I lean towards keeping the flag checks as they are, they are fairly clear.

[fredlllll]

there is a double negation in there (NO and !) so they are much harder to read than "getEmitsSound"

a macro would mean no extra call to a function for such a simple operation

[bsxf-47]

As far as I'm concerned, macros are just preprocessor crutches. I wouldn't worry about efficiency as the function would probably get inlined, and this is not a hot codepath anyway.

[meszaros-lajos-gyorgy]

I would also lean towards a method in the Spell class as opposed to adding a macro because macros are leaning the codebase back towards C and low level constructs while a method could be migrated more easily to a more modern language if needed.

[fredlllll]

could all or some of these ARX_SOUND calls get a conditional variant where you pass the bool in? yes of course its more efficient to put the if() around each call instead of doing a method call just to jump back immediately, but i dont think this would affect anything.

just easier than to have 500 if(emitsSound) clauses in each spell file.

alternatively, create a macro

#define conditional_call(condition, func) if(condition){func;}
conditional_call(emitsSound, ARX_SOUND_PlaySFX_loop(g_snd.SPELL_VISION_LOOP, &m_caster_pos, 1.f))

[bsxf-47]

If I understand correctly, you'd pass a bool to a function just to tell it if it should do anything or not? That doesn't sound good.
It looks like the flag is set at the start of a spell and not anywhere else, so why not check the bit value once and use a member variable for that? Maybe see if you can use Spell base class to simplify what you want to do.

[bsxf-47]

Alternatively, ARX_SOUND_PlaySFX_loop() could be wrapped in a member function, say playSoundLoop() that checks for the member variable away from the main code, if reducing clutter is the goal.

[fredlllll]

that playSoundLoop would do exactly what i proposed first with a function. the conditional macro thing would not add any extra function calls but make it a oneliner that is much easier to read. the files are long enough as they are

[bsxf-47]

I can't imagine reading that code a year later without having to inspect that macro and spending time on it. This doesn't feel like self-documenting code.

[bsxf-47]

There seems to be more going on than what the commit message says. Please split formatting changes into a separate commit.

[meszaros-lajos-gyorgy]

I have restored the whitespace here and a few other places as it's not necessary for this PR. I'll try to split my commits in the future according to what you wrote.