-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add console command to toggle muzzle flashes #296
base: master
Are you sure you want to change the base?
Conversation
@@ -133,6 +134,25 @@ ConsoleCommand2 mesh_static_lighting_cmd{ | |||
"Toggle mesh static lighting calculation", | |||
}; | |||
|
|||
CallHook<void(rf::Entity&)> entity_update_muzzle_flash_light_hook{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it belongs to this file. obj_light.cpp
is related to generating vertex colors for meshes so they become lit (think of it as a lightmap but less expensive).
This feature has nothing to do with that. The function name starts with entity
so a good candidate would be entity.cpp
. I generally try to layout the code the same as it was in original RF code. From IDA you can see that this function is close to other entity related functions. Keeping things grouped based on RF modules make it easier to avoid collisions between patches
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I will move it.
Can servers have an option to disable this feature? Is it not a "cheat"? |
Technically sure, but no more than 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,
If rafalh wanted it that way, I could apply the same method rafalh already uses for |
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.
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. |
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 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 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) |
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 todamage_screen_flash
.This feature was requested by heat.