Change block shading to better match vanilla#534
Change block shading to better match vanilla#534anodic01 wants to merge 5 commits intozardoy:nextfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughPer-face lighting in the mesher was reworked to separate unscaled face light and directional side shading, AO math adjusted with theme-dependent bias/scale; mesher/worldrenderer configs gained Changes
Sequence Diagram(s)sequenceDiagram
participant Mesher as Mesher
participant Mesh as Mesh (vertex colors)
participant Renderer as Renderer (Material)
participant GPU as GPU
Note over Mesher,Mesh: Precompute per-vertex lighting (directional + AO)
Mesher->>Mesher: compute sideShading, faceLight, AO\ninterpolate cornerLightResult
Mesher->>Mesh: write vertex colors (lighting + AO)
Note over Renderer,GPU: Unlit rendering of baked colors
Renderer->>Mesh: assign MeshBasicMaterial (vertexColors)
Renderer->>GPU: submit mesh with precomputed colors
GPU->>GPU: rasterize using vertex colors (no Lambert lighting)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@renderer/viewer/lib/mesher/models.ts`:
- Line 461: The assignment to light violates ESLint operator-spacing rules;
update the expression in the light assignment (the line using sideShading, ao,
and cornerLightResult) to add spaces around the + and * operators so it reads
with consistent spacing (e.g., use "0.4 + 0.2 * ao" inside the parentheses)
while keeping the existing operands sideShading and (cornerLightResult / 15).
- Around line 390-392: Update the expression and comments around
sideShading/faceLight/baseLight: add spaces around all operators in the
sideShading calculation to satisfy ESLint (e.g., around +, *, -, / and function
args), and add a brief comment above the sideShading assignment documenting the
vanilla shading constants (up=1.0, down=0.5, north/south=0.8, east/west=0.6) and
how the magic numbers map to those values; keep the subsequent faceLight =
world.getLight(neighborPos, undefined, undefined, block.name) and baseLight =
sideShading * faceLight / 15 code logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7b37f527-91e7-40e5-bef7-2cff5d860b5f
📒 Files selected for processing (2)
renderer/viewer/lib/mesher/models.tsrenderer/viewer/three/worldrendererThree.ts
|
On line 432 was supposed to be changed to faceLight, oops |
|
backport to minecraft-renderer |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@renderer/viewer/lib/mesher/models.ts`:
- Around line 390-392: The face shading is being applied twice: sideShading is
multiplied into baseLight (const baseLight = sideShading * faceLight / 15) and
then applied again later when smoothLighting is false (cornerLightResult and
later multiplication). Change the computation so baseLight is the normalized
face light (const baseLight = faceLight / 15) and only apply sideShading once in
the non-smooth path (use baseLight * sideShading when smoothLighting is false)
while leaving the smoothLighting path to use its existing corner interpolation;
update references in this block (sideShading, faceLight, baseLight,
cornerLightResult, smoothLighting, world.getLight) accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 365dc495-ad68-4715-af02-0fa6b4edcfde
📒 Files selected for processing (1)
renderer/viewer/lib/mesher/models.ts
renderer/viewer/lib/mesher/models.ts
Outdated
| const sideShading = 0.75 + 0.25*dir[1] + 0.05*(Math.abs(dir[2])-3*Math.abs(dir[0])) | ||
| const faceLight = world.getLight(neighborPos, undefined, undefined, block.name) | ||
| const baseLight = sideShading * faceLight / 15 |
There was a problem hiding this comment.
Prevent double-applying directional shading in non-smooth lighting.
Line 392 makes baseLight already directional (sideShading * faceLight / 15), but downstream initialization still uses cornerLightResult = baseLight * 15 (Line 432) and Line 461 multiplies by sideShading again. This squares directional shading when smoothLighting is off, causing over-dark faces.
🔧 Proposed fix
- const sideShading = 0.75 + 0.25*dir[1] + 0.05*(Math.abs(dir[2])-3*Math.abs(dir[0]))
+ const sideShading = 0.75 + 0.25 * dir[1] + 0.05 * (Math.abs(dir[2]) - 3 * Math.abs(dir[0]))
const faceLight = world.getLight(neighborPos, undefined, undefined, block.name)
const baseLight = sideShading * faceLight / 15
@@
- let cornerLightResult = baseLight * 15
+ let cornerLightResult = faceLight🧰 Tools
🪛 ESLint
[error] 390-390: Operator '*' must be spaced.
(@stylistic/space-infix-ops)
[error] 390-390: Operator '*' must be spaced.
(@stylistic/space-infix-ops)
[error] 390-390: Operator '-' must be spaced.
(@stylistic/space-infix-ops)
[error] 390-390: Operator '*' must be spaced.
(@stylistic/space-infix-ops)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@renderer/viewer/lib/mesher/models.ts` around lines 390 - 392, The face
shading is being applied twice: sideShading is multiplied into baseLight (const
baseLight = sideShading * faceLight / 15) and then applied again later when
smoothLighting is false (cornerLightResult and later multiplication). Change the
computation so baseLight is the normalized face light (const baseLight =
faceLight / 15) and only apply sideShading once in the non-smooth path (use
baseLight * sideShading when smoothLighting is false) while leaving the
smoothLighting path to use its existing corner interpolation; update references
in this block (sideShading, faceLight, baseLight, cornerLightResult,
smoothLighting, world.getLight) accordingly.
|
Do I need to make another with the smooth-lighting-off case fixed or can you edit it yourself before merging it? |
|
/delpoy |
|
Feel free to edit it |
|
/deploy |
|
Deployed to Vercel Preview: https://prismarine-6rgdjufmr-zaro.vercel.app |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
renderer/viewer/lib/mesher/models.ts (1)
390-390:⚠️ Potential issue | 🟡 MinorFix operator spacing to satisfy ESLint rules.
The
sideShadingexpression has multiple spacing violations flagged by@stylistic/space-infix-ops.🔧 Proposed fix
- const sideShading = 0.75 + 0.25*dir[1] + 0.05*(Math.abs(dir[2])-3*Math.abs(dir[0])) + const sideShading = 0.75 + 0.25 * dir[1] + 0.05 * (Math.abs(dir[2]) - 3 * Math.abs(dir[0]))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@renderer/viewer/lib/mesher/models.ts` at line 390, The expression assigning sideShading contains missing spaces around infix operators; update the calculation in the sideShading assignment so all binary operators have surrounding spaces (e.g. use 0.25 * dir[1], 0.05 * (Math.abs(dir[2]) - 3 * Math.abs(dir[0])), and add spaces around the + and - operators) to satisfy `@stylistic/space-infix-ops` while keeping the same numeric logic.
🧹 Nitpick comments (1)
renderer/viewer/lib/mesher/models.ts (1)
390-392: Verify the AO formula constants match vanilla Minecraft.The PR objective states these AO values should "match vanilla Minecraft exactly." The formula
0.4 + 0.2 * ao(whereaoranges 0–3) produces values 0.4, 0.6, 0.8, 1.0 for the respective AO levels.Combined with the
sideShadingformula, this is a significant change to how blocks appear. Consider adding a brief comment documenting the vanilla reference or derivation of these constants for future maintainers.Also applies to: 461-461
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@renderer/viewer/lib/mesher/models.ts` around lines 390 - 392, The AO/side shading constants in the sideShading calculation (symbol sideShading) do not clearly match vanilla Minecraft; replace or validate the coefficients so the per-AO multiplier uses vanilla's 0.4 + 0.2*ao pattern (ao in 0..3) and ensure baseLight (derived from faceLight via world.getLight) uses that value; update the sideShading expression(s) at the occurrences around sideShading/faceLight/baseLight (also the identical expression at the later occurrence) to match vanilla numbers exactly and add a short inline comment referencing the vanilla formula (e.g., "vanilla AO: 0.4 + 0.2 * ao") so future maintainers know the source.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@renderer/viewer/lib/mesher/models.ts`:
- Line 390: The expression assigning sideShading contains missing spaces around
infix operators; update the calculation in the sideShading assignment so all
binary operators have surrounding spaces (e.g. use 0.25 * dir[1], 0.05 *
(Math.abs(dir[2]) - 3 * Math.abs(dir[0])), and add spaces around the + and -
operators) to satisfy `@stylistic/space-infix-ops` while keeping the same numeric
logic.
---
Nitpick comments:
In `@renderer/viewer/lib/mesher/models.ts`:
- Around line 390-392: The AO/side shading constants in the sideShading
calculation (symbol sideShading) do not clearly match vanilla Minecraft; replace
or validate the coefficients so the per-AO multiplier uses vanilla's 0.4 +
0.2*ao pattern (ao in 0..3) and ensure baseLight (derived from faceLight via
world.getLight) uses that value; update the sideShading expression(s) at the
occurrences around sideShading/faceLight/baseLight (also the identical
expression at the later occurrence) to match vanilla numbers exactly and add a
short inline comment referencing the vanilla formula (e.g., "vanilla AO: 0.4 +
0.2 * ao") so future maintainers know the source.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 38958f46-fa7e-48f9-a09b-5b71896f90a0
📒 Files selected for processing (1)
renderer/viewer/lib/mesher/models.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
renderer/viewer/lib/mesher/models.ts (1)
421-422: Remove trailing whitespace on empty line.Line 422 appears to have trailing whitespace.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@renderer/viewer/lib/mesher/models.ts` around lines 421 - 422, Remove the trailing whitespace on the empty line following the declaration of the variable "light" (the statement "let light = 1") in models.ts; delete the extra spaces on that blank line so the file has no trailing whitespace there.src/watchOptions.ts (1)
110-113: Existing chunks won't update whenshadingThemechanges.The subscription correctly syncs the option to
inWorldRenderingConfig, butgetMesherConfig()is only called when chunks are meshed. Existing chunks retain their original shading until re-meshed (e.g., via block updates or chunk reload). This matches howsmoothLightingworks, so it's consistent—but users may need to reload chunks to see the change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/watchOptions.ts` around lines 110 - 113, The current subscribeKey callback only updates appViewer.inWorldRenderingConfig.shadingTheme but doesn't re-mesh existing chunks, so existing geometry keeps the old shading; after setting appViewer.inWorldRenderingConfig.shadingTheme in the subscribeKey callback (and where initial assignment occurs) invoke the code path that updates already-loaded chunks—e.g., call the same remeshing logic used when chunk options change: obtain getMesherConfig() and either iterate loaded chunks to mark them dirty / enqueue them for remesh or call the engine's chunk-invalidator/remesher (the function in your codebase that performs per-chunk remesh) so all current chunks are reprocessed with the new shadingTheme. Ensure this runs inside the subscribeKey callback so updates appear immediately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@renderer/viewer/lib/mesher/models.ts`:
- Around line 462-467: The variable ao is never reassigned and the code has
stray semicolons; change the declaration of ao from let to const (so const ao =
(side1Block && side2Block) ? 0 : (3 - (side1Block + side2Block + cornerBlock)))
and remove any extraneous semicolons around the ao_bias and ao_scale
declarations and the subsequent light calculation; keep usage of ao_bias,
ao_scale and light intact (they remain const/let as currently intended) so only
the ao declaration and stray semicolons are fixed.
- Around line 390-395: The block defining sideShading and baseLight has ESLint
style issues: fix indentation to 6 spaces, ensure proper operator spacing around
+ and *, replace the zero fraction 0.0 with 0, and remove any stray semicolons;
specifically update the expressions using shadingTheme, dir, and faceLight in
the sideShading and baseLight calculations (refer to variables/shims:
smoothLighting, shadingTheme, world.getLight(neighborPos, undefined, undefined,
block.name), faceLight, sideShading, baseLight, dir) so the conditional ternary
branches follow the project's spacing/indentation rules and the numeric literal
uses 0 instead of 0.0.
In `@src/defaultOptions.ts`:
- Around line 255-260: Fix ESLint comma-spacing in the shadingTheme
configuration by adding a space after each comma inside the possibleValues
entries; update the possibleValues array used by shadingTheme (the two inner
arrays currently written as ['high-contrast','High Contrast'] and
['vanilla','Vanilla']) so they read with a space after the comma (e.g.,
['high-contrast', 'High Contrast'] and ['vanilla', 'Vanilla']) to satisfy the
comma-spacing rule.
---
Nitpick comments:
In `@renderer/viewer/lib/mesher/models.ts`:
- Around line 421-422: Remove the trailing whitespace on the empty line
following the declaration of the variable "light" (the statement "let light =
1") in models.ts; delete the extra spaces on that blank line so the file has no
trailing whitespace there.
In `@src/watchOptions.ts`:
- Around line 110-113: The current subscribeKey callback only updates
appViewer.inWorldRenderingConfig.shadingTheme but doesn't re-mesh existing
chunks, so existing geometry keeps the old shading; after setting
appViewer.inWorldRenderingConfig.shadingTheme in the subscribeKey callback (and
where initial assignment occurs) invoke the code path that updates
already-loaded chunks—e.g., call the same remeshing logic used when chunk
options change: obtain getMesherConfig() and either iterate loaded chunks to
mark them dirty / enqueue them for remesh or call the engine's
chunk-invalidator/remesher (the function in your codebase that performs
per-chunk remesh) so all current chunks are reprocessed with the new
shadingTheme. Ensure this runs inside the subscribeKey callback so updates
appear immediately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e5b8a767-a97b-40cc-9f4c-9806b729d55d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
renderer/viewer/lib/mesher/models.tsrenderer/viewer/lib/mesher/shared.tsrenderer/viewer/lib/worldrendererCommon.tssrc/defaultOptions.tssrc/optionsGuiScheme.tsxsrc/watchOptions.ts
✅ Files skipped from review due to trivial changes (1)
- src/optionsGuiScheme.tsx
renderer/viewer/lib/mesher/models.ts
Outdated
| const { smoothLighting, shadingTheme } = world.config | ||
| const faceLight = world.getLight(neighborPos, undefined, undefined, block.name) | ||
| const sideShading = (shadingTheme === 'high-contrast') ? | ||
| (0.8 + 0.5*Math.max(0.0,0.66*dir[0]+0.66*dir[1]+0.33*dir[2])) : //old directional light behavior | ||
| (0.75 + 0.25*dir[1] + 0.05*(Math.abs(dir[2])-3*Math.abs(dir[0]))); | ||
| const baseLight = sideShading * faceLight / 15 |
There was a problem hiding this comment.
Fix formatting and style issues per ESLint rules.
Multiple ESLint violations: indentation (should be 6 spaces), operator spacing, zero fraction (0.0 → 0), and extra semicolon.
🔧 Proposed fix
- const { smoothLighting, shadingTheme } = world.config
- const faceLight = world.getLight(neighborPos, undefined, undefined, block.name)
- const sideShading = (shadingTheme === 'high-contrast') ?
- (0.8 + 0.5*Math.max(0.0,0.66*dir[0]+0.66*dir[1]+0.33*dir[2])) : //old directional light behavior
- (0.75 + 0.25*dir[1] + 0.05*(Math.abs(dir[2])-3*Math.abs(dir[0])));
+ const { smoothLighting, shadingTheme } = world.config
+ const faceLight = world.getLight(neighborPos, undefined, undefined, block.name)
+ const sideShading = (shadingTheme === 'high-contrast')
+ ? (0.8 + 0.5 * Math.max(0, 0.66 * dir[0] + 0.66 * dir[1] + 0.33 * dir[2])) // old directional light behavior
+ : (0.75 + 0.25 * dir[1] + 0.05 * (Math.abs(dir[2]) - 3 * Math.abs(dir[0])))📝 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.
| const { smoothLighting, shadingTheme } = world.config | |
| const faceLight = world.getLight(neighborPos, undefined, undefined, block.name) | |
| const sideShading = (shadingTheme === 'high-contrast') ? | |
| (0.8 + 0.5*Math.max(0.0,0.66*dir[0]+0.66*dir[1]+0.33*dir[2])) : //old directional light behavior | |
| (0.75 + 0.25*dir[1] + 0.05*(Math.abs(dir[2])-3*Math.abs(dir[0]))); | |
| const baseLight = sideShading * faceLight / 15 | |
| const { smoothLighting, shadingTheme } = world.config | |
| const faceLight = world.getLight(neighborPos, undefined, undefined, block.name) | |
| const sideShading = (shadingTheme === 'high-contrast') | |
| ? (0.8 + 0.5 * Math.max(0, 0.66 * dir[0] + 0.66 * dir[1] + 0.33 * dir[2])) // old directional light behavior | |
| : (0.75 + 0.25 * dir[1] + 0.05 * (Math.abs(dir[2]) - 3 * Math.abs(dir[0]))) | |
| const baseLight = sideShading * faceLight / 15 |
🧰 Tools
🪛 ESLint
[error] 393-393: Expected indentation of 6 spaces but found 4.
(@stylistic/indent)
[error] 393-393: Operator '*' must be spaced.
(@stylistic/space-infix-ops)
[error] 393-393: Don't use a zero fraction in the number.
(unicorn/no-zero-fractions)
[error] 393-393: A space is required after ','.
(@stylistic/comma-spacing)
[error] 393-393: Operator '*' must be spaced.
(@stylistic/space-infix-ops)
[error] 393-393: Operator '+' must be spaced.
(@stylistic/space-infix-ops)
[error] 393-393: Operator '*' must be spaced.
(@stylistic/space-infix-ops)
[error] 393-393: Operator '+' must be spaced.
(@stylistic/space-infix-ops)
[error] 393-393: Operator '*' must be spaced.
(@stylistic/space-infix-ops)
[error] 394-394: Expected indentation of 6 spaces but found 4.
(@stylistic/indent)
[error] 394-394: Operator '*' must be spaced.
(@stylistic/space-infix-ops)
[error] 394-394: Operator '*' must be spaced.
(@stylistic/space-infix-ops)
[error] 394-394: Operator '-' must be spaced.
(@stylistic/space-infix-ops)
[error] 394-394: Operator '*' must be spaced.
(@stylistic/space-infix-ops)
[error] 394-394: Extra semicolon.
(@stylistic/semi)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@renderer/viewer/lib/mesher/models.ts` around lines 390 - 395, The block
defining sideShading and baseLight has ESLint style issues: fix indentation to 6
spaces, ensure proper operator spacing around + and *, replace the zero fraction
0.0 with 0, and remove any stray semicolons; specifically update the expressions
using shadingTheme, dir, and faceLight in the sideShading and baseLight
calculations (refer to variables/shims: smoothLighting, shadingTheme,
world.getLight(neighborPos, undefined, undefined, block.name), faceLight,
sideShading, baseLight, dir) so the conditional ternary branches follow the
project's spacing/indentation rules and the numeric literal uses 0 instead of
0.0.
| let ao = (side1Block && side2Block) ? 0 : (3 - (side1Block + side2Block + cornerBlock)) | ||
| const ao_bias = (shadingTheme === 'high-contrast') ? 0.25 : 0.4; | ||
| const ao_scale = (shadingTheme === 'high-contrast') ? 0.25 : 0.2; | ||
|
|
||
| // todo light should go upper on lower blocks | ||
| light = (ao + 1) / 4 * (cornerLightResult / 15) | ||
| light = sideShading * (ao * ao_scale + ao_bias) * (cornerLightResult / 15) |
There was a problem hiding this comment.
Fix style issues: use const and remove extra semicolons.
ESLint flags ao as never reassigned (should be const) and extra semicolons on lines 463-464.
🔧 Proposed fix
- let ao = (side1Block && side2Block) ? 0 : (3 - (side1Block + side2Block + cornerBlock))
- const ao_bias = (shadingTheme === 'high-contrast') ? 0.25 : 0.4;
- const ao_scale = (shadingTheme === 'high-contrast') ? 0.25 : 0.2;
-
+ const ao = (side1Block && side2Block) ? 0 : (3 - (side1Block + side2Block + cornerBlock))
+ const ao_bias = (shadingTheme === 'high-contrast') ? 0.25 : 0.4
+ const ao_scale = (shadingTheme === 'high-contrast') ? 0.25 : 0.2📝 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.
| let ao = (side1Block && side2Block) ? 0 : (3 - (side1Block + side2Block + cornerBlock)) | |
| const ao_bias = (shadingTheme === 'high-contrast') ? 0.25 : 0.4; | |
| const ao_scale = (shadingTheme === 'high-contrast') ? 0.25 : 0.2; | |
| // todo light should go upper on lower blocks | |
| light = (ao + 1) / 4 * (cornerLightResult / 15) | |
| light = sideShading * (ao * ao_scale + ao_bias) * (cornerLightResult / 15) | |
| const ao = (side1Block && side2Block) ? 0 : (3 - (side1Block + side2Block + cornerBlock)) | |
| const ao_bias = (shadingTheme === 'high-contrast') ? 0.25 : 0.4 | |
| const ao_scale = (shadingTheme === 'high-contrast') ? 0.25 : 0.2 | |
| // todo light should go upper on lower blocks | |
| light = sideShading * (ao * ao_scale + ao_bias) * (cornerLightResult / 15) |
🧰 Tools
🪛 ESLint
[error] 462-462: 'ao' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 463-463: Extra semicolon.
(@stylistic/semi)
[error] 464-464: Extra semicolon.
(@stylistic/semi)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@renderer/viewer/lib/mesher/models.ts` around lines 462 - 467, The variable ao
is never reassigned and the code has stray semicolons; change the declaration of
ao from let to const (so const ao = (side1Block && side2Block) ? 0 : (3 -
(side1Block + side2Block + cornerBlock))) and remove any extraneous semicolons
around the ao_bias and ao_scale declarations and the subsequent light
calculation; keep usage of ao_bias, ao_scale and light intact (they remain
const/let as currently intended) so only the ao declaration and stray semicolons
are fixed.
| shadingTheme: { | ||
| possibleValues: [ | ||
| ['high-contrast','High Contrast'], | ||
| ['vanilla','Vanilla'] | ||
| ] | ||
| }, |
There was a problem hiding this comment.
Fix comma spacing per ESLint rules.
Missing spaces after commas in the possibleValues array.
🔧 Proposed fix
shadingTheme: {
possibleValues: [
- ['high-contrast','High Contrast'],
- ['vanilla','Vanilla']
+ ['high-contrast', 'High Contrast'],
+ ['vanilla', 'Vanilla']
]
},📝 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.
| shadingTheme: { | |
| possibleValues: [ | |
| ['high-contrast','High Contrast'], | |
| ['vanilla','Vanilla'] | |
| ] | |
| }, | |
| shadingTheme: { | |
| possibleValues: [ | |
| ['high-contrast', 'High Contrast'], | |
| ['vanilla', 'Vanilla'] | |
| ] | |
| }, |
🧰 Tools
🪛 ESLint
[error] 257-257: A space is required after ','.
(@stylistic/comma-spacing)
[error] 258-258: A space is required after ','.
(@stylistic/comma-spacing)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/defaultOptions.ts` around lines 255 - 260, Fix ESLint comma-spacing in
the shadingTheme configuration by adding a space after each comma inside the
possibleValues entries; update the possibleValues array used by shadingTheme
(the two inner arrays currently written as ['high-contrast','High Contrast'] and
['vanilla','Vanilla']) so they read with a space after the comma (e.g.,
['high-contrast', 'High Contrast'] and ['vanilla', 'Vanilla']) to satisfy the
comma-spacing rule.
|
Wow! It seems to be perfect! |
|
/deploy |
There was a problem hiding this comment.
🧹 Nitpick comments (5)
renderer/viewer/three/worldrendererThree.ts (1)
275-277: Fix extra semicolon and consider chunk invalidation on dimension change.
ESLint flags the extra semicolon on line 276.
When
cardinalLightchanges (e.g., player enters Nether), the new shading only applies to newly meshed chunks. Already-rendered chunks retain stale shading until they're naturally reloaded. Consider triggeringrerenderAllChunks()here if dimension transitions should immediately update all visible chunks.🔧 Proposed fix for semicolon + optional chunk invalidation
changeCardinalLight (cardinalLight: string): void { - this.worldRendererConfig.cardinalLight = cardinalLight; + this.worldRendererConfig.cardinalLight = cardinalLight + // Optional: invalidate existing chunks so shading updates immediately + // this.rerenderAllChunks() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@renderer/viewer/three/worldrendererThree.ts` around lines 275 - 277, Remove the stray semicolon in changeCardinalLight and update it to set this.worldRendererConfig.cardinalLight = cardinalLight (no trailing semicolon), and if you want immediate shading update on dimension change call the renderer invalidation method (rerenderAllChunks) from within changeCardinalLight after assigning the new value so already-meshed chunks are re-rendered; locate changeCardinalLight and the property this.worldRendererConfig.cardinalLight and add the optional call to rerenderAllChunks() (or equivalent chunk invalidation method) to force an immediate update.src/mineflayer/playerState.ts (1)
56-63: Fix extra semicolons per project style.ESLint flags extra semicolons on lines 58, 61, and 63.
🔧 Proposed fix
let cardinalLight = 'default' try { - cardinalLight = data.dimension.value.effects.value === 'minecraft:the_nether' ? 'nether' : 'default'; + cardinalLight = data.dimension.value.effects.value === 'minecraft:the_nether' ? 'nether' : 'default' } catch {} try { - cardinalLight = data.dimension.value.cardinal_light.value; //servers after 1.21.11, untested + cardinalLight = data.dimension.value.cardinal_light.value // servers after 1.21.11, untested } catch {} - this.reactive.cardinalLight = cardinalLight; + this.reactive.cardinalLight = cardinalLight🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mineflayer/playerState.ts` around lines 56 - 63, Remove the stray semicolons flagged by ESLint in the cardinalLight assignment block: eliminate the extra semicolons after the ternary assignment to cardinalLight (where data.dimension.value.effects.value is checked) and after the subsequent try-assignment from data.dimension.value.cardinal_light.value, and ensure the final this.reactive.cardinalLight = cardinalLight line has no trailing semicolon; locate these changes around the cardinalLight variable usage in playerState.ts (references: cardinalLight, data.dimension.value.effects.value, data.dimension.value.cardinal_light.value, this.reactive.cardinalLight).renderer/viewer/lib/mesher/models.ts (3)
390-399: Fix multiple style issues: operator spacing, zero fraction, indentation, and semicolon.ESLint flags numerous violations in the
sideShadingcalculation. The logic is correct, but the formatting needs cleanup.🔧 Proposed fix for style compliance
const { smoothLighting, shadingTheme, cardinalLight } = world.config const faceLight = world.getLight(neighborPos, undefined, undefined, block.name) - const sideShading = (shadingTheme === 'high-contrast') ? - (0.8 + 0.5*Math.max(0.0,0.66*dir[0]+0.66*dir[1]+0.33*dir[2])) : //old directional light behavior - ( - cardinalLight === 'nether' ? - (0.5 + Math.abs(0.1*dir[0]+0.4*dir[1]+0.3*dir[2])) : - (0.75 + 0.25*dir[1] + 0.05*(Math.abs(dir[2])-3*Math.abs(dir[0]))) - ); + const sideShading = (shadingTheme === 'high-contrast') + ? (0.8 + 0.5 * Math.max(0, 0.66 * dir[0] + 0.66 * dir[1] + 0.33 * dir[2])) // old directional light behavior + : ( + cardinalLight === 'nether' + ? (0.5 + Math.abs(0.1 * dir[0] + 0.4 * dir[1] + 0.3 * dir[2])) + : (0.75 + 0.25 * dir[1] + 0.05 * (Math.abs(dir[2]) - 3 * Math.abs(dir[0]))) + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@renderer/viewer/lib/mesher/models.ts` around lines 390 - 399, Reformat the sideShading/baseLight block to satisfy ESLint: normalize operator spacing (spaces around + - * / and after commas), replace the literal 0.0 with 0, fix the indentation of the ternary branches so they align with the const declaration, and ensure statements end with semicolons; specifically update the code around variables shadingTheme, cardinalLight, dir, and faceLight (calculation assigned to sideShading) and add a trailing semicolon to the const baseLight = ... declaration so the expressions and spacing are consistent with project style.
466-471: Fix style issues: useconstforaoand remove extra semicolons.ESLint flags that
aois never reassigned (should beconst) and there are extra semicolons on lines 467-468.🔧 Proposed fix
- let ao = (side1Block && side2Block) ? 0 : (3 - (side1Block + side2Block + cornerBlock)) - const ao_bias = (shadingTheme === 'high-contrast') ? 0.25 : 0.4; - const ao_scale = (shadingTheme === 'high-contrast') ? 0.25 : 0.2; - + const ao = (side1Block && side2Block) ? 0 : (3 - (side1Block + side2Block + cornerBlock)) + const ao_bias = (shadingTheme === 'high-contrast') ? 0.25 : 0.4 + const ao_scale = (shadingTheme === 'high-contrast') ? 0.25 : 0.2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@renderer/viewer/lib/mesher/models.ts` around lines 466 - 471, Change the declaration of ao from let to const since ao is never reassigned and remove any stray extra semicolons on the nearby lines; specifically update the block that computes ao and uses shadingTheme/ao_bias/ao_scale (variables: ao, ao_bias, ao_scale, shadingTheme) and ensure the subsequent light calculation (uses sideShading and cornerLightResult) has no trailing semicolons or extra punctuation.
426-426: Minor: Remove trailing whitespace on blank line.Line 426 appears to have trailing whitespace after the removal of code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@renderer/viewer/lib/mesher/models.ts` at line 426, Remove the trailing whitespace on the blank line in the mesher models module (models.ts) by deleting the extra spaces on that empty line (or run the project's trailing-whitespace fixer/formatter) so the file has a clean blank line with no trailing characters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@renderer/viewer/lib/mesher/models.ts`:
- Around line 390-399: Reformat the sideShading/baseLight block to satisfy
ESLint: normalize operator spacing (spaces around + - * / and after commas),
replace the literal 0.0 with 0, fix the indentation of the ternary branches so
they align with the const declaration, and ensure statements end with
semicolons; specifically update the code around variables shadingTheme,
cardinalLight, dir, and faceLight (calculation assigned to sideShading) and add
a trailing semicolon to the const baseLight = ... declaration so the expressions
and spacing are consistent with project style.
- Around line 466-471: Change the declaration of ao from let to const since ao
is never reassigned and remove any stray extra semicolons on the nearby lines;
specifically update the block that computes ao and uses
shadingTheme/ao_bias/ao_scale (variables: ao, ao_bias, ao_scale, shadingTheme)
and ensure the subsequent light calculation (uses sideShading and
cornerLightResult) has no trailing semicolons or extra punctuation.
- Line 426: Remove the trailing whitespace on the blank line in the mesher
models module (models.ts) by deleting the extra spaces on that empty line (or
run the project's trailing-whitespace fixer/formatter) so the file has a clean
blank line with no trailing characters.
In `@renderer/viewer/three/worldrendererThree.ts`:
- Around line 275-277: Remove the stray semicolon in changeCardinalLight and
update it to set this.worldRendererConfig.cardinalLight = cardinalLight (no
trailing semicolon), and if you want immediate shading update on dimension
change call the renderer invalidation method (rerenderAllChunks) from within
changeCardinalLight after assigning the new value so already-meshed chunks are
re-rendered; locate changeCardinalLight and the property
this.worldRendererConfig.cardinalLight and add the optional call to
rerenderAllChunks() (or equivalent chunk invalidation method) to force an
immediate update.
In `@src/mineflayer/playerState.ts`:
- Around line 56-63: Remove the stray semicolons flagged by ESLint in the
cardinalLight assignment block: eliminate the extra semicolons after the ternary
assignment to cardinalLight (where data.dimension.value.effects.value is
checked) and after the subsequent try-assignment from
data.dimension.value.cardinal_light.value, and ensure the final
this.reactive.cardinalLight = cardinalLight line has no trailing semicolon;
locate these changes around the cardinalLight variable usage in playerState.ts
(references: cardinalLight, data.dimension.value.effects.value,
data.dimension.value.cardinal_light.value, this.reactive.cardinalLight).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e4b986a-a401-493a-a6a1-da19a1d74eaa
📒 Files selected for processing (6)
renderer/viewer/lib/basePlayerState.tsrenderer/viewer/lib/mesher/models.tsrenderer/viewer/lib/mesher/shared.tsrenderer/viewer/lib/worldrendererCommon.tsrenderer/viewer/three/worldrendererThree.tssrc/mineflayer/playerState.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- renderer/viewer/lib/mesher/shared.ts
- renderer/viewer/lib/worldrendererCommon.ts
2 participants
My first ever git pull request, hope I did everything correctly.
Summary by CodeRabbit
Graphics & Rendering
New Features