From 269380b94ced9311f9f5320bcc1c7cd68cd505bd Mon Sep 17 00:00:00 2001 From: Bertie690 Date: Sat, 26 Apr 2025 17:55:26 -0400 Subject: [PATCH] Update linting.md, fix lefthook etc. --- biome.jsonc | 4 +- docs/comments.md | 53 ++++++++++++--------- docs/enemy-ai.md | 117 ++++++++++++++++++++++++----------------------- docs/linting.md | 70 ++++++++++++++++++++-------- lefthook.yml | 5 +- 5 files changed, 144 insertions(+), 105 deletions(-) diff --git a/biome.jsonc b/biome.jsonc index 9d0e6a9b5ff..293e2e96006 100644 --- a/biome.jsonc +++ b/biome.jsonc @@ -99,10 +99,10 @@ "linter": { "rules": { "performance": { - "noDelete": "off" + "noDelete": "off" // ???? }, "style": { - "noNamespaceImport": "off" + "noNamespaceImport": "off" // TODO: Refactor and mark as fixed } } } diff --git a/docs/comments.md b/docs/comments.md index b1794f70b56..78b0b0559e2 100644 --- a/docs/comments.md +++ b/docs/comments.md @@ -1,44 +1,53 @@ -## How do I comment my code? +# Commenting code -### While we're not enforcing a strict standard, there are some things to keep in mind: +People spend more time reading code than writing it (sometimes substantially more so). As such, comments and documentation are **vital** for any large codebase like this. + +## While we're not enforcing a strict standard, here are some things to keep in mind: - Make comments meaningful - - Comments should be explaining why a line or block of code exists and what the reason behind it is - - Comments should not be repeating chunks of code or explaining what 'true' and 'false' means in typescript + - Comments should explain _why_ a line or block of code exists - its "raison d'être", if you will. + - Comments should not repeat chunks of code or explain concepts obvious to the average reader like the meaning of `&&=` or `??` +- Make comments readable + - The length of a comment should roughly scale with the complexity of whatever it is describing. Small quips are fine for inline comments and explanations, but summarizing a 300 line function with "Starts a battle" is about as good as doing nothing. + - Long comments should ideally be broken into multiple lines at around the 100-120 character mark. - Make sure comments exist on Functions, Classes, Methods, and Properties - - This may be the most important things to comment. When someone goes to use a function/class/method/etc., having a comment reduces the need to flip back and forth between files to figure out how something works. Peek Definition is great until you're three nested functions deep. - - The best example of this is TSDoc-style comments as seen below: - - When formatted this way, the comment gets shown by intellisense in VS Code or similar IDEs just by hovering over the text! - - Functions also show each the comment for parameter as you type them, making keeping track of what each one does in lengthy functions much more clear + - These may be the most important things to comment. When someone goes to use a function/class/method/etc., having a comment reduces the need to flip back and forth between files to figure out what XYZ does. Peek Definition is great until you're three nested levels deep. + - The best example of this is TSDoc-style comments as seen below: + - When formatted this way, the comment gets shown within VS Code or similar IDEs just by hovering over the function! + - Functions also show the comment for each parameter as you type them, making keeping track of arguments inside lengthy functions much more clear. + ```ts /** - * Change the type-based weather modifier if this move's power would be reduced by it + * Change the type-based weather modifier if this move's power would be reduced by it. * @param user - The {@linkcode Pokemon} using this move * @param target - The {@linkcode Pokemon} being targeted by this move - * @param move - The {@linkcode Move} being used - * @param args - [0]: a {@linkcode NumberHolder} for arenaAttackTypeMultiplier - * @returns whether the move effect was applied + * @param move - The {@linkcode Move} being used (unused) + * @param args - [0]: A {@linkcode NumberHolder} for arenaAttackTypeMultiplier + * @returns - Whether the move effect was applied successfully */ apply(user: Pokemon, target: Pokemon, move: Move, args: [NumberHolder, ...any]): boolean { } -/** Set to true when experimental animated sprites from Gen6+ are used */ -public experimentalSprites: boolean = false; - +/** Class containing */ +class Settings { + /** Set to `true` when experimental animated sprites from Gen6+ should be used */ + public experimentalSprites: boolean = false; +} /** * Cure the user's party of non-volatile status conditions, ie. Heal Bell, Aromatherapy */ export class DontHealThePartyPlsAttr extends MoveAttr { } + ``` You'll notice this contains an `{@linkcode Object}` tag for each parameter. This provides an easy type denomination and hyperlink to that type using VS Code's Intellisense. `@linkcode` is used instead of `@link` so that the text appears in monospace which is more obviously a `type` rather than a random hyperlink. -If you're interested in going more in depth, you can find a reference guide for how comments like these work [here](https://tsdoc.org) +If you're interested in going more in depth, you can find a reference guide for how comments like these work [here](https://tsdoc.org). ### What not to do: - Don't leave comments for code you don't understand - - Incorrect information is worse than no information. If you aren't sure how something works, don't make something up to explain it. Ask for help instead. + - Incorrect information is worse than no information. If you aren't sure how something works, don't make something up to explain it - ask for help and/or mark it as TODO. - Don't over-comment - - Not everything needs an explanation. Try to summarize blocks of code instead of singular lines where possible. Single line comments should call out specific oddities. + - Not everything needs an explanation. Try to summarize blocks of code instead of singular lines where possible. Single line comments should call out specific oddities or features. ## How do Abilities and Moves differ from other classes? While other classes should be fully documented, Abilities and Moves heavily incoperate inheritance (i.e. the `extends` keyword). Because of this, much of the functionality in these classes is duplicated or only slightly changed between classes. @@ -48,14 +57,14 @@ While other classes should be fully documented, Abilities and Moves heavily inco - Class member variables must be documented - You can use a single line documentation comment for these `/** i.e. a comment like this */` - `args` parameters must be documented if used - - This should look something like this when there are multiple: + - This should look something vaguely like this when there are multiple: ```ts /** ... * @param args - - * - `[0]` {@linkcode NumberHolder} `paramA` - paramA description here - * - `[1]` {@linkcode BooleanHolder} `paramB` - paramB description here - * - `[2]` {@linkcode BooleanHolder} `paramC` - paramC description here + * `[0]` The {@linkcode Move} being used + * `[1]` A {@linkcode BooleanHolder} used to track XYZ + * `[2]` {@linkcode BooleanHolder} `paramC` - paramC description here ... */ ``` diff --git a/docs/enemy-ai.md b/docs/enemy-ai.md index 6e73243786d..d73b0af980e 100644 --- a/docs/enemy-ai.md +++ b/docs/enemy-ai.md @@ -37,12 +37,12 @@ The `EnemyCommandPhase` follows this process to determine whether or not an enem 1. If the Pokémon has a move already queued (e.g. they are recharging after using Hyper Beam), or they are trapped (e.g. by Bind or Arena Trap), skip to resolving a `FIGHT` command (see next section). 2. For each Pokémon in the enemy's party, [compute their matchup scores](#calculating-matchup-scores) against the active player Pokémon. If there are two active player Pokémon in the battle, add their matchup scores together. 3. Take the party member with the highest matchup score and apply a multiplier to the score that reduces the score based on how frequently the enemy trainer has switched Pokémon in the current battle. - - The multiplier scales off of a counter that increments when the enemy trainer chooses to switch a Pokémon and decrements when they choose to use a move. + - The multiplier scales off of a counter that increments when the enemy trainer chooses to switch a Pokémon and decrements when they choose to use a move. 4. Compare the result of Step 3 with the active enemy Pokémon's matchup score. If the party member's matchup score is at least three times that of the active Pokémon, switch to that party member. - - "Boss" trainers only require the party member's matchup score to be at least two times that of the active Pokémon, so they are more likely to switch than other trainers. The full list of boss trainers in the game is as follows: - - All gym leaders, Elite 4 members, and Champions - - All Evil Team leaders - - The last three Rival Fights (on waves 95, 145, and 195) + - "Boss" trainers only require the party member's matchup score to be at least two times that of the active Pokémon, so they are more likely to switch than other trainers. The full list of boss trainers in the game is as follows: + - All gym leaders, Elite 4 members, and Champions + - All Evil Team leaders + - The last three Rival Fights (on waves 95, 145, and 195) 5. If the enemy decided to switch, send a switch `turnCommand` and end this `EnemyCommandPhase`; otherwise, move on to resolving a `FIGHT` enemy command. ## Step 2: Selecting a Move @@ -54,33 +54,34 @@ At this point, the enemy (a wild or trainer Pokémon) has decided against switch In `getNextMove()`, the enemy Pokémon chooses a move to use in the following steps: 1. If the Pokémon has a move in its Move Queue (e.g. the second turn of a charging move), and the queued move is still usable, use that move against the given target. 2. Filter out any moves it can't use within its moveset. The remaining moves make up the enemy's **move pool** for the turn. - 1. A move can be unusable if it has no PP left or it has been disabled by another move or effect - 2. If the enemy's move pool is empty, use Struggle. + 1. A move can be unusable if it has no PP left or it has been disabled by another move or effect. + 2. If the enemy's move pool is empty, use Struggle. 3. Calculate the **move score** of each move in the enemy's move pool. - 1. A move's move score is equivalent to the move's maximum **target score** among all of the move's possible targets on the field ([more on this later](#calculating-move-and-target-scores)). - 2. A move's move score is set to -20 if at least one of these conditions are met: - - The move is unimplemented (or, more precisely, the move's name ends with " (N)"). - - Conditions for the move to succeed are not met (unless the move is Sucker Punch, Upper Hand, or Thunderclap, as those moves' conditions can't be resolved before the turn starts). - - The move's target scores are 0 or `NaN` for each target. In this case, the game assumes the target score calculation for that move is unimplemented. + 1. A move's move score is equivalent to the move's maximum **target score** among all of the move's possible targets on the field ([more on this later](#calculating-move-and-target-scores)). + 2. A move's move score is set to -20 if at least one of these conditions are met: + - The move is unimplemented (or, more precisely, the move's name ends with "(N)"). + - Conditions for the move to succeed are not met (unless the move is Sucker Punch, Upper Hand or Thunderclap, as those moves' conditions can't be resolved until after the turn starts). + - The move's target scores are 0 or `NaN` for each target. In this case, the game assumes the target score calculation for that move is unimplemented. 4. Sort the move pool in descending order of move scores. 5. From here, the enemy's move selection varies based on its `aiType`. If the enemy is a Boss Pokémon or has a Trainer, it uses the `SMART` AI type; otherwise, it uses the `SMART_RANDOM` AI type. - 1. Let $m_i$ be the *i*-th move in the sorted move pool $M$: - - If `aiType === SMART_RANDOM`, the enemy has a 5/8 chance of selecting $m_0$ and a 3/8 chance of advancing to the next best move $m_1$, where it then repeats this roll. This process stops when a move is selected or the last move in the move pool is reached. - - If `aiType === SMART`, a similar loop is used to decide between selecting the move $m_i$ and advancing to the next iteration with the move $m_{i+1}$. However, instead of using a flat probability, the following conditions need to be met to advance from selecting $m_i$ to $m_{i+1}$: - - $\text{sign}(s_i) = \text{sign}(s_{i+1})$, where $s_i$ is the move score of $m_i$. - - $\text{randInt}(0, 100) < \text{round}(\frac{s_{i+1}}{s_i}\times 50)$. In other words: if the scores of $m_i$ and $m_{i+1}$ have the same sign, the chance to advance to the next iteration with $m_{i+1}$ is proportional to how close the scores are to each other. The probability to advance to the next iteration is at most 50 percent (when $s_i$ and $s_{i+1}$ are equal). + 1. Let $m_i$ be the *i*-th move in the sorted move pool $M$: + - If `aiType === SMART_RANDOM`, the enemy has a 5/8 chance of selecting $m_0$ and a 3/8 chance of advancing to the next best move $m_1$, where it then repeats this roll. This process stops when a move is selected or the last move in the move pool is reached. + - If `aiType === SMART`, a similar loop is used to decide between selecting the move $m_i$ and advancing to the next iteration with the move $m_{i+1}$. However, instead of using a flat probability, the following conditions need to be met to advance from selecting $m_i$ to $m_{i+1}$: + - $\text{sign}(s_i) = \text{sign}(s_{i+1})$, where $s_i$ is the move score of $m_i$. + - $\text{randInt}(0, 100) < \text{round}(\frac{s_{i+1}}{s_i}\times 50)$. In other words: if the scores of $m_i$ and $m_{i+1}$ have the same sign, the chance to advance to the next iteration with $m_{i+1}$ is proportional to how close the scores are to each other. The probability to advance to the next iteration is at most 50 percent (when $s_i$ and $s_{i+1}$ are equal). 6. The enemy will use the move selected in Step 5 against the target(s) with the highest [**target selection score (TSS)**](#choosing-targets-with-getnexttargets) ### Calculating Move and Target Scores -As part of the move selection process, the enemy Pokémon must compute a **target score (TS)** for each legal target for each move in its move pool. The base target score for all moves is a combination of the move's **user benefit score (UBS)** and **target benefit score (TBS)**. +As part of the move selection process, the enemy Pokémon must compute a **target score (TS)** for each legal target for each move in its move pool. The base target score is a combination of the move's **user benefit score (UBS)** and **target benefit score (TBS)**, representing how much the move helps or hinders the user and/or its target(s). $$ -\text{TS} = \text{UBS} + \text{TBS} \times +\text{TS} = \text{UBS} + \left( \text{TBS} \times \begin{cases} -1 & \text{if target is an opponent} \\ 1 & \text{otherwise} \end{cases} +\right) $$ A move's UBS and TBS are computed with the respective functions in the `Move` class: @@ -104,36 +105,36 @@ In addition to the base score from `Move.getTargetBenefitScore()`, attack moves More specifically, the following steps are taken to compute the move's `attackScore`: 1. Compute a multiplier based on the move's type effectiveness: - $$ - \text{typeMult} = - \begin{cases} - 2 & \text{if move is super effective (or better)} \\ - -2 & \text{otherwise} - \end{cases} - $$ + $$ + \text{typeMult} = + \begin{cases} + 2 & \text{if move is super effective (or better)} \\ + -2 & \text{otherwise} + \end{cases} + $$ 2. Compute a multiplier based on the move's category and the user's offensive stats: - 1. Compute the user's offensive stat ratio: + 1. Compute the user's offensive stat ratio: + + $$ + \text{statRatio} = + \begin{cases} + \frac{\text{userSpAtk}}{\text{userAtk}} & \text{if move is physical} \\ + \frac{\text{userAtk}}{\text{userSpAtk}} & \text{otherwise} + \end{cases} + $$ + 2. Compute the stat-based multiplier: $$ - \text{statRatio} = + \text{statMult} = \begin{cases} - \frac{\text{userSpAtk}}{\text{userAtk}} & \text{if move is physical} \\ - \frac{\text{userAtk}}{\text{userSpAtk}} & \text{otherwise} + 2 & \text{if statRatio} \leq 0.75 \\ + 1.5 & \text{if } 0.75 \leq \text{statRatio} \leq 0.875 \\ + 1 & \text{otherwise} \end{cases} $$ - 2. Compute the stat-based multiplier: - - $$ - \text{statMult} = - \begin{cases} - 2 & \text{if statRatio} \leq 0.75 \\ - 1.5 & \text{if } 0.75 \leq \text{statRatio} \leq 0.875 \\ - 1 & \text{otherwise} - \end{cases} - $$ 3. Calculate the move's `attackScore`: - $\text{attackScore} = (\text{typeMult}\times \text{statMult})+\lfloor \frac{\text{power}}{5} \rfloor$ + $\text{attackScore} = (\text{typeMult}\times \text{statMult})+\lfloor \frac{\text{power}}{5} \rfloor$ The maximum total multiplier in `attackScore` ($\text{typeMult}\times \text{statMult}$) is 4, which occurs for attacks that are super effective against the target and are categorically aligned with the user's offensive stats (e.g. the move is physical, and the user has much higher Attack than Sp. Atk). The minimum total multiplier of -4 occurs (somewhat confusingly) for attacks that are not super effective but are categorically aligned with the user's offensive stats. @@ -162,19 +163,19 @@ Once the TSS is calculated for each target, the target is selected as follows: 1. Sort the targets (indexes) in decreasing order of their target selection scores (or weights). Let $t_i$ be the index of the *i*-th target in the sorted list, and let $w_i$ be that target's corresponding TSS. 2. Normalize the weights. Let $w_n$ be the lowest-weighted target in the sorted list, then: - $$ - W_i = - \begin{cases} - w_i + |w_n| & \text{if } w_n \text{ is negative} \\ - w_i & \text{otherwise} - \end{cases} - $$ + $$ + W_i = + \begin{cases} + w_i + |w_n| & \text{if } w_n \text{ is negative} \\ + w_i & \text{otherwise} + \end{cases} + $$ 3. Remove all weights from the list such that $W_i < \frac{W_0}{2}$ 4. Generate a random integer $R=\text{rand}(0, W_{\text{total}})$ where $W_{\text{total}}$ is the sum of all the remaining weights after Step 3. 5. For each target $(t_i, W_i)$, - 1. if $R \le \sum_{j=0}^{i} W_i$, or if $t_i$ is the last target in the list, **return** $t_i$ - 2. otherwise, advance to the next target $t_{i+1}$ and repeat this check. + 1. if $R \le \sum_{j=0}^{i} W_i$, or if $t_i$ is the last target in the list, **return** $t_i$ + 2. otherwise, advance to the next target $t_{i+1}$ and repeat this check. Once the target is selected, the enemy has successfully determined its next action for the turn, and its corresponding `EnemyCommandPhase` ends. From here, the `TurnStartPhase` processes the enemy's commands alongside the player's commands and begins to resolve the turn. @@ -183,15 +184,15 @@ Once the target is selected, the enemy has successfully determined its next acti Suppose you enter a single battle against an enemy trainer with the following Pokémon in their party: 1. An [Excadrill](https://bulbapedia.bulbagarden.net/wiki/Excadrill_(Pok%C3%A9mon)) with the Ability Sand Force and the following moveset - 1. Earthquake - 2. Iron Head - 3. Crush Claw - 4. Swords Dance + 1. Earthquake + 2. Iron Head + 3. Crush Claw + 4. Swords Dance 2. A [Heatmor](https://bulbapedia.bulbagarden.net/wiki/Heatmor_(Pok%C3%A9mon)) with the Ability Flash Fire and the following moveset - 1. Fire Lash - 2. Inferno - 3. Hone Claws - 4. Shadow Claw + 1. Fire Lash + 2. Inferno + 3. Hone Claws + 4. Shadow Claw The enemy trainer leads with their Heatmor, and you lead with a [Dachsbun](https://bulbapedia.bulbagarden.net/wiki/Dachsbun_(Pok%C3%A9mon)) with the Ability Well-Baked Body. We'll cover the enemy's behavior over the next two turns. diff --git a/docs/linting.md b/docs/linting.md index ff512740a80..7758444daa3 100644 --- a/docs/linting.md +++ b/docs/linting.md @@ -1,34 +1,64 @@ -# Biome +# Linting & Formatting -## Key Features +> "Any fool can write code that a computer can understand. Good programmers write code that humans can understand." +> +> — Martin Fowler -1. **Automation**: - - A pre-commit hook has been added to automatically run Biome on the added or modified files, ensuring code quality before commits. +Writing clean, readable code is important, and linters and formatters are an integral part of ensuring code quality and readability. +It is for this reason we are using [Biome](https://biomejs.dev), an opinionated linter/formatter (akin to Prettier) with a heavy focus on speed and performance. -2. **Manual Usage**: - - If you prefer not to use the pre-commit hook, you can manually run biome to automatically fix issues using the command: +### Installation +You probably installed Biome already without noticing it - it's included inside `package.json` and should've been downloaded when you ran `npm install` after cloning the repo (assuming you followed proper instructions, that is). If you haven't done that yet, go do it. - ```sh - npx @biomejs/biome --write - ``` +# Using Biome - - Running this command will lint all files in the repository. +For the most part, Biome attempts to stay "out of your hair", letting you write code while enforcing a consistent formatting standard and only notifying for errors it can't automatically fix.\ +On the other hand, if Biome complains about a piece of code, **there's probably a good reason why**. Disable comments should be used sparingly or when readabilty demands it - your first instinct should be to fix the code in question, not disable the rule. -3. **GitHub Action**: - - A GitHub Action has been added to automatically run Biome on every push and pull request, ensuring code quality in the CI/CD pipeline. +## Editor Integration +Biome has integration with many popular code editors. See [these](https://biomejs.dev/guides/editors/first-party-extensions/) [pages](https://biomejs.dev/guides/editors/third-party-extensions/) for information about enabling Biome in your editor of choice. -If you are getting linting errors from biome and want to see which files they are coming from, you can find that out by running biome in a way that is configured to only show the errors for that specific rule: ``npx @biomejs/biome lint --only=category/ruleName`` +## Automated Runs +Generally speaking, most users shouldn't need to run Biome directly; in addition to editor integration, [pre-commit hook](../lefthook.yml) will periodically run Biome before each commit. +You will **not** be able to push code with `error`-level linting problems - fix them beforehand. -## Summary of Biome Rules +We also have a [Github Action](../.github/workflows/quality.yml) to verify code quality each time a PR is updated, preventing bad code from inadvertently making its way upstream. -We use the [recommended ruleset](https://biomejs.dev/linter/rules/) for Biome, with some customizations to better suit our project's needs. +### Why am I getting errors for code I didn't write? + +To save time and minimize friction with existing code, both the pre-commit hook and workflow run will only check files **directly changed** by a given PR or commit. +As a result, changes to files not updated since Biome's introduction can cause any _prior_ linting errors in them to resurface and get flagged. +This should occur less and less often as time passes and more files are updated to the new standard. -For a complete list of rules and their configurations, refer to the `biome.jsonc` file in the project root. +## Running Biome via CLI +If you want Biome to check your files manually, you can run it from the command line like so: + +```sh +npx biome check --[flags] +``` + +A full list of flags and options can be found on [their website](https://biomejs.dev/reference/cli/), but here's a few useful ones to keep in mind: + +- `--write` will cause Biome to write all "safe" fixes and formatting changes directly to your files (rather than just complaining and doing nothing). +- `--changed` and `--staged` will only perform checks on all changed or staged files respectively. Biome sources this info from the relevant version control system (in this case Git). +- `--only=XXX` will filter reported lint issues to a specific category or rule name. Useful for checking errors after Lefthook flags your code (summary screen doesn't show exact line numbers). +- `diagnostic-level=XXX` will only show diagnostics with at least the given severity level (`info/warn/error`). Useful to only focus on errors causing a failed workflow run or similar. + +## Linting Rules + +We primarily use Biome's [recommended ruleset](https://biomejs.dev/linter/rules/) for linting JS/TS, with some customizations to better suit our project's needs[^1]. Some things to consider: -- We have disabled rules that prioritize style over performance, such as `useTemplate` -- Some rules are currently marked as warnings (`warn`) to allow for gradual refactoring without blocking development. Do not write new code that triggers these warnings. -- The linter is configured to ignore specific files and folders, such as large or complex files that are pending refactors, to improve performance and focus on actionable areas. +- We have disabled rules that prioritize style over performance, such as `useTemplate`. +- Some rules are currently disabled or marked as warnings (`warn`) to allow for gradual refactoring without blocking development. **Do not write new code that triggers these warnings.** +- The linter is configured to ignore specific files and folders (such as excessively large files or ones in need of refactoring) to improve performance and focus on actionable areas. -Formatting is also handled by Biome. You should not have to worry about manually formatting your code. +Any questions about linting rules can be brought up in the `#dev-corner` channel in the discord. We are willing to discuss adding or changing existing rules, though not w + +[^1]: A complete list of rules can be found in the `biome.jsonc` file in the project root. + +## What about ESLint? + + +Our project migrated away from ESLint around March 2025 due to it simply not scaling well enough with the codebase's ever-growing size. The [existing eslint rules](../eslint.config.js) are considered _deprecated_, only kept due to Biome lacking the corresponding rules. \ No newline at end of file diff --git a/lefthook.yml b/lefthook.yml index ddf875f15de..ff0ac00f9e5 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -2,13 +2,12 @@ pre-commit: parallel: true commands: biome-lint: - glob: "*.{js,jsx,ts,tsx}" - run: npx @biomejs/biome check --write --reporter=summary {staged_files} --no-errors-on-unmatched + run: npx biome check --write --reporter=summary --staged --no-errors-on-unmatched stage_fixed: true skip: - merge - rebase - + post-merge: commands: update-submodules: