Skip to content

Conversation

@techs-sus
Copy link
Collaborator

We can rely on the client to provide multi command handling.

This is still missing g/no. support (which might be hard to add?).

@techs-sus
Copy link
Collaborator Author

Also, can someone test if you are able to run namespaces with capital letters inside? I forgot to check if that still works.

@techs-sus
Copy link
Collaborator Author

It does work without the string.lower, but we should play safe and use a string.lower when doing namespace lookup anyway.

all network callbacks are called with a Player
- allows namespaces to run their own callbacks
- mod commands can be implemented easier
- this is NOT wired to anything yet because it is a prototype
- client can call forward(...) to invoke server code safely
- forward is NOT implicitly called in client_registry.luau because i am scared it can cause bugs
- we need a more ergonomic way to port default & get because help_callback feels weird to use
@techs-sus techs-sus changed the title Add client-heavy multi command system Add client-heavy multi command system (and experimental rewrite) Nov 27, 2025
…(legacy logic)

- should still not used in ANY code yet
- added forward in help_callback; the rationale for this is that maybe
  some namespaces have custom server logic which can influence help
  results? i was mainly thinking of dynamically updated mod command help
  texts/ranking (this is a nitpick / extra feature)
- we should figure out how to get nice, ergonomic Result types because
  error handling in Luau is not pretty and currently we have to do alot
  of casts
- serverDefineSimple is a shim to support literally just copy pasting
  old define code blocks and it should help them work; we should port
  this old code to a cleaner, newer interface but i don't have any ideas
  yet
@techs-sus techs-sus requested a review from ewd3v December 19, 2025 01:54
help_callback = function(): impl.HelpResults?
return {
c = table.freeze({
aliases = { "run/" },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"run" and "c" (execute) are different commands


help_callback = function(): impl.HelpResults?
return {
c = table.freeze({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically the real command name is "execute" with "c" being an alias for it


return {
table.freeze({
prefixes = { "c", "run" },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default namespace isn't supposed to have prefixes? Prefixes are what you add before your command string to tell the parser to look at commands from that prefix (so like the "get" or "g" in get/respawn)

Copy link
Member

@ewd3v ewd3v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't exactly understand the reason for defining (not the define() function, that should be called implement() tbh) commands in code rather than in toml files like before.
It also doesn't look like all commands have gotten implemented yet?

I am ok with parsing multiple commands on client which then tells the server which ones to run.
But my solution to this was to add an actual "get" command that splits the 1st argument it receives by spaces and then run each one of those as it's own command from the "get" namespace, it then receiving the context.Data object from each of those commands after they run and then passing all of those to the server version of the "get" command which can then pass the client context data to the respective command it runs.
In short moving the namespace parsing code into their own commands and only executing commands in the default namespace from the chat / console. (This comes with the benefit that the "get" command, or in the future, a "sb" / "mod" command can check permissions before it runs any actual commands).

@techs-sus
Copy link
Collaborator Author

I haven't implemented all commands yet because I wanted to see we wanted to keep going with this approach. The "default" namespace was a test snippet, because I wanted to see if the API was ergonomic enough and it partially serves as an example namespace.

I don't think using TOML is a bad idea, and the define() function was to see if it was possible to quickly port old code. The main reason I moved some TOML data was to prototype a better API surface. But yeah, we should be using TOML for description, aliases, name, arguments (?), etc.

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.

3 participants