[chat] 'chat:removeMode' event will not remove the "all" mode immediately

CritteR · December 19, 2021, 10:26pm

Client Version: Canary
Server Version: (if needed): 1.0.0.4394

Issue: (default resource) chat:

chat:removeMode client event will not remove the “all” mode for users that already have the mode selected. The mode will be removed only once the user goes to another chat mode, using the “Tab” key.

Reproduction:

Prerequisits: Server has multiple chat modes. (can test using bubblyRestrictedChat (ACL-restricted chat))

User would start with “all” mode active.

Use the following command, while in “all” mode:

RegisterCommand('removeall', function(source, args)
    TriggerEvent('chat:removeMode', 'all')
end)

Expected behavior: Mode is removed and chat mode is moved to the next available mode.

Current behavior: User can still use “all” mode, until he/she switches to another mode, then it will disappear.

CritteR · December 23, 2021, 11:08am

Hey all,

I tried digging through the chat resource more today, and I think I might have found the issue.

The research:

First, I handled _chat:messageEntered event on my own resource, to see the actual mode that chat returns.

On message, 1, I sent it through the normal “all” mode (id 0 in chat.modes array). That works! It returns “al”
Message 2 is custom mode that I made. It should be id 1 chat.modes array, but because there is a “_global” hidden chat that is already id 1, my chat becomes id 2 (this is important later).

Between message 2 and 3, I switch back to “all” chat, and REMOVED IT, using “chat:removeMode”.
Because I was still still on “all” chat (id 0), I still see “All” as a chat mode, and I can still send messages, BUT, when receiving the message on server, I see that the mode is “_global” !

The investigation:

Chat resource starts with 2 modes. defaultMode, which is All (named, All, visible, id 0) and globalMode (also named “All”, hidden, id 1)

github.com

citizenfx/cfx-server-data/blob/44fc68d7ee1b94ad67a211a6ff8234ce4ff760c8/resources/[gameplay]/chat/html/App.ts#L88-L89

    
      
          modes: [defaultMode, globalMode] as Mode[],
          modeIdx: 0,

When removing a mode, the chat resource simply removes that mode from the array (which also reorders the index, I think…I don’t know vue OR js.)

github.com

citizenfx/cfx-server-data/blob/44fc68d7ee1b94ad67a211a6ff8234ce4ff760c8/resources/[gameplay]/chat/html/App.ts#L264-L270

    
      
          ON_MODE_REMOVE({ name }: { name: string }) {
            this.modes = this.modes.filter(a => a.name !== name);
          
          
  if (this.modes.length === 0) {
              this.modes = [defaultMode];
            }
          },

Notice how it doesn’t check if you were on that mode, this will become a pattern.

Below you can see events for opening the menu (where you can see that mode that you are in) and the sending message event:

github.com

citizenfx/cfx-server-data/blob/44fc68d7ee1b94ad67a211a6ff8234ce4ff760c8/resources/[gameplay]/chat/html/App.ts#L212-L230

    
      
          ON_OPEN() {
            this.showInput = true;
            this.showWindow = true;
            if (this.showWindowTimer) {
              clearTimeout(this.showWindowTimer);
            }
            this.focusTimer = window.setInterval(() => {
              if (this.$refs.input) {
                (this.$refs.input as HTMLInputElement).focus();
              } else {
                clearInterval(this.focusTimer);
              }
            }, 100);
          },
          ON_MESSAGE({ message }: { message: Message }) {
            message.id = `${new Date().getTime()}${Math.random()}`;
            message.modeData = this.modes.find(mode => mode.name === message.mode);
            this.messages.push(message);
          },

This is where the actual problem is!
There isn’t any point in the resource, where modeIdx is checked, to make sure that it’s valid! (apart from switching modes, which is why I thought that “all” chat doesn’t disappear unless switching, but in fact I was seeing the _global chat)

Solutions

Easiest hotfix: move globalMode to 0, defaultMode to 1 and default modeIdx to 1. This will always keep the modes outside of “globalMode” reach. But it will not fix the errors that appear when a modeIdx becomes NULL
```
modes: [globalMode, defaultMode] as Mode[],
modeIdx: 1,
```

Proper (I think) fix: Check chat mode on ON_OPEN() event

// Again, I don't know js. Can't really add code here, but I guess you can get the "Tab" button logic.

CritteR · December 25, 2021, 8:36pm

Posted a PR of my attempt at fixing this:

github.com/citizenfx/cfx-server-data

fix(chat): Make sure user doesn't land on hidden or null chat mode, after using "chat:removeMode"

citizenfx:master ← CritteRo:chat-patch

opened 03:20PM - 23 Dec 21 UTC

CritteRo

+13 -4

PR started from issue: https://forum.cfx.re/t/chat-chat-removemode-event-will-no…t-remove-the-all-mode-immediately/4790531/2 Changes made: * moved globalMode (_global) as the first mode, and defaultMode as second mode and changed default `modeIdx` value to reflect that. This should fix the situations where users on default mode will have their "all" chat removed, but then could see "_global" chat. * Added a check on `ON_MODE_REMOVE()`, to make sure that `modeIdx` doesn't point to an `undefined` mode after removing the chat mode. (But it could still point to hidden modes!! )

It works well for the use cases that I encountered:

stopped “_global” mode from becoming usable, when “all” chat is removed.
removing modes shouldn’t allow the user to land on NULL chat modes, but it does on hidden ones, because I couldn’t find a safe way to filter those channels safely.