Skip to content
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

AddCommandHandler problem #418

Open
CrosRoad95 opened this issue Sep 5, 2018 · 5 comments · Fixed by #3017
Open

AddCommandHandler problem #418

CrosRoad95 opened this issue Sep 5, 2018 · 5 comments · Fixed by #3017
Labels
bug Something isn't working

Comments

@CrosRoad95
Copy link
Contributor

Describe the bug
If you trying to add command example for showmemstat command, addCommandHandler returning true even command isn't added.

To Reproduce
clientside

created = addCommandHandler("showmemstat", function()
  outputChatBox( "showmemstat working?") -- will never displayed
end)
outputChatBox( tostring(created) ) -- true

if you execute command showmemstat, only mem stat window appear.

Expected behavior
Should return false for some exeptions

Screenshots

MTA Client (please complete the following information):
Multi Theft Auto v1.5.5-release-13968

MTA Server (please complete the following information):
MTA:SA Server v1.5.5-release-11751

Additional context

@qaisjp qaisjp added the bug Something isn't working label Sep 5, 2018
@qaisjp qaisjp added this to the Backlog milestone Sep 5, 2018
@qaisjp qaisjp added the good first issue Good for newcomers label Sep 5, 2018
@qaisjp
Copy link
Contributor

qaisjp commented Sep 5, 2018

Low priority but returning false seems reasonable.

Can you give an example of why this might be useful? Most people don't check the return value of addCommandHandler but I agree it should not return true if it wasn't a success.

@qaisjp
Copy link
Contributor

qaisjp commented Sep 5, 2018

This is also not implementable server-side as the server doesn't know what client commands there are.

@CrosRoad95
Copy link
Contributor Author

the server doesn't know what client commands there are. know, you can make blacklist for client and server with showmemstat and other functions

@qaisjp
Copy link
Contributor

qaisjp commented Sep 5, 2018

that's not really a clean solution imo

@znvjder
Copy link
Contributor

znvjder commented Sep 5, 2018

servers shouldn't apply. It's enough for client to prepare a table with commands available to client and omit them (return false). Its my solution.

@patrikjuvonen patrikjuvonen removed the good first issue Good for newcomers label Apr 16, 2023
patrikjuvonen pushed a commit that referenced this issue May 21, 2023
@patrikjuvonen patrikjuvonen modified the milestones: Backlog, 1.6.0 May 21, 2023
patrikjuvonen added a commit that referenced this issue Jun 12, 2023
… it should return false in certain scenarios (#3017)"

Requires more input and conversation.

This reverts commit 5947012.
@patrikjuvonen patrikjuvonen reopened this Jun 12, 2023
@patrikjuvonen patrikjuvonen removed this from the 1.6.0 milestone Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants