Filled out design decisions doc

docs/DesignDecisions.md

@@ -1 +1,125 @@
-Coming Soon™
+# Using the Command Class
+
+## any[] Parameters For Subcommand Run
+
+Unless there's some sort of TypeScript wizardry to solve this, the `args` parameter in the subcommand type will have to be `any[]` because it's simply too context-dependent to statically figure it out.
+- Each subcommand is its own layer which doesn't know about parent commands at compile-time.
+- Subcommands can be split into different files for code maintainability.
+- Even though the last argument is able to be strongly-typed, if you have multiple parameters, you'd essentially only get static benefits for one of the arguments, and you wouldn't even know the location of that one argument.
+- Overall, it's just easier to use your best judgement then use type assertions.
+
+## Channel Type Type Guards
+
+As of right now, it's currently not feasible to implement type guards for channel types. [Discriminated unions with a default parameter don't work with callbacks.](https://github.com/microsoft/TypeScript/issues/41759) In order to implement type guards, the `channelType` parameter would have to be required, making each command layer quite tedious.
+
+So instead, use non-null assertions when setting the `channelType`. For example:
+
+```ts
+import {Command, NamedCommand, CHANNEL_TYPE} from "../core";
+import {TextChannel} from "discord.js";
+
+export default new NamedCommand({
+	channelType: CHANNEL_TYPE.GUILD,
+	async run({message, channel, guild, author, member, client, args}) {
+		console.log(guild!.name);
+		console.log(member!.nickname);
+		console.log((channel as TextChannel).name !== "dm");
+	}
+});
+```
+
+```ts
+import {Command, NamedCommand, CHANNEL_TYPE} from "../core";
+import {DMChannel} from "discord.js";
+
+export default new NamedCommand({
+	channelType: CHANNEL_TYPE.DM,
+	async run({message, channel, guild, author, member, client, args}) {
+		console.log(guild === null);
+		console.log(member === null);
+		console.log((channel as DMChannel).type === "dm");
+	}
+});
+```
+
+The three guarantees are whether or not `guild` will be `null`, whether or not `member` will be `null`, and the type of `channel`.
+
+*Take note that `member` can still be `null` even in a guild (for example, if you target a message by someone who left), `member` cannot be `null` here because the `message` being sent must be by someone who is in the guild by this point.*
+
+## Uneven Return Paths
+
+`Command.run` doesn't use the return values for anything, so it's safe to do `return channel.send(...)` to merge those two statements. However, you'll come across an error: `Not all code paths return a value.`
+
+There are several ways to resolve this issue:
+- Split all `return channel.send(...)` statements to `{channel.send(...); return;}`
+- Set an explicit any return type in the function header: `async run(...): Promise<any> {`
+- Add an extra `return` statement at the end of each path
+
+## Type Guards
+
+The `Command` class is implemented in a certain way to provide type guards which reduce unnecessary properties at compile-time rather than warning the user at runtime.
+- The reason `NamedCommand` (which extends `Command`) exists is to provide a type guard for `aliases`. After all, `aliases` doesn't really make sense for generic subcommand types - how would you handle an alias for a type that accepts a number for example?
+- The `endpoint` property changes what other properties are available via a discriminated union. If `endpoint` is `true`, no subcommands of any type can be defined. After all, it wouldn't make sense logically.
+
+## Boolean Types
+
+Boolean subcommand types won't be implemented:
+- Since there are only two values, why not just put it under `subcommands`?
+- If boolean types were to be implemented, how many different types of input would have to be considered? `yes`/`no`, `y`/`n`, `true`/`false`, `1`/`0`, etc.
+
+## Hex and Octal Number Types
+
+For common use cases, there wouldn't be a need to go accept numbers of different bases. The only time it would be applicable is if there was some sort of base converter command, and even then, it'd be better to just implement custom logic.
+
+# The Command Handler
+
+## The Scope of the Command Handler
+
+What this does:
+- Provides the `Command`/`NamedCommand` classes.
+- Dynamically loads commands and attaches runtime metadata.
+- Provides utility functions specific to Discord to make certain patterns of commands less tedious to implement.
+
+What this doesn't do:
+- Manage the general file system or serialization/deserialization of data.
+- Provide general utility functions.
+- Provide any Discord-related functionality besides strictly command handling.
+
+## Client Creation
+
+Creating the client is beyond the scope of the command handler and will not be abstracted away. Instead, the user will simply attach the command handler to the client to initialize it.
+- This makes it so if a user wants to specify their own `ClientOptions` when instantiating the client, it's less troublesome to implement.
+- The user can export the client and use it throughout different parts of their code.
+
+## Bot-Specific Mentions
+
+Pinging the bot will display the current guild prefix. The bot mention will not serve as an alternate prefix.
+- When talking about a bot, the bot might be pinged to show who it is. It could be in the middle (so don't listen for a prefix anywhere) or it could be at the start (so only listen to a standalone ping).
+- It likely isn't a common use case to ping the bot. The only time it would really shine is in the event two bots have a prefix conflict, but the command that changes prefixes can simply add a parameter to deal with that case. For example, instead of `@bot set prefix <prefix>`, you'd use `set prefix <prefix> @bot`.
+
+## Direct Messages
+
+When direct messaging a bot, no prefixes will be used at all because it's assumed that you're executing a command. Because the only people allowed is the user and the bot, NSFW-only commands can also be executed here.
+
+## Permission Setup
+
+Because the command handler provides no specific permission set, it's up to the user to come up with functions to add permissions as well as create the enum that assigns permissions.
+- The `permission` property of a `Command` instance is `-1` by default, which means to inherit the permission level from the parent command. If you want, you can create your enum like this: `enum Permissions {INHERIT = -1, USER, ADMIN}`, where `Permissions.USER = 0` and `Permissions.ADMIN = 1`.
+
+# Miscellaneous
+
+## Static Event Loading
+
+While dynamic loading fits very well with commands, it was more or less clunky design to try and make events fit the same model:
+- There are no restrictions when it comes to command names, and the name of the file will determine the name of the command, which avoids repetition. Events on the other hand involved lots of boilerplate to get static types back.
+- Since there can be multiple listeners per event, large event files can be split up into more organized blocks.
+- Likewise, small event listeners which span multiple events can be grouped together like `channelCreate` and `channelDelete`, showing the relation in one single file rather than splitting them up just because they're two different events.
+
+## Testing
+
+For TravBot, there'll be two types of tests: standard unit tests and manual integration tests.
+- Standard unit tests are executed only on isolated functions and are part of the pre-commit hook.
+- Somehow, including the bot in an import chain will cause the system to crash (same error message as [this](https://stackoverflow.com/questions/66102858/discord-clientuser-is-not-a-constructor)). That's why the integration tests are manually done. There would be a list of inputs and outputs to check of each command for tests while simultaneously serving as a help menu with examples of all possible inputs/outputs for others to see.
+- An idea which will not be implemented is prompting the user for inputs during the tests. This is no better than manual tests, worse actually, because if this had to run before each commit, it'd quickly become a nightmare.
+- Maybe take some ideas from something like [this](https://github.com/stuyy/jest-unit-tests-demo) in the future to get tests to properly work.
+- Another possibility is to use `client.emit(...)` then mock the `message.channel.send(...)` function which would listen if the input is correct.

docs/Documentation.md

@@ -31,6 +31,13 @@ Because versions are assigned to batches of changes rather than single changes (
 
 *Note: This system doesn't retroactively apply to TravBot-v2, which is why this version naming system won't make sense for v2's changelog.*
 
+# Message Subcommand Type
+
+- `https://discord.com/channels/<id>/<id>/<id>` comes from the `Copy Message Link` button.
+- `<id>-<id>` comes from holding `Shift` on the desktop application and clicking on the `Copy ID` button.
+
+
+
 # Cleanup is Soon™
 
 ## Convenience Functions

docs/Overview.md

@@ -72,7 +72,7 @@ export default new NamedCommand({
 
 Here, . For example, if this file was named `test.ts`, `$test <@237359961842253835>` would get the user by the ID `237359961842253835` into `args[0]` as a [User](https://discord.js.org/#/docs/main/stable/class/User) object. `$test experiment` would run as if you just called `$test` *(given that [endpoint](Documentation.md#) isn't set to `true`)*.
 
-If you want, you can typecast the argument to be more strongly typed, because the type of `args` is `any[]`. *([See why if you're curious.](DesignDecisions.md#))*
+If you want, you can typecast the argument to be more strongly typed, because the type of `args` is `any[]`. *([See why if you're curious.](DesignDecisions.md#any[]-parameters-for-subcommand-run))*
 
 ```ts
 import {Command, NamedCommand} from "../core";
@@ -201,7 +201,7 @@ This will just run whatever code is in there.
 
 ## Listening for events
 
-Rather than have an `events` folder which contains dynamically loaded events, you add an event listener directly via `client.on("...", () => {})`. *([See why if you're curious.](DesignDecisions.md#))* The client can be imported from the index file.
+Rather than have an `events` folder which contains dynamically loaded events, you add an event listener directly via `client.on("...", () => {})`. *([See why if you're curious.](DesignDecisions.md#static-event-loading))* The client can be imported from the index file.
 
 ```ts
 import {client} from "..";

src/core/command.ts

@@ -139,6 +139,7 @@ export const defaultMetadata = {
     channelType: CHANNEL_TYPE.ANY
 };
 
+// Each Command instance represents a block that links other Command instances under it.
 export class Command {
     public readonly description: string;
     public readonly endpoint: boolean;
@@ -146,17 +147,19 @@ export class Command {
     public readonly permission: number; // -1 (default) indicates to inherit, 0 is the lowest rank, 1 is second lowest rank, and so on.
     public readonly nsfw: boolean | null; // null (default) indicates to inherit
     public readonly channelType: CHANNEL_TYPE | null; // null (default) indicates to inherit
-    protected run: (($: CommandMenu) => Promise<any>) | string;
+    // The execute and subcommand properties are restricted to the class because subcommand recursion could easily break when manually handled.
-    protected readonly subcommands: Collection<string, NamedCommand>; // This is the final data structure you'll actually use to work with the commands the aliases point to.
+    // The class will handle checking for null fields.
-    protected channel: Command | null;
+    private run: (($: CommandMenu) => Promise<any>) | string;
-    protected role: Command | null;
+    private readonly subcommands: Collection<string, NamedCommand>; // This is the final data structure you'll actually use to work with the commands the aliases point to.
-    protected emote: Command | null;
+    private channel: Command | null;
-    protected message: Command | null;
+    private role: Command | null;
-    protected user: Command | null;
+    private emote: Command | null;
-    protected id: Command | null;
+    private message: Command | null;
-    protected idType: ID | null;
+    private user: Command | null;
-    protected number: Command | null;
+    private id: Command | null;
-    protected any: Command | null;
+    private idType: ID | null;
+    private number: Command | null;
+    private any: Command | null;
 
     constructor(options?: CommandOptions) {
         this.description = options?.description || "No description.";
@@ -240,6 +243,9 @@ export class Command {
 
     // Go through the arguments provided and find the right subcommand, then execute with the given arguments.
     // Will return null if it successfully executes, SingleMessageOptions if there's an error (to let the user know what it is).
+    //
+    // Calls the resulting subcommand's execute method in order to make more modular code, basically pushing the chain of execution to the subcommand.
+    // For example, a numeric subcommand would accept args of [4] then execute on it.
     public async execute(
         args: string[],
         menu: CommandMenu,
@@ -261,12 +267,12 @@ export class Command {
             // 1. Does this command specify a required channel type? If so, does the channel type match?
             if (
                 metadata.channelType === CHANNEL_TYPE.GUILD &&
-                (!(menu.channel instanceof GuildChannel) || menu.guild === null)
+                (!(menu.channel instanceof GuildChannel) || menu.guild === null || menu.member === null)
             ) {
                 return {content: "This command must be executed in a server."};
             } else if (
                 metadata.channelType === CHANNEL_TYPE.DM &&
-                (menu.channel.type !== "dm" || menu.guild !== null)
+                (menu.channel.type !== "dm" || menu.guild !== null || menu.member !== null)
             ) {
                 return {content: "This command must be executed as a direct message."};
             }

src/core/libd.ts

@@ -247,3 +247,14 @@ export async function callMemberByUsername(
         else send(`Couldn't find a user by the name of \`${username}\`!`);
     } else send("You must execute this command in a server!");
 }
+
+// TO DO Section //
+
+// getGuildByID() - checks for guild.available (boolean)
+// getGuildByName()
+// findMemberByNickname() - gets a member by their nickname or their username
+// findUserByUsername()
+
+// For "get x by y" methods:
+// Caching: All guilds, channels, and roles are fully cached, while the caches for messages, users, and members aren't complete.
+// It's more reliable to get users/members by fetching their IDs. fetch() will searching through the cache anyway.