Skip to content

Commit

Permalink
feat: make context executor required
Browse files Browse the repository at this point in the history
This was previously made optional as the execute command could be ran without an executor.

After some consideration, this change has been reverted, as there aren't many use cases for running a command without an executor, and it often leads to more verbose code.
  • Loading branch information
paradoxuum committed Aug 2, 2024
1 parent 47fa67f commit f6dc2ba
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 30 deletions.
7 changes: 1 addition & 6 deletions docs/src/content/docs/reference/dispatcher.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Executes a command.
**Parameters**:
- `path` - The command's path
- `args` - The arguments to pass to the command
- `executor` - The command's executor, or `undefined` if the executor is the server
- `executor` - The command's executor

**Returns**:
A `Promise` that resolves when the command has been executed. The value contained in the Promise is
Expand All @@ -25,14 +25,9 @@ Executes a command.

If the path points to a server command, the command will be executed on the server.

If `runAsPlayer` is set to `false`, the command will be executed with no executor. This only
applies to commands registered on the client - commands registered on the server will always
execute with the local player as the executor.

**Parameters**:
- `path` - The command's path
- `args` - The arguments to pass to the command
- `runAsPlayer` - Whether to run the command as the local player.

**Returns**:
A `Promise` that resolves when the command has been executed. The value contained in the Promise is
Expand Down
9 changes: 2 additions & 7 deletions packages/core/src/client/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,19 @@ export class ClientDispatcher extends BaseDispatcher<
*
* If the path points to a server command, the command will be executed on the server.
*
* If `runAsPlayer` is set to `false`, the command will be executed with no executor. This only
* applies to commands registered on the client - commands registered on the server will always
* execute with the local player as the executor.
*
* @param path The command's path.
* @param args The arguments to pass to the command.
* @param runAsPlayer Whether to run the command as the local player.
* @returns A {@link HistoryEntry} containing the command's response
* @returns A {@link Promise} that resolves when the command has been executed. The value contained in the
* Promise is a `HistoryEntry` object containing the command's response and the time it was executed.
*/
async run(path: RegistryPath, args: string[] = [], runAsPlayer = true) {
async run(path: RegistryPath, args: string[] = []) {
const inputText = getInputText(path, args);
const [success, context] = this.executeCommand(
Players.LocalPlayer,
path,
inputText,
args,
runAsPlayer ? Players.LocalPlayer : undefined,
).await();

if (!success) {
Expand Down
18 changes: 8 additions & 10 deletions packages/core/src/server/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export class ServerDispatcher extends BaseDispatcher<

const commandPath = RegistryPath.fromString(path);

const [success, data] = this.run(commandPath, args, executor)
const [success, data] = this.run(executor, commandPath, args)
.timeout(5)
.await();

Expand All @@ -45,7 +45,7 @@ export class ServerDispatcher extends BaseDispatcher<
contextData = data.getData();
} else {
const inputText = getInputText(commandPath, args);
this.handleError(inputText, data, executor);
this.handleError(executor, inputText, data);
contextData = {
args,
input: inputText,
Expand All @@ -71,29 +71,27 @@ export class ServerDispatcher extends BaseDispatcher<
* @returns A {@link Promise} that resolves when the command has been executed. The value contained in the
* Promise is a {@link CommandContext} instance containing the execution result.
*/
async run(path: RegistryPath, args: string[] = [], executor?: Player) {
async run(executor: Player, path: RegistryPath, args: string[] = []) {
const inputText = getInputText(path, args);
return this.executeCommand(path, inputText, args, executor).catch((err) => {
this.handleError(inputText, err, executor);
return this.executeCommand(executor, path, inputText, args).catch((err) => {
this.handleError(executor, inputText, err);

const context = new CommandContext(
this.logger,
executor,
path,
args,
inputText,
executor,
);
context.state = this.config.defaultContextState;
context.error("An error occurred.");
return context;
});
}

private handleError(input: string, err: unknown, executor?: Player) {
const executorText =
executor !== undefined ? `executor '${executor.Name}'` : "no executor";
private handleError(executor: Player, input: string, err: unknown) {
this.logger.warn(
`Failed to execute '${input}' with ${executorText}: ${err}`,
`Failed to execute '${input}' with executor '${executor.Name}': ${err}`,
);
}
}
9 changes: 8 additions & 1 deletion packages/core/src/shared/__tests__/context.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { expect, test } from "@rbxts/jest-globals";
import { Players } from "@rbxts/services";
import { CommandContext } from "../core/context";
import { RegistryPath } from "../core/path";
import { CommandContextData, CommandReply } from "../types";
Expand All @@ -8,7 +9,13 @@ import { getInputText } from "../util/string";
const createContext = (args: string[] = []) => {
const path = RegistryPath.fromString("test");
const logger = new CenturionLogger(CenturionLogLevel.Debug);
return new CommandContext(logger, path, args, getInputText(path, args));
return new CommandContext(
logger,
Players.LocalPlayer,
path,
args,
getInputText(path, args),
);
};

test("contexts can be replied to", () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/shared/core/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ export class CommandContext<S = unknown> {

constructor(
private readonly logger: CenturionLogger,
readonly executor: Player,
readonly path: RegistryPath,
readonly args: string[],
readonly input: string,
readonly executor?: Player,
) {}

/**
Expand Down
8 changes: 3 additions & 5 deletions packages/core/src/shared/core/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,26 +21,24 @@ export abstract class BaseDispatcher<
}

protected async executeCommand(
executor: Player,
path: RegistryPath,
inputText: string,
args: string[] = [],
executor?: Player,
) {
if (this.logger.level === CenturionLogLevel.Debug) {
const executorText =
executor !== undefined ? `executor '${executor.Name}'` : "no executor";
this.logger.debug(
`Executing command with ${executorText}: ${getInputText(path, args)}`,
`Executing command with executor '${executor.Name}': ${getInputText(path, args)}`,
);
}

const command = this.registry.getCommand(path);
const context = new CommandContext(
this.logger,
executor,
path,
args,
inputText,
executor,
);
context.state = this.config.defaultContextState;

Expand Down

0 comments on commit f6dc2ba

Please sign in to comment.