Skip to content

feat(world-builder): multi-layer tile opacity and blend mode validator - #136

Open
Rodrigoue9 wants to merge 6 commits into
Bitcoindefi:mainfrom
Rodrigoue9:feat/world-builder-layer-opacity
Open

feat(world-builder): multi-layer tile opacity and blend mode validator#136
Rodrigoue9 wants to merge 6 commits into
Bitcoindefi:mainfrom
Rodrigoue9:feat/world-builder-layer-opacity

Conversation

@Rodrigoue9

@Rodrigoue9 Rodrigoue9 commented Aug 25, 2026

Copy link
Copy Markdown

Feature Summary

  • Adds multi-layer tile opacity and blend mode validator for World Builder canvas layers.
  • Guarantees normalized opacity bounds [0.0, 1.0] and deterministic blend modes.

Resolves layer blending requirements! 🚀


Summary by Gitar

  • Database Schema Updates:
    • Added clan_members table definition and associated indexes before applying dependent migrations in api/schema.sql
  • NPC & Map Features:
    • Implemented map NPC placement and boundary-aware patrol path generator in api/src/repositories/mapNpcPlacement.ts
    • Added seed npcs.json dataset for characters and vendors in api/src/jsons/npcs.json

This will update automatically on new commits.

Comment on lines +14 to +15
const layerIndex = Math.max(0, Math.floor(config.layerIndex ?? 0));
const opacity = Math.min(1.0, Math.max(0.0, config.opacity ?? 1.0));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Edge Case: NaN opacity/layerIndex bypass clamping

Math.max(0.0, Math.max/floor(NaN)) returns NaN, so a caller passing opacity: NaN (or layerIndex: NaN) produces a config with NaN values, violating the stated [0.0,1.0] normalization guarantee. Add a Number.isFinite fallback, e.g. const opacity = Number.isFinite(config.opacity) ? Math.min(1, Math.max(0, config.opacity!)) : 1.0; (and similarly for layerIndex).

Guard against NaN/Infinity before clamping:

const rawIndex = config.layerIndex;
const layerIndex = Number.isFinite(rawIndex) ? Math.max(0, Math.floor(rawIndex as number)) : 0;
const rawOpacity = config.opacity;
const opacity = Number.isFinite(rawOpacity) ? Math.min(1.0, Math.max(0.0, rawOpacity as number)) : 1.0;
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

Comment thread PR_DESCRIPTION_DRAFT.md
Comment on lines +1 to +10
# feat(world-builder): register uploaded graphics and extend palette schemas (#6)

## Summary
Resolves #6 by providing `paletteEntrySchema` and `validatePaletteEntry` to validate multi-layer palette definitions, enforce non-colliding graphic index allocations (`UPLOADED_GRAPHIC_INDEX_START = 1_000_000`), and verify graphic existence across engine and uploaded assets.

### Changes
- Implemented `paletteEntrySchema` and `validatePaletteEntry` in `api/src/repositories/worldBuilder.ts`.
- Added unit tests in `api/src/repositories/__tests__/paletteValidation.test.ts`.

Closes #6

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Quality: Stray PR_DESCRIPTION_DRAFT.md committed to repo root

PR_DESCRIPTION_DRAFT.md appears to be a working draft accidentally committed, and its content describes a different feature (#6 palette schemas) unrelated to this PR. Remove it from the commit so it doesn't clutter the repository root.

Delete the accidentally committed draft file:

git rm PR_DESCRIPTION_DRAFT.md
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

@gitar-bot

gitar-bot Bot commented Aug 25, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 0 resolved / 2 findings

Adds multi-layer tile opacity and blend mode validator alongside map NPC placement and database migration fixes. Consider addressing the NaN opacity/layerIndex bypass and removing the stray PR draft file.

💡 Edge Case: NaN opacity/layerIndex bypass clamping

📄 api/src/lib/layerBlendMode.ts:14-15

Math.max(0.0, Math.max/floor(NaN)) returns NaN, so a caller passing opacity: NaN (or layerIndex: NaN) produces a config with NaN values, violating the stated [0.0,1.0] normalization guarantee. Add a Number.isFinite fallback, e.g. const opacity = Number.isFinite(config.opacity) ? Math.min(1, Math.max(0, config.opacity!)) : 1.0; (and similarly for layerIndex).

Guard against NaN/Infinity before clamping
const rawIndex = config.layerIndex;
const layerIndex = Number.isFinite(rawIndex) ? Math.max(0, Math.floor(rawIndex as number)) : 0;
const rawOpacity = config.opacity;
const opacity = Number.isFinite(rawOpacity) ? Math.min(1.0, Math.max(0.0, rawOpacity as number)) : 1.0;
💡 Quality: Stray PR_DESCRIPTION_DRAFT.md committed to repo root

📄 PR_DESCRIPTION_DRAFT.md:1-10

PR_DESCRIPTION_DRAFT.md appears to be a working draft accidentally committed, and its content describes a different feature (#6 palette schemas) unrelated to this PR. Remove it from the commit so it doesn't clutter the repository root.

Delete the accidentally committed draft file
git rm PR_DESCRIPTION_DRAFT.md
🤖 Prompt for agents
Code Review: Adds multi-layer tile opacity and blend mode validator alongside map NPC placement and database migration fixes. Consider addressing the NaN opacity/layerIndex bypass and removing the stray PR draft file.

1. 💡 Edge Case: NaN opacity/layerIndex bypass clamping
   Files: api/src/lib/layerBlendMode.ts:14-15

   `Math.max(0.0, Math.max/floor(NaN))` returns `NaN`, so a caller passing `opacity: NaN` (or `layerIndex: NaN`) produces a config with `NaN` values, violating the stated [0.0,1.0] normalization guarantee. Add a `Number.isFinite` fallback, e.g. `const opacity = Number.isFinite(config.opacity) ? Math.min(1, Math.max(0, config.opacity!)) : 1.0;` (and similarly for `layerIndex`).

   Fix (Guard against NaN/Infinity before clamping):
   const rawIndex = config.layerIndex;
   const layerIndex = Number.isFinite(rawIndex) ? Math.max(0, Math.floor(rawIndex as number)) : 0;
   const rawOpacity = config.opacity;
   const opacity = Number.isFinite(rawOpacity) ? Math.min(1.0, Math.max(0.0, rawOpacity as number)) : 1.0;

2. 💡 Quality: Stray PR_DESCRIPTION_DRAFT.md committed to repo root
   Files: PR_DESCRIPTION_DRAFT.md:1-10

   `PR_DESCRIPTION_DRAFT.md` appears to be a working draft accidentally committed, and its content describes a different feature (#6 palette schemas) unrelated to this PR. Remove it from the commit so it doesn't clutter the repository root.

   Fix (Delete the accidentally committed draft file):
   git rm PR_DESCRIPTION_DRAFT.md

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Important

Your trial ends in 7 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant