mirror of
https://github.com/pagefaultgames/pokerogue.git
synced 2025-07-06 08:22:16 +02:00
Update linting.md, fix lefthook etc.
This commit is contained in:
parent
a09e8a8183
commit
269380b94c
@ -99,10 +99,10 @@
|
||||
"linter": {
|
||||
"rules": {
|
||||
"performance": {
|
||||
"noDelete": "off"
|
||||
"noDelete": "off" // ????
|
||||
},
|
||||
"style": {
|
||||
"noNamespaceImport": "off"
|
||||
"noNamespaceImport": "off" // TODO: Refactor and mark as fixed
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -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.
|
||||
- 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 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
|
||||
- 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 */
|
||||
/** 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
|
||||
...
|
||||
*/
|
||||
```
|
||||
|
@ -54,13 +54,13 @@ 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
|
||||
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 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.
|
||||
@ -73,14 +73,15 @@ In `getNextMove()`, the enemy Pokémon chooses a move to use in the following st
|
||||
|
||||
### 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:
|
||||
|
@ -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.
|
||||
|
||||
# Using Biome
|
||||
|
||||
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.
|
||||
|
||||
## 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.
|
||||
|
||||
## 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.
|
||||
|
||||
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.
|
||||
|
||||
### Why am I getting errors for code I didn't write?
|
||||
<!-- TODO: Remove this if/when we perform a project wide linting spree -->
|
||||
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.
|
||||
|
||||
## 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 @biomejs/biome --write
|
||||
npx biome check --[flags]
|
||||
```
|
||||
|
||||
- Running this command will lint all files in the repository.
|
||||
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:
|
||||
|
||||
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.
|
||||
- `--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.
|
||||
|
||||
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``
|
||||
## Linting Rules
|
||||
|
||||
## Summary of Biome Rules
|
||||
|
||||
We use the [recommended ruleset](https://biomejs.dev/linter/rules/) for Biome, with some customizations to better suit our project's needs.
|
||||
|
||||
For a complete list of rules and their configurations, refer to the `biome.jsonc` file in the project root.
|
||||
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?
|
||||
|
||||
<!-- Remove if/when we finally ditch eslint for good -->
|
||||
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.
|
@ -2,8 +2,7 @@ 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
|
||||
|
Loading…
Reference in New Issue
Block a user