Merge pull request #2 from DayKev/pr/5682-review

This commit is contained in:
Bertie690 2025-04-29 10:44:20 -04:00 committed by GitHub
commit 71ee27d487
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 66 additions and 46 deletions

View File

@ -99,10 +99,10 @@
"linter": { "linter": {
"rules": { "rules": {
"performance": { "performance": {
"noDelete": "off" // ???? "noDelete": "off" // TODO: evaluate if this is necessary for the test(s) to function
}, },
"style": { "style": {
"noNamespaceImport": "off" // TODO: Refactor and mark as fixed "noNamespaceImport": "off" // this is required for `vi.spyOn` to work in some tests
} }
} }
} }

View File

@ -1,60 +1,84 @@
# Commenting code # Commenting code
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 to function. 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: ## General Guidelines
While we're not enforcing a strict standard, here are some things to keep in mind:
- Make comments meaningful - Make comments meaningful
- Comments should **NOT** repeat _what_ code _does_[^1] or explain concepts obvious to someone with a basic understanding of the language at hand. Instead, focus on explaining _why_ a line or block of code exists - its "raison d'être", if you will. - Comments should **NOT** repeat _what_ code _does_[^1] or explain concepts obvious to someone with a basic understanding of the language at hand. Instead, focus on explaining _why_ a line or block of code exists.
- Anyone with basic reading comprehension and a good IDE can figure out what code does; gaining a _post hoc_ understanding of the _reasons_ behind its existence takes a lot more digging, effort and bloodshed. - Anyone with basic reading comprehension and a good IDE can figure out what code does; gaining a _post hoc_ understanding of the _reasons_ behind its existence takes a lot more digging, effort and bloodshed.
- Keep comments readable - Keep comments readable
- A comment's verbosity should roughly scale with the complexity of its subject matter. Some people naturally write shorter or longer comments as a personal style, but summarizing a 300 line function with "does a thing" is about as good as writing nothing. Conversely, writing a paragraph-level response where a basic one-liner would suffice is equally unhelpful. - A comment's verbosity should roughly scale with the complexity of its subject matter. Some people naturally write shorter or longer comments as a personal style, but summarizing a 300 line function with "does a thing" is about as good as writing nothing. Conversely, writing a paragraph-level response where a basic one-liner would suffice is no less undesirable.
- Long comments should ideally be broken into multiple lines at around the 100-120 character mark. This isn't _mandatory_, but avoids unnecessary scrolling in terminals and IDEs. - Long comments should ideally be broken into multiple lines at around the 100-120 character mark. This isn't _mandatory_, but avoids unnecessary scrolling in terminals and IDEs.
- Make sure comments exist on Functions, Classes, Methods, and Properties - Make sure comments exist on Functions, Classes, Methods, and Properties
- 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. - 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.
[^1]: With exceptions for extremely long, convoluted or unintuitive methods (though an over-dependency on said comments is likely asymptomatic of poorly structured code). [^1]: With exceptions for extremely long, convoluted or unintuitive methods (though an over-dependency on said comments is likely a symptom of poorly structured code).
Given all this, look at the following
# TSDoc # TSDoc
Taking all these together, one notable example of good comments are those adhering to the TSDoc standard. These comments (frequentl) The codebase makes extensive use of [TSDoc](https://tsdoc.org), which is a TypeScript-specific version of [JSDoc](https://jsdoc.app/about-getting-started)
When formatted this way, these comments are shown within VS Code or similar IDEs just by hovering over the function! that uses similar syntax and attaches to functions, classes, etc.
When formatted correctly, these comments are shown within VS Code or similar IDEs just by hovering over the function or object.
- Functions also show the comment for each parameter as you type them, making keeping track of arguments inside lengthy functions much more clear. - Functions also show the comment for each parameter as you type them, making keeping track of arguments inside lengthy functions much more clear.
They can also be used to generate a commentated overview of the codebase. There is a GitHub action that automatically updates [this docs site](https://pagefaultgames.github.io/pokerogue/main/index.html)
and you can generate it locally as well via `npm run docs` which will generate into the `typedoc/` directory.
## Syntax ## Syntax
For an example of how TSDoc comments work, here are several real examples of TSDoc comments at work, taken from various files inside this very repo[^2]: For an example of how TSDoc comments work, here are some TSDoc comments taken from `src/data/moves/move.ts`:
```ts ```ts
/** /**
* Change the type-based weather modifier if this move's power would be reduced by it. * Attribute to put in a {@link https://bulbapedia.bulbagarden.net/wiki/Substitute_(doll) | Substitute Doll} for the user.
* @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 (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 { export class AddSubstituteAttr extends MoveEffectAttr {
} /** The ratio of the user's max HP that is required to apply this effect */
private hpCost: number;
/** Whether the damage taken should be rounded up (Shed Tail rounds up) */
private roundUp: boolean;
/** Class containing configurable user settings. */ constructor(hpCost: number, roundUp: boolean) {
class Settings { // code removed
/** 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 * Removes 1/4 of the user's maximum HP (rounded down) to create a substitute for the user
* @param user - The {@linkcode Pokemon} that used the move.
* @param target - n/a
* @param move - The {@linkcode Move} with this attribute.
* @param args - n/a
* @returns `true` if the attribute successfully applies, `false` otherwise
*/ */
export class DontHealThePartyPlsAttr extends MoveAttr { apply(user: Pokemon, target: Pokemon, move: Move, args: any[]): boolean {
} // code removed
}
getUserBenefitScore(user: Pokemon, target: Pokemon, move: Move): number {
// code removed
}
getCondition(): MoveConditionFunc {
// code removed
}
/**
* Get the substitute-specific failure message if one should be displayed.
* @param user - The pokemon using the move.
* @returns The substitute-specific failure message if the conditions apply, otherwise `undefined`
*/
getFailedText(user: Pokemon, _target: Pokemon, _move: Move): string | undefined {
// code removed
}
}
``` ```
Looking at the example given, you'll notice this contains an `{@linkcode XYZ}` tag for each parameter. This provides an easy type denomination and hyperlink to that type in most modern IDEs. (`@linkcode` is used here instead of `@link` so that the text appears in monospace which is more obviously a `type` rather than a random hyperlink.) \ Looking at the example given, you'll notice this contains an `{@linkcode XYZ}` tag in some of the parameters. This provides a clickable hyperlink to that type or object in most modern IDEs. (`@linkcode` is used here instead of `@link` so that the text appears in monospace which is more obviously a `type` rather than a random hyperlink.) \
Also note the dashes (` - `) between the parameter names and descriptions - these are **mandatory** under the TSDoc spec[^3]. Also note the dashes (` - `) between the parameter names and descriptions - these are **mandatory** under the TSDoc spec[^2].
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 [on the TSDoc website](https://tsdoc.org).
The [playground page](https://tsdoc.org/play/) there can also be used for live testing of examples.
[^3]: Incidentally, this is also the only place dashes are explicitly _required_. [^2]: Incidentally, this is also the only place dashes are explicitly _required_.
### What not to do: ### What not to do:
- Don't leave comments for code you don't understand - Don't leave comments for code you don't understand

View File

@ -122,7 +122,6 @@ import { MoveFlags } from "#enums/MoveFlags";
import { MoveEffectTrigger } from "#enums/MoveEffectTrigger"; import { MoveEffectTrigger } from "#enums/MoveEffectTrigger";
import { MultiHitType } from "#enums/MultiHitType"; import { MultiHitType } from "#enums/MultiHitType";
import { invalidAssistMoves, invalidCopycatMoves, invalidMetronomeMoves, invalidMirrorMoveMoves, invalidSleepTalkMoves } from "./invalid-moves"; import { invalidAssistMoves, invalidCopycatMoves, invalidMetronomeMoves, invalidMirrorMoveMoves, invalidSleepTalkMoves } from "./invalid-moves";
import { TrainerVariant } from "#app/field/trainer";
import { SelectBiomePhase } from "#app/phases/select-biome-phase"; import { SelectBiomePhase } from "#app/phases/select-biome-phase";
type MoveConditionFunc = (user: Pokemon, target: Pokemon, move: Move) => boolean; type MoveConditionFunc = (user: Pokemon, target: Pokemon, move: Move) => boolean;
@ -1803,10 +1802,7 @@ export class HalfSacrificialAttr extends MoveEffectAttr {
} }
/** /**
* Attribute to put in a {@link https://bulbapedia.bulbagarden.net/wiki/Substitute_(doll) | Substitute Doll} * Attribute to put in a {@link https://bulbapedia.bulbagarden.net/wiki/Substitute_(doll) | Substitute Doll} for the user.
* for the user.
* @extends MoveEffectAttr
* @see {@linkcode apply}
*/ */
export class AddSubstituteAttr extends MoveEffectAttr { export class AddSubstituteAttr extends MoveEffectAttr {
/** The ratio of the user's max HP that is required to apply this effect */ /** The ratio of the user's max HP that is required to apply this effect */
@ -1823,11 +1819,11 @@ export class AddSubstituteAttr extends MoveEffectAttr {
/** /**
* Removes 1/4 of the user's maximum HP (rounded down) to create a substitute for the user * Removes 1/4 of the user's maximum HP (rounded down) to create a substitute for the user
* @param user the {@linkcode Pokemon} that used the move. * @param user - The {@linkcode Pokemon} that used the move.
* @param target n/a * @param target - n/a
* @param move the {@linkcode Move} with this attribute. * @param move - The {@linkcode Move} with this attribute.
* @param args n/a * @param args - n/a
* @returns true if the attribute successfully applies, false otherwise * @returns `true` if the attribute successfully applies, `false` otherwise
*/ */
apply(user: Pokemon, target: Pokemon, move: Move, args: any[]): boolean { apply(user: Pokemon, target: Pokemon, move: Move, args: any[]): boolean {
if (!super.apply(user, target, move, args)) { if (!super.apply(user, target, move, args)) {
@ -1848,12 +1844,12 @@ export class AddSubstituteAttr extends MoveEffectAttr {
} }
getCondition(): MoveConditionFunc { getCondition(): MoveConditionFunc {
return (user, target, move) => !user.getTag(SubstituteTag) && user.hp > (this.roundUp ? Math.ceil(user.getMaxHp() * this.hpCost) : Math.floor(user.getMaxHp() * this.hpCost)) && user.getMaxHp() > 1; return (user, _target, _move) => !user.getTag(SubstituteTag) && user.hp > (this.roundUp ? Math.ceil(user.getMaxHp() * this.hpCost) : Math.floor(user.getMaxHp() * this.hpCost)) && user.getMaxHp() > 1;
} }
/** /**
* Get the substitute-specific failure message if one should be displayed. * Get the substitute-specific failure message if one should be displayed.
* @param user The pokemon using the move. * @param user - The pokemon using the move.
* @returns The substitute-specific failure message if the conditions apply, otherwise `undefined` * @returns The substitute-specific failure message if the conditions apply, otherwise `undefined`
*/ */
getFailedText(user: Pokemon, _target: Pokemon, _move: Move): string | undefined { getFailedText(user: Pokemon, _target: Pokemon, _move: Move): string | undefined {