MrAlaux / Nugget-Doom

Nugget Doom is a fork of Woof! with additional features.
GNU General Public License v2.0
60 stars 3 forks source link

sp_chat doesn't block some key binds #75

Closed d-bind closed 10 months ago

d-bind commented 10 months ago

Singleplayer chat is great for recording feedback runs! However, at the moment it's also dangerous because it doesn't block things like the level restart hotkey, which might be bound to a comma. Though I haven't tested throroughly if it's a specific key or a specific command problem.

MrAlaux commented 10 months ago

I can't believe that I'm not the only one using sp_chat...

Anyways, if you know of any other troublesome binds, could you list them?

d-bind commented 10 months ago

To be honest, the only reason I found the feature was that I needed to set up Woof to test something, and was diffing against Nugget config. Why not enable it by default? Demo compat? Anyway, I think all the menu binds get let through, though I'm not sure. E.g. pressing ESC will just bring up the menu on top of message in progress, where it should probably cancel the typing instead.

MrAlaux commented 10 months ago

Why not enable it by default? Demo compat?

It's simply not standard behavior. Woof doesn't do it, nor does it have a setting for it, and I usually stick to Woof's defaults. Demo compat shouldn't be an issue, since the inputs are seemingly fully ignored, so what you see during recording is what you see during playback; that is, you don't see the chat typing in the demo, but the actions bound to the keys pressed during typing aren't executed, because those key presses are "eaten" by the chat.

I could enable it by default. I'll have to see.

Anyway, I think all the menu binds get let through, though I'm not sure.

Guess I'll have to check the code.

E.g. pressing ESC will just bring up the menu on top of message in progress, where it should probably cancel the typing instead.

I think that's by design. I was curious about other inputs more akin to the level restart.

MrAlaux commented 10 months ago

From what I've seen, there are some inputs that may be valid for the Chat (i.e. characters that can be typed into it) but also bound to some action (e.g. player movement).

In some cases (probably most), the Chat eats the input first and so its bound action isn't executed.

Some inputs, though, are checked for before they're processed by the Chat. This is the case at least with the inputs for level restart, multiplayer spy mode, taking screenshots, opening the menu and, if the menu is open, the menu navigation inputs. There may be other inputs.

https://github.com/MrAlaux/Nugget-Doom/commit/67e31e4c6d6c9c36e3a2fd51523170d9584a12df makes it so that spy mode and level restart are ignored.

I think I'll leave the screenshot and menu inputs alone. If you want to clear the message like you expect Escape to do, you can set up an empty Chat macro (Options -> Chat Strings), then use it by holding Alt and pressing the number corresponding to the macro. Otherwise, you could just send the message; it should make no difference if you're recording a video in singleplayer, since what you were typing could be seen anyways.

About recording: when you say "recording feedback runs", are you talking about recording videos? Chat messages aren't saved in demos, so the only possibility I see is that you type in Chat while recording a video.

d-bind commented 10 months ago

Thanks. I don't think menus / ESC are a big deal, it's just what would have been consistent with e.g. GZDoom or Quake. Edit: oh, you might want to at least block input_menu_nextlevel too. Like I said, I happened to have these bound to < and >.

Chat messages aren't saved in demos, so the only possibility I see is that you type in Chat while recording a video.

Yes, video recording. I'm spoiled by the Quake demo format, which records SP chat, allows starting the recording mid-map or after loading a save, and stores server messages instead of inputs. I don't feel FDAs are nearly as useful for giving feedback, so I'm making the best of what exists.

This isn't a feature request, I realize this is beyond the scope, and if people wanted it, it would have happened in some port by now. Still, out of curiosity, is there anything actually preventing storing chat in demos as a sort of metadata? I may be incorrect, but IIRC, in earlier DSDA verions you could record a strict demo with freelook, and it would play back in DSDA with freelook, and in other ports without. Again though, don't bother implementing it on my account, since I'd likely still prefer videos regardless.

MrAlaux commented 10 months ago

Edit: oh, you might want to at least block input_menu_nextlevel too. Like I said, I happened to have these bound to < and >.

I think that's one of the inputs that go through the Chat first, so it'd be ignored already unless it were bound to a key that the Chat doesn't process. I'll check again.

An example to clarify: if you have the previous and next weapon inputs bound to the mouse wheel and use the mouse wheel while in the Chat, the weapon switch will be performed as the Chat doesn't use the wheel input. However, if you bind those inputs to Q and E, and type those while in the Chat, the Chat will "eat" them and no switch will be performed.

What this means is that it shouldn't matter what key you have your input bound to, because if the input gets through the Chat and performs its action, it probably means that you can't use that key to type something in the Chat anyways.

Still, out of curiosity, is there anything actually preventing storing chat in demos as a sort of metadata? I may be incorrect, but IIRC, in earlier DSDA verions you could record a strict demo with freelook, and it would play back in DSDA with freelook, and in other ports without. Again though, don't bother implementing it on my account, since I'd likely still prefer videos regardless.

I'm not knowledgeable when it comes to demos, but I think it's possible. DSDA already has its own exclusive "dsdademo" format which lets you save and reload a save while recording and probably do other things, and storing Chat messages sounds like something it could do as well.

MrAlaux commented 10 months ago

Alright, it looks like the only responder functions called before HU_Responder() are M_Responder() and G_Responder() (which itself calls HU_Responder()), and their inputs already check for the Chat being up if prudent. I think there are no more troublesome inputs.

Fixed by https://github.com/MrAlaux/Nugget-Doom/commit/67e31e4c6d6c9c36e3a2fd51523170d9584a12df.