Skip to content

sv_chat.lua for okokChat - #207

Closed
okokscripts wants to merge 1 commit into
citizenfx:masterfrom
okokscripts:patch-1
Closed

sv_chat.lua for okokChat#207
okokscripts wants to merge 1 commit into
citizenfx:masterfrom
okokscripts:patch-1

Conversation

@okokscripts

@okokscripts okokscripts commented Feb 10, 2022

Copy link
Copy Markdown

Changed sv_chat.lua join and leave message to okokChat with possibility to choose if only admins can see them.
Removed message sent when typing no command.

This sv_chat.lua file has been modified to support okokChat.
@smallo92

Copy link
Copy Markdown

Use the built in chatmodes for this sort of thing. You don't need to edit the base resource. See #101 for examples.

@smallo92

Copy link
Copy Markdown

Oh wait, I thought you were trying to do an admin chat channel lol. What you are adding can be easily added in your own script using hooks there. There is no need to have this as part of the base resource.

@tabarra

tabarra commented Feb 11, 2022

Copy link
Copy Markdown
Contributor

Oof
Base resources should be framework agnostic.
They are the base other resources should build atop, not the other way around.

@titanium-cfx

Copy link
Copy Markdown

As mentioned, base resources must be framework agnostic.
You should take a look at this pending PR on how to use chat hooks: citizenfx/fivem-docs#234

@technetium-cfx

Copy link
Copy Markdown
Contributor

You removed functionality from a default resource because it was not useful to you. That's a red flag already. Framework-specific code does not belong here, and no one on the forums will have told you so. This was likely a misinterpretation.

We welcome additional default functionality or general improvements. Any change here must be relevant for everyone, not just those who use a subset of frameworks out there.

@logan-mcgee

logan-mcgee commented Feb 11, 2022

Copy link
Copy Markdown

adding "functionality" doesnt mean hardcoding your own logic in, instead providing a nice way of being able to handle that from other resources

@technetium-cfx

technetium-cfx commented Feb 11, 2022

Copy link
Copy Markdown
Contributor

We removed the functionality because it would send a default message without any command before the message that we do not wish to have

I'm not sure what causes that to happen, but that does not seem to originate from here.

we also changed the join and leave messages to be compatible with okokChat and added the possibility to choose if only the admins can see it.

Modifying a default resource to be compatible with your extension should not be necessary, and is highly discouraged. The built-in ACL system should be used.

Sounds like you either want to cancel the chatMessage event or use chat modes. Also if you are missing functionality in the chat resource then PR an api for it don’t tell people to make those changes.

This does not include removing existing functionality just because your resource doesn't need it. What they meant is that framework-agnostic changes are accepted, not this.

Do you know any other way of doing this?

Convars, built-in ACL system, anything that doesn't depend on external frameworks.

@blattersturm

blattersturm commented Feb 11, 2022

Copy link
Copy Markdown
Contributor

Chat modes/message hooks are already an API that exists and the first reply to this PR of yours mentioned that, as did the post you are quoting.

Also, this PR by itself would break stuff for anyone not using 'esx' or 'qbcore' and removes random other stuff unconditionally so it's entirely the opposite of 'making an API to handle stuff if it's not implemented yet'.

@okokscripts
okokscripts deleted the patch-1 branch February 15, 2022 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants