rafalh / dashfaction

A community patch for the Red Faction (2001) FPS game
https://www.factionfiles.com/ff.php?action=file&id=6252
Mozilla Public License 2.0
61 stars 11 forks source link

Add console command to toggle muzzle flashes #296

Open GooberRF opened 1 month ago

GooberRF commented 1 month ago

This PR adds the muzzle_flash command, which toggles whether the game should create muzzle flash dynamic lights.

This is separate from the global Dynamic Lighting toggle (in Video options in stock game) - that feature forces all dynamic lights off, including projectiles, CTF flags, amp/invuln, etc. The muzzle_flash option only affects muzzle flashes, and is very useful in multiplayer where constantly blinking muzzle flashes can get extremely annoying/headache-inducing but rendering dynamic lights for CTF flags and amp, etc. is preferred. I view this as an accessibility feature, similar to damage_screen_flash.

This feature was requested by heat.

is-this-c commented 3 weeks ago

Can servers have an option to disable this feature? Is it not a "cheat"?

GooberRF commented 3 weeks ago

Is it not a "cheat"?

Technically sure, but no more than damage_screen_flash or things like Direct Input/Linear Pitch. There has to be a line drawn somewhere on things like that, and I don't think this crosses it.

It's also worth considering for this and other features that... effectively everyone runs Dash. As a result, it's not fully accurate to consider things that are available to all Dash users as "cheats" - everyone has access to them. That said, Saving Enabled for example, for obvious reasons, even though it wouldn't be a "cheat", server operators should be able to disable. A desire to disable blinking muzzle flash lights however, I don't see as necessary to allow a server to veto.

Can servers have an option to disable this feature?

If rafalh wanted it that way, I could apply the same method rafalh already uses for Saving Enabled and FOV caps (and I use for some other options in #285 ), but it's my view that it's unnecessary.

nickalreadyinuse commented 3 weeks ago

Technically sure, but no more than damage_screen_flash or things like Direct Input/Linear Pitch. There has to be a line drawn somewhere on things like that, and I don't think this crosses it.

I generally support this feature being added, but I could easily be convinced that it ought to be server controlled. Damage screen flash likely should have been implemented similarly. In an ideal world, many options and tweaks would not be usable outside of Dash servers as well, but the leg work there may not be worth it.

It's also worth considering for this and other features that... effectively everyone runs Dash. As a result, it's not fully accurate to consider things that are available to all Dash users as "cheats" - everyone has access to them.

Nah, this isn't a good argument.

Dash is the most used client for sure. But we need to be careful about using ease of access as a contributing factor towards determining what's a cheat and what isn't. It's true that nearly everyone uses Dash, but not everyone uses a Dash which enables them to pick and choose what actions produce dynamic light (it's all or nothing currently). Toggling dynamic lights has always been a choice between having more information available to you as a player or less visual distraction in fights. In TDM and 1on1, you don't really lose much by turning it off.

But historically, this choice was - at least perceptually - a fairly significant one in higher level CTF games. It's often the difference between knowing which route a flag runner took, whether they backpedaled, if they're waiting or camping in some spot, etc. It's potentially a lot of information, and you'd be really stupid to turn it off unless your game sense is extremely good. This PR, comparatively, is having your cake and eating it too. By using it you'd still lose out on some visual information compared to the full setting (the dynamic light of enemies firing on floors above you as one example) but in most of the scenarios I can think of sound provides the same information generally.

All that said, this setting was always problematic because players being able to choose whether this effect is on, off, or some mixture of both (as with this PR) creates a situation where not all players are playing with the same rules. This PR complicates that situation even further. I would argue that the server should almost always dictate what the "rules" of the game actually are to the greatest extent possible. And so in an ideal world, the server would be able to control the client's dynamic light setting entirely (on/off/mixed as in this PR). But I'd guess that the work that would go into making that possible would be wasted, because I doubt anyone is going to run a server enforcing something like this nowadays.

And so finally, my 2c are this: if it's a console command most users will never see or interact with it. As an accessibility feature, which I think is potentially valid (or at least as valid as damage screen flash) it should be in the Dash setup options as a toggle there at minimum. Damage screen flash should be there too, and maybe the color should be configurable (maybe someone wants purple flash or yellow or whatever instead). It doesn't make good sense for accessibility features to be buried away in the console; in fact it seems quite wrong really. It might also make sense for this to have some kind of MP/SP filter or mask, so that players could choose to have it on in SP but off in MP, or whatever.

GooberRF commented 3 weeks ago

It might also make sense for this to have some kind of MP/SP filter or mask, so that players could choose to have it on in SP but off in MP, or whatever.

I really like this idea - honestly, it's been a pain point for me to have to choose between on/off globally for settings like Show First Person Weapons and nearest_neighbor_filtering , both of which I would prefer OFF in all cases in SP but ON in all cases in MP, and it's very annoying to have to remember to switch when going between SP/MP. This toggle would be the same.

Using an enum as below rather than a bool for this (and similar options) would make that quite easy, and this would be an intuitive, legacy-compatible approach that could be applied to all other current toggles (at least the ones that apply to both SP and MP) so they all behave in a standard and expected way:

("on" in this context would refer to the application of a deviation from the standard behaviour - so in the case of the command in this PR, "on" would mean no muzzle flash lights, since the standard behaviour is for muzzle flash lights to be shown. At that point, the command probably should be disable_muzzle_flash_lights or similar)

Syntax could be:

The server could still force "risky" options off, and that would overrule the client's desire for it to be on.

(the launcher config option should probably still just be a 1/0 on/off toggle for simplicity and since for most users, maintaining the same value between SP/MP is fine)

(I don't disagree with anything else you've said, this was just the only part I had time to reply to right now)