client: make client biome impl equivalent to contracts #2660
Conversation
|
Hi @credence0x! You need to be added as a user to interact with me. Please ask @ponderingdemocritus to add you on the settings page. You are receiving this comment because I am set to review all PRs. That is configurable here. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
client/apps/game/src/three/managers/biome.tsOops! Something went wrong! :( ESLint: 9.18.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by WalkthroughThe pull request introduces a comprehensive implementation of fixed-point arithmetic for biome generation in a game environment. The changes involve creating utility classes for fixed-point numbers ( Changes
Sequence DiagramsequenceDiagram
participant Biome as Biome Manager
participant Noise as Simplex Noise Generator
participant Fixed as Fixed-Point Arithmetic
participant Vec3 as Vector Operations
Biome->>Fixed: Convert input parameters
Biome->>Noise: Generate elevation noise
Noise->>Vec3: Create vector representation
Noise->>Fixed: Perform noise calculations
Noise-->>Biome: Return fixed-point elevation
Biome->>Noise: Generate moisture noise
Noise-->>Biome: Return fixed-point moisture
Biome->>Biome: Determine biome type
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Failed to generate code suggestions for PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
client/apps/game/src/utils/biome/simplex-noise.ts (2)
144-157: Consider removing the unused export 'noise_octaves'.The function
noise_octavesis exported but not used anywhere in the codebase, as indicated by the pipeline failure[warning] 144-144: Unused export 'noise_octaves'. If this function is not needed, consider removing it to clean up the codebase. If it's intended for future use, you might suppress the warning or ensure it's properly utilized.🧰 Tools
🪛 GitHub Actions: knip
[warning] 144-144: Unused export 'noise_octaves'
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
1-157: Run Prettier to fix code formatting issues.The code formatting does not match Prettier standards, as indicated by the pipeline failure. Please run Prettier with the
--writeoption to automatically fix formatting issues.🧰 Tools
🪛 GitHub Actions: knip
[warning] 144-144: Unused export 'noise_octaves'
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
client/apps/game/src/three/managers/biome.ts (2)
Line range hint
1-139: Run Prettier to fix code formatting issues.The code formatting does not match Prettier standards, as indicated by the pipeline failure. Please run Prettier with the
--writeoption to automatically fix formatting issues.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
129-139: Remove commented-out code or relocate it to appropriate test files.The commented-out test code at the end of the file may clutter the production code. Consider removing it if it's no longer needed, or moving it to a dedicated test file to keep the codebase clean and maintainable.
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
client/apps/game/src/utils/biome/fixed-point.ts (2)
67-356: RefactorFixedTraitto use a module or standalone functions instead of a class with only static members.The
FixedTraitclass contains only static methods and no instance members, which is not recommended. According to the static analysis hint(lint/complexity/noStaticOnlyClass), it's preferable to use simple functions or a module. RefactoringFixedTraitinto a module or exporting the functions directly can improve code clarity and adhere to best practices.🧰 Tools
🪛 Biome (1.9.4)
[error] 67-356: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
1-356: Run Prettier to fix code formatting issues.The code formatting does not match Prettier standards, as indicated by the pipeline failure. Please run Prettier with the
--writeoption to automatically fix formatting issues.🧰 Tools
🪛 Biome (1.9.4)
[error] 67-356: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
client/apps/game/src/utils/biome/vec3.ts (1)
9-15: Consider renaming methods and removing redundant factory method.The
newstatic method is redundant as it provides the same functionality as the constructor. Consider:
- Removing the
newmethod as it doesn't add value- Renaming
splatto something more descriptive likeuniformorfill- static new(x: Fixed, y: Fixed, z: Fixed): Vec3 { - return new Vec3(x, y, z); - } - static splat(v: Fixed): Vec3 { + static uniform(v: Fixed): Vec3 { return new Vec3(v, v, v); }🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
client/apps/game/src/utils/biome/vec4.ts (1)
1-71: Fix formatting issues.The pipeline indicates formatting issues. Run Prettier to fix the formatting:
prettier --write "client/apps/game/src/utils/biome/vec*.ts"🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
client/apps/game/src/three/managers/biome.ts(3 hunks)client/apps/game/src/utils/biome/fixed-point.ts(1 hunks)client/apps/game/src/utils/biome/simplex-noise.ts(1 hunks)client/apps/game/src/utils/biome/vec3.ts(1 hunks)client/apps/game/src/utils/biome/vec4.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: knip
client/apps/game/src/utils/biome/simplex-noise.ts
[warning] 144-144: Unused export 'noise_octaves'
🪛 GitHub Actions: lint
client/apps/game/src/utils/biome/simplex-noise.ts
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
client/apps/game/src/three/managers/biome.ts
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
client/apps/game/src/utils/biome/vec4.ts
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
client/apps/game/src/utils/biome/vec3.ts
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
client/apps/game/src/utils/biome/fixed-point.ts
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
🪛 Biome (1.9.4)
client/apps/game/src/utils/biome/fixed-point.ts
[error] 67-356: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
🔇 Additional comments (1)
client/apps/game/src/utils/biome/vec3.ts (1)
1-7: LGTM! Clean and idiomatic TypeScript implementation.The class structure and constructor implementation follow TypeScript best practices by using public constructor parameters for automatic property creation and initialization.
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
| dot(other: Vec3): Fixed { | ||
| let x = this.x.mul(other.x); | ||
| let y = this.y.mul(other.y); | ||
| let z = this.z.mul(other.z); | ||
| return x.add(y).add(z); | ||
| } | ||
|
|
||
| minus(other: Vec3): Vec3 { | ||
| return new Vec3(this.x.sub(other.x), this.y.sub(other.y), this.z.sub(other.z)); | ||
| } | ||
|
|
||
| add(other: Vec3): Vec3 { | ||
| return new Vec3(this.x.add(other.x), this.y.add(other.y), this.z.add(other.z)); | ||
| } | ||
|
|
||
| mul(other: Vec3): Vec3 { | ||
| return new Vec3(this.x.mul(other.x), this.y.mul(other.y), this.z.mul(other.z)); | ||
| } | ||
|
|
||
| mod(other: Vec3): Vec3 { | ||
| return new Vec3(this.x.mod(other.x), this.y.mod(other.y), this.z.mod(other.z)); | ||
| } | ||
|
|
||
| div(other: Vec3): Vec3 { | ||
| return new Vec3(this.x.div(other.x), this.y.div(other.y), this.z.div(other.z)); | ||
| } | ||
|
|
||
| floor(): Vec3 { | ||
| return new Vec3(this.x.floor(), this.y.floor(), this.z.floor()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add essential vector operations and error handling.
The current implementation is missing several common vector operations and safety checks:
- Add length/magnitude calculation
- Add normalization
- Add error handling for division by zero
Consider adding these methods:
length(): Fixed {
return this.dot(this).sqrt();
}
normalize(): Vec3 {
const len = this.length();
if (len.isZero()) {
throw new Error("Cannot normalize zero vector");
}
return this.divScalar(len);
}Also, add division by zero checks in div and divScalar methods:
div(other: Vec3): Vec3 {
+ if (other.x.isZero() || other.y.isZero() || other.z.isZero()) {
+ throw new Error("Division by zero");
+ }
return new Vec3(this.x.div(other.x), this.y.div(other.y), this.z.div(other.z));
}🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
| remScaler(scalar: Fixed): Vec3 { | ||
| return new Vec3(this.x.mod(scalar), this.y.mod(scalar), this.z.mod(scalar)); | ||
| } | ||
|
|
||
| divScalar(scalar: Fixed): Vec3 { | ||
| return new Vec3(this.x.div(scalar), this.y.div(scalar), this.z.div(scalar)); | ||
| } | ||
|
|
||
| mulScalar(scalar: Fixed): Vec3 { | ||
| return new Vec3(this.x.mul(scalar), this.y.mul(scalar), this.z.mul(scalar)); | ||
| } | ||
|
|
||
| addScalar(scalar: Fixed): Vec3 { | ||
| return new Vec3(this.x.add(scalar), this.y.add(scalar), this.z.add(scalar)); | ||
| } | ||
|
|
||
| abs(): Vec3 { | ||
| return new Vec3(this.x.abs(), this.y.abs(), this.z.abs()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix typo in method name and add missing scalar operation.
- The method name
remScalerhas a typo (should beremScalar) - Missing
subScalaroperation for consistency
- remScaler(scalar: Fixed): Vec3 {
+ remScalar(scalar: Fixed): Vec3 {
return new Vec3(this.x.mod(scalar), this.y.mod(scalar), this.z.mod(scalar));
}
+ subScalar(scalar: Fixed): Vec3 {
+ return new Vec3(this.x.sub(scalar), this.y.sub(scalar), this.z.sub(scalar));
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| remScaler(scalar: Fixed): Vec3 { | |
| return new Vec3(this.x.mod(scalar), this.y.mod(scalar), this.z.mod(scalar)); | |
| } | |
| divScalar(scalar: Fixed): Vec3 { | |
| return new Vec3(this.x.div(scalar), this.y.div(scalar), this.z.div(scalar)); | |
| } | |
| mulScalar(scalar: Fixed): Vec3 { | |
| return new Vec3(this.x.mul(scalar), this.y.mul(scalar), this.z.mul(scalar)); | |
| } | |
| addScalar(scalar: Fixed): Vec3 { | |
| return new Vec3(this.x.add(scalar), this.y.add(scalar), this.z.add(scalar)); | |
| } | |
| abs(): Vec3 { | |
| return new Vec3(this.x.abs(), this.y.abs(), this.z.abs()); | |
| } | |
| } | |
| remScalar(scalar: Fixed): Vec3 { | |
| return new Vec3(this.x.mod(scalar), this.y.mod(scalar), this.z.mod(scalar)); | |
| } | |
| divScalar(scalar: Fixed): Vec3 { | |
| return new Vec3(this.x.div(scalar), this.y.div(scalar), this.z.div(scalar)); | |
| } | |
| mulScalar(scalar: Fixed): Vec3 { | |
| return new Vec3(this.x.mul(scalar), this.y.mul(scalar), this.z.mul(scalar)); | |
| } | |
| addScalar(scalar: Fixed): Vec3 { | |
| return new Vec3(this.x.add(scalar), this.y.add(scalar), this.z.add(scalar)); | |
| } | |
| subScalar(scalar: Fixed): Vec3 { | |
| return new Vec3(this.x.sub(scalar), this.y.sub(scalar), this.z.sub(scalar)); | |
| } | |
| abs(): Vec3 { | |
| return new Vec3(this.x.abs(), this.y.abs(), this.z.abs()); | |
| } | |
| } |
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
| import { Fixed } from "./fixed-point"; | ||
|
|
||
| export class Vec4 { | ||
| constructor( | ||
| public x: Fixed, | ||
| public y: Fixed, | ||
| public z: Fixed, | ||
| public w: Fixed | ||
| ) {} | ||
|
|
||
| static new(x: Fixed, y: Fixed, z: Fixed, w: Fixed): Vec4 { | ||
| return new Vec4(x, y, z, w); | ||
| } | ||
|
|
||
| static splat(v: Fixed): Vec4 { | ||
| return new Vec4(v, v, v, v); | ||
| } | ||
|
|
||
| dot(other: Vec4): Fixed { | ||
| let x = this.x.mul(other.x); | ||
| let y = this.y.mul(other.y); | ||
| let z = this.z.mul(other.z); | ||
| let w = this.w.mul(other.w); | ||
| return x.add(y).add(z).add(w); | ||
| } | ||
|
|
||
| minus(other: Vec4): Vec4 { | ||
| return new Vec4(this.x.sub(other.x), this.y.sub(other.y), this.z.sub(other.z), this.w.sub(other.w)); | ||
| } | ||
|
|
||
| add(other: Vec4): Vec4 { | ||
| return new Vec4(this.x.add(other.x), this.y.add(other.y), this.z.add(other.z), this.w.add(other.w)); | ||
| } | ||
|
|
||
| mul(other: Vec4): Vec4 { | ||
| return new Vec4(this.x.mul(other.x), this.y.mul(other.y), this.z.mul(other.z), this.w.mul(other.w)); | ||
| } | ||
|
|
||
| mod(other: Vec4): Vec4 { | ||
| return new Vec4(this.x.mod(other.x), this.y.mod(other.y), this.z.mod(other.z), this.w.mod(other.w)); | ||
| } | ||
|
|
||
| div(other: Vec4): Vec4 { | ||
| return new Vec4(this.x.div(other.x), this.y.div(other.y), this.z.div(other.z), this.w.div(other.w)); | ||
| } | ||
|
|
||
| floor(): Vec4 { | ||
| return new Vec4(this.x.floor(), this.y.floor(), this.z.floor(), this.w.floor()); | ||
| } | ||
|
|
||
|
|
||
| remScaler(scalar: Fixed): Vec4 { | ||
| return new Vec4(this.x.mod(scalar), this.y.mod(scalar), this.z.mod(scalar), this.w.mod(scalar)); | ||
| } | ||
|
|
||
| divScalar(scalar: Fixed): Vec4 { | ||
| return new Vec4(this.x.div(scalar), this.y.div(scalar), this.z.div(scalar), this.w.div(scalar)); | ||
| } | ||
|
|
||
| mulScalar(scalar: Fixed): Vec4 { | ||
| return new Vec4(this.x.mul(scalar), this.y.mul(scalar), this.z.mul(scalar), this.w.mul(scalar)); | ||
| } | ||
|
|
||
| addScalar(scalar: Fixed): Vec4 { | ||
| return new Vec4(this.x.add(scalar), this.y.add(scalar), this.z.add(scalar), this.w.add(scalar)); | ||
| } | ||
|
|
||
| abs(): Vec4 { | ||
| return new Vec4(this.x.abs(), this.y.abs(), this.z.abs(), this.w.abs()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a generic base class to reduce code duplication.
The Vec4 implementation largely duplicates code from Vec3. Consider creating a generic base class to share common functionality:
abstract class VecBase<T extends VecBase<T>> {
protected abstract create(...components: Fixed[]): T;
abstract dot(other: T): Fixed;
mul(other: T): T {
// Implement common logic
}
// ... other common methods
}
class Vec3 extends VecBase<Vec3> {
constructor(public x: Fixed, public y: Fixed, public z: Fixed) {
super();
}
protected create(x: Fixed, y: Fixed, z: Fixed): Vec3 {
return new Vec3(x, y, z);
}
// ... Vec3 specific methods
}
class Vec4 extends VecBase<Vec4> {
constructor(public x: Fixed, public y: Fixed, public z: Fixed, public w: Fixed) {
super();
}
protected create(x: Fixed, y: Fixed, z: Fixed, w: Fixed): Vec4 {
return new Vec4(x, y, z, w);
}
// ... Vec4 specific methods
}Also:
- Fix the extra blank line at line 51
- Apply the same improvements suggested for Vec3 (method naming, error handling, additional operations)
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
| FixedTrait.divi(FixedTrait.fromInt(1n), FixedTrait.fromInt(10n)) // 0.1 | ||
| ]; | ||
| const ELEVATION_OCTAVES_SUM = ELEVATION_OCTAVES.reduce((a, b) => a.add(b), FixedTrait.ZERO); | ||
|
|
||
|
|
||
| export type BiomeType = |
There was a problem hiding this comment.
should we use an enum instead ?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
client/apps/game/src/three/managers/biome.ts (3)
74-91: Extract magic numbers into named constants.Consider extracting magic numbers like
100and2into named constants for better maintainability and clarity.+const HUNDRED = FixedTrait.fromInt(100n); +const TWO = FixedTrait.fromInt(2n); private calculateElevation( col: number, row: number, mapAmplitude: Fixed, octaves: Fixed[], octavesSum: Fixed, ): Fixed { let elevation = FixedTrait.ZERO; - let _100 = FixedTrait.fromInt(100n); - let _2 = FixedTrait.fromInt(2n); for (const octave of octaves) { let x = FixedTrait.fromInt(BigInt(col)).div(octave).div(mapAmplitude); let z = FixedTrait.fromInt(BigInt(row)).div(octave).div(mapAmplitude); let sn = snoise(Vec3.new(x, FixedTrait.ZERO, z)); - const noise = sn.add(FixedTrait.ONE).mul(_100).div(_2); + const noise = sn.add(FixedTrait.ONE).mul(HUNDRED).div(TWO); elevation = elevation.add(octave.mul(noise.floor())); } - return elevation.div(octavesSum).div(FixedTrait.fromInt(100n)); + return elevation.div(octavesSum).div(HUNDRED);🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
95-99: Reuse the same constants from calculateElevation.For consistency, use the same constants (
HUNDREDandTWO) suggested forcalculateElevation.private calculateMoisture(col: number, row: number, mapAmplitude: Fixed, moistureOctave: Fixed): Fixed { const moistureX = moistureOctave.mul(FixedTrait.fromInt(BigInt(col))).div(mapAmplitude); const moistureZ = moistureOctave.mul(FixedTrait.fromInt(BigInt(row))).div(mapAmplitude); - const noise = snoise(Vec3.new(moistureX, FixedTrait.ZERO, moistureZ)).add(FixedTrait.ONE).mul(FixedTrait.fromInt(100n)).div(FixedTrait.fromInt(2n)); + const noise = snoise(Vec3.new(moistureX, FixedTrait.ZERO, moistureZ)).add(FixedTrait.ONE).mul(HUNDRED).div(TWO); return FixedTrait.floor(noise).div(FixedTrait.fromInt(100n));🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
102-127: Extract moisture thresholds into named constants.Consider extracting moisture threshold ratios into named constants for better maintainability and readability.
+const MOISTURE_THRESHOLDS = { + VERY_DRY: FixedTrait.fromRatio(10n, 100n), + DRY: FixedTrait.fromRatio(16n, 100n), + MODERATE: FixedTrait.fromRatio(33n, 100n), + MOIST: FixedTrait.fromRatio(40n, 100n), + HUMID: FixedTrait.fromRatio(50n, 100n), + VERY_HUMID: FixedTrait.fromRatio(66n, 100n), + WET: FixedTrait.fromRatio(83n, 100n), +}; private determineBiome(elevation: Fixed, moisture: Fixed, level: typeof LEVEL): BiomeType { if (elevation.value < level.DEEP_OCEAN.value) return BiomeType.DeepOcean; if (elevation.value < level.OCEAN.value) return BiomeType.Ocean; if (elevation.value < level.SAND.value) return BiomeType.Beach; if (elevation.value > level.MOUNTAIN.value) { - if (moisture.value < FixedTrait.fromRatio(10n, 100n).value) return BiomeType.Scorched; - if (moisture.value < FixedTrait.fromRatio(40n, 100n).value) return BiomeType.Bare; - if (moisture.value < FixedTrait.fromRatio(50n, 100n).value) return BiomeType.Tundra; + if (moisture.value < MOISTURE_THRESHOLDS.VERY_DRY.value) return BiomeType.Scorched; + if (moisture.value < MOISTURE_THRESHOLDS.MOIST.value) return BiomeType.Bare; + if (moisture.value < MOISTURE_THRESHOLDS.HUMID.value) return BiomeType.Tundra; return BiomeType.Snow; }🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
client/apps/game/src/utils/biome/fixed-point.ts (3)
18-65: Add readonly modifier to prevent accidental value mutations.Consider adding the
readonlymodifier to thevalueproperty to prevent accidental mutations.export class Fixed { - public value: bigint; + public readonly value: bigint; constructor(value: bigint) { this.value = value; }🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
67-352: Convert FixedTrait class to namespace for better TypeScript idioms.The static analysis tool correctly flags that this class contains only static members. Consider converting it to a namespace, which is more idiomatic in TypeScript for this use case.
-export class FixedTrait { +export namespace FixedTrait { static ONE_64x64 = ONE_64x64; // ... rest of the implementation🧰 Tools
🪛 Biome (1.9.4)
[error] 67-352: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
303-334: Make Newton's method iterations configurable.Consider making the number of Newton's method iterations configurable to allow trade-offs between precision and performance.
+const DEFAULT_NEWTON_ITERATIONS = 7; -static sqrt(x: Fixed): Fixed { +static sqrt(x: Fixed, iterations: number = DEFAULT_NEWTON_ITERATIONS): Fixed { // ... initial code ... // Newton's method iterations r = (r + x.value / r) >> 1n; - r = (r + x.value / r) >> 1n; - r = (r + x.value / r) >> 1n; - r = (r + x.value / r) >> 1n; - r = (r + x.value / r) >> 1n; - r = (r + x.value / r) >> 1n; - r = (r + x.value / r) >> 1n; + for (let i = 1; i < iterations; i++) { + r = (r + x.value / r) >> 1n; + }🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/apps/game/src/three/managers/biome.ts(3 hunks)client/apps/game/src/utils/biome/fixed-point.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: lint
client/apps/game/src/three/managers/biome.ts
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
client/apps/game/src/utils/biome/fixed-point.ts
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
🪛 Biome (1.9.4)
client/apps/game/src/utils/biome/fixed-point.ts
[error] 67-352: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
🔇 Additional comments (4)
client/apps/game/src/three/managers/biome.ts (3)
1-13: LGTM! Constants correctly converted to fixed-point.The conversion of constants to fixed-point using
FixedTraitis well-implemented and aligns with the PR objective.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
16-33: LGTM! BiomeType successfully converted to enum.The conversion from union type to enum improves type safety and maintainability, as previously suggested.
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
55-60: LGTM! Level thresholds correctly converted to fixed-point.The conversion of level thresholds using
FixedTrait.fromRatiomaintains precision while aligning with the fixed-point implementation.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
client/apps/game/src/utils/biome/fixed-point.ts (1)
150-204: Add test cases for multiplication and division edge cases.The custom implementation of
mulanddivdiffers from ABDK Math and handles signs separately. Consider adding comprehensive test cases to verify behavior, especially for edge cases like:
- Numbers close to MIN_64x64/MAX_64x64
- Numbers with different signs
- Numbers with extreme differences in magnitude
Would you like me to help create these test cases?
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards. Run Prettier with --write to fix.
Summary by CodeRabbit
Release Notes
New Features
Vec3) and four-dimensional (Vec4) vector operations.Technical Improvements
Performance