Skip to content

feat(compiler): add automated vector master linter gate - #234

Merged
BharathASL merged 4 commits into
mainfrom
feat/compiler-vector-linter
Sep 24, 2026
Merged

BharathASL merged 4 commits into
mainfrom
feat/compiler-vector-linter

Conversation

@BharathASL

@BharathASL BharathASL commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Description

Adds a static vector master linter, tools/lib/vector-linter.mjs, and wires it into npm test and CI as a blocking gate. npm run lint:svg walks every SVG under textures/ and fails if any master:

  • has a root viewBox other than exactly 0 0 512 512;
  • declares an Inkscape, Sodipodi or Adobe editor namespace, detected by namespace URI under any prefix (including values declared through <!ENTITY>), or carries inkscape:/sodipodi: editor elements;
  • embeds a raster, either through <image> or through a <feImage> whose href (unprefixed or under any prefix bound to the xlink namespace) is not a same-document #id;
  • has a <clipPath> without an id, a url() or href reference that is not a same-document #id or resolves to nothing, or a reference whose target is the wrong element type (fill/stroke → paint server, clip-path → <clipPath>, mask → <mask>, filter → <filter>, marker-start/mid/end → <marker>);
  • uses any reference form resvg does not honour: quoted url('#id')/url("#id") (including &quot;/&apos; entity forms), any case variant of url(, a space before (, upper-case property names; the marker shorthand is also rejected (resvg ignores it outside <style> rules).

@resvg/resvg-js does not fail on a dangling or unsupported reference, so a broken clip, mask, gradient or <use> reference changes the rendered texture without any build error. The linter tokenizes each master, tracks namespace scope, decodes entities and character references, and collects every attribute, style declaration and <style> rule, so only the forms resvg 2.6 actually renders pass. Each rejected form was confirmed against @resvg/resvg-js 2.6.2.

tools/test/vector-linter.test.mjs (npm run test:vector-linter) covers each rule with passing and failing fixtures, including feImage file/xlink/data-URI/fragment cases, data-id, case-variant (viewbox, data-viewBox, ID, <clippath>), dangling fragment href and quoted url() cases. Its first suite lints every master under textures/ and derives the expected count from findSvgFiles(DEFAULT_TEXTURES_DIR), so a PR that adds a texture does not have to touch the test. CONTRIBUTING.md documents the enforced rules under Canvas & Grid Alignment and adds npm run lint:svg to the Pull Request Checklist, and CHANGELOG.md has an [Unreleased] → Added entry.

No tracking issue exists for this gate. The closed toolchain epic #40 and its sub-tasks (#54–#59, #226) do not cover static SVG validation, and a search of the org's issues for linter, viewBox, editor namespace, raster and clipPath terms found nothing. No issue is linked.

Affected Assets

  • tools/lib/vector-linter.mjs (new)
  • tools/test/vector-linter.test.mjs (new)
  • package.json (lint:svg, test:vector-linter, npm test aggregator)
  • .github/workflows/ci.yml (Run Vector Master Linter, Vector Master Linter Test Suite)
  • CONTRIBUTING.md, CHANGELOG.md
  • No textures, blockstates or models changed.

Type of Change

  • feat: New vector texture master, blockstate, or compiler capability
  • fix: Tiling fix, palette correction, or bugfix
  • docs: Documentation improvement
  • chore / refactor: Maintenance, dependencies, or codebase cleanup

Contributor Checklist

Vector Texture Standards (if adding/modifying SVGs)

  • Authored as a clean $512\times512$ SVG (viewBox="0 0 512 512"). Not applicable: no SVGs added or modified. The linter now enforces this for every existing master.
  • Primary shapes are aligned to the $32\text{px}$ texel grid ($16\times16$ grid). Not applicable: no SVGs added or modified.
  • Seamless Toroidal Tiling: Checked and verified that elements wrapping across $X$ and $Y$ edges align perfectly without seams or chopped shapes. Not applicable: no SVGs added or modified.
  • Ore Consistency: If adding an ore texture, the stone background geometry and corner radius (rx) match textures/block/stone.svg exactly. Not applicable: no ore added.
  • Free of leftover AI generation comments, unnecessary editor namespaces, or embedded raster images. Not applicable: no SVGs added or modified. npm run lint:svg passes on all existing masters.

Build & Verification

  • Ran npm run build locally and verified that the pack compiles successfully.
  • Verified textures in-game or inspected the rasterized PNG outputs in dist/. Not applicable: tooling-only change, no texture output changes.

Git Hygiene & Standards

  • Commit message(s) follow Conventional Commits (e.g., feat(textures): ...).
  • Every commit is signed off with the Developer Certificate of Origin (git commit -s).
  • Branch is rebased cleanly onto latest main with no merge commits.
  • Formatting complies with .editorconfig (2-space indent, LF endings, trailing newline).

Verification

  • npm run lint:svg: all masters under textures/ pass.
  • npm run test:vector-linter: 193/193.
  • npm test: all suites pass.
  • check-standards.py: no violations.

@ninja6-agent ninja6-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blockers

  • tools/lib/vector-linter.mjs:L1, tools/test/vector-linter.test.mjs:L1: Both files start with a UTF-8 BOM (EF BB BF) before #!/usr/bin/env node. .editorconfig sets charset = utf-8 (no BOM), they are the only BOM-prefixed scripts under tools/, and the shebang is no longer the first bytes, so running the file directly on Linux fails. Both files also lack the trailing newline (tools/lib/vector-linter.mjs:L258, tools/test/vector-linter.test.mjs:L369), which breaks insert_final_newline = true. Re-save both as UTF-8 without BOM, LF line endings, ending in a newline.
  • tools/test/vector-linter.test.mjs:L84: The master count is hardcoded to 30 (again at L88–L89). The next texture PR that merges breaks this suite, npm test, and the Vector Master Linter Test Suite CI step. Open #195 adds textures/block/cobblestone.svg, which would make it 31. Suite 1 also only walks textures/block/, while lint:svg covers all of textures/. Derive the expected set from findSvgFiles(DEFAULT_TEXTURES_DIR), then assert that it is non-empty and that filesChecked equals its length.
  • PR description: The repo's pull_request_template.md is not used. The description has no Affected Assets, Type of Change, or Contributor Checklist, and no linked issue (Closes #N). It also starts with a stray BOM. CONTRIBUTING.md §4.3 requires the checklist. Rewrite the description on the template, fill in the checklist, and link the tracking issue.

Nits

  • tools/lib/vector-linter.mjs:L112: <filter><feImage href="x.png"/></filter> passes the linter. I reproduced this locally, and resvg renders feImage raster sources. Extend the raster check to feImage and add a fixture for it.
  • tools/lib/vector-linter.mjs:L118: \bid\s*= also matches data-id="g", so fill="url(#g)" passes when no element has id="g" (reproduced). Anchor the match on attribute whitespace, for example (?:^|\s)id\s*=.
  • CHANGELOG.md:L15: There is no [Unreleased] → ### Added entry for the linter gate. Prior tooling PRs (#227) added one. Add an entry.
  • CONTRIBUTING.md:L104: The canvas and editor-residue rules are now machine-enforced. Name npm run lint:svg here and add it to the Pull Request Checklist (L250), as the base-sync and tiling sections already do for their checks.
  • Commit 64ed447: The body is 5 lines, with lines up to 199 characters, which breaks N6-COMMIT-05 (CONTRIBUTING.md §3.5: a line or two, wrapped at 72). Trim the squash-merge message to the subject plus sign-off.

Signed-off-by: Bharath <bharathasl74185@gmail.com>
@BharathASL
BharathASL force-pushed the feat/compiler-vector-linter branch from 64ed447 to e99be63 Compare September 23, 2026 19:13

@ninja6-agent ninja6-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blockers

  • tools/lib/vector-linter.mjs:L88, L132, L143, L147: the attribute and element regexes use the i flag, but SVG is XML and case-sensitive, and resvg ignores the case variants that these regexes accept. So the gate passes masters that render wrong without any error, which is the failure it was added to catch. Checked against @resvg/resvg-js: a <linearGradient ID="g"> counts as a definition for fill="url(#g)", but resvg does not resolve it, so the rect renders fully transparent. A <clippath id="c"> counts as a clipPath, but resvg drops the clip and draws the element unclipped. viewbox="0 0 512 512" and data-viewBox="0 0 512 512" both pass the canvas rule, although resvg ignores both. Fix: drop i from the viewBox, id and <clipPath> patterns. Anchor viewBox on preceding whitespace the way id already is. Add failing fixtures for viewbox=, data-viewBox=, ID= and <clippath> to tools/test/vector-linter.test.mjs.

Nits

  • tools/lib/vector-linter.mjs:L122-L123, L156: only url(#id) is checked for resolution. A same-document href/xlink:href="#id" on <use>, on a gradient's template link or on the <feImage> fragment this rule explicitly allows is never checked, and resvg drops it silently in exactly the same way (<feImage href="#nope"> and <use href="#missing"> both lint clean). Run these fragment hrefs through the same definedIds check.
  • tools/lib/vector-linter.mjs:L156: url(&quot;#id&quot;), the form that editors emit inside style attributes, does not match the URL pattern, so a dangling reference in that form lints clean. Decode &quot;/&apos; before matching, or accept them as the quote.

Signed-off-by: Bharath <bharathasl74185@gmail.com>
@BharathASL

Copy link
Copy Markdown
Contributor Author

Resolved in 1fc7fb5:

  • Case-sensitivity: dropped i from the root <svg>, viewBox, id and <clipPath> patterns and anchored viewBox on preceding whitespace. viewbox=, data-viewBox=, ID= and <clippath> now fail; fixtures added. Each case confirmed against @resvg/resvg-js 2.6.2 (unresolved gradient renders transparent, lowercase clip is dropped, both viewBox variants ignored).
  • Fragment hrefs: every href/xlink:href="#id" (<use>, gradient template links, <feImage>) now goes through the definedIds check; fixtures added for each.
  • url(&quot;#id&quot;): &quot;/&apos; (and numeric forms) are decoded before matching, so dangling entity-quoted references fail. resvg 2.6.2 also does not resolve any quoted url() even when the id exists (fill falls back to black; clip-path, mask and filter are dropped), so quoted forms are now rejected outright. No master uses them.

CONTRIBUTING.md and the CHANGELOG.md entry updated to match. npm run lint:svg passes on all 30 masters; npm test green (vector linter 117/117).

@ninja6-agent ninja6-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blockers

  • tools/lib/vector-linter.mjs:L162: urlRegex still has the i flag, so URL(#id) counts as a resolved reference. resvg 2.6.2 does not recognise the uppercase function. With @resvg/resvg-js, clip-path="URL(#c)" and style="clip-path:URL(#c)" lint clean, but the clip is dropped and the element draws unclipped. fill="URL(#g)", both inline and in a <style> rule, also lints clean and renders black instead of the gradient. This is the same case-variant false pass as the previous round, and CONTRIBUTING.md:L113 now claims it cannot happen. Fix: match url( case-sensitively. Also reject any url( spelled in another case, because a case-insensitive match that accepts URL( is exactly the problem. Drop i from clipPathRefRegex (L186) as well. Add failing fixtures for URL(#id) in an attribute, in style, and in <style>.

Nits

  • tools/lib/vector-linter.mjs:L186-L193: only clip-path is checked against the type of the element it references. fill="url(#r)" where #r is a <rect> lints clean, and resvg renders the whole shape transparent. mask="url(#c)" pointing at a <clipPath>, and filter="url(#g)" pointing at a gradient, also lint clean, and resvg renders the element invisible. Check the target type for fill/stroke, mask, filter and marker-* the same way.
  • tools/lib/vector-linter.mjs:L162, L176: references that are not a same-document fragment are never checked. <use href="other.svg#r"/> and fill="url(other.svg#g)" both lint clean. resvg drops the <use> and paints the fill black. Reject any url() or href on a non-<feImage> element that is not #id.
  • tools/lib/vector-linter.mjs:L126, L176: the href patterns only recognise the literal xlink: prefix. xmlns:x="http://www.w3.org/1999/xlink" with <feImage x:href="data:image/png;base64,..."> lints clean, and resvg renders the embedded PNG, so the raster ban can be bypassed. x:href="#missing" on <use> also escapes the reference check. Accept any prefix on href, or reject any xlink namespace prefix other than xlink.
  • tools/lib/vector-linter.mjs:L26-L30, L105: editor namespaces are detected by prefix, not by URI. xmlns:ns1="http://www.inkscape.org/namespaces/inkscape" passes. Illustrator never writes xmlns:illustrator: it writes xmlns:i="&ns_ai;" with ns_ai = http://ns.adobe.com/AdobeIllustrator/10.0/, so real Illustrator residue passes, even though CONTRIBUTING.md:L113 lists it as enforced. Match the Inkscape, Sodipodi and Adobe namespace URIs (including inside <!ENTITY> declarations) instead of the prefixes.

…nter

Signed-off-by: Bharath <bharathasl74185@gmail.com>
Signed-off-by: Bharath <bharathasl74185@gmail.com>
@BharathASL

Copy link
Copy Markdown
Contributor Author

Resolved in 9709dfe:

  • url() case variants: any spelling of url( is now found; only lowercase, unquoted url(#id) with no space before ( passes. URL(/Url( in attributes, style and <style> fail. Upper-case property names (CLIP-PATH=, FILL:) are rejected too.
  • Reference target types: fill/stroke must reference a paint server, mask a <mask>, filter a <filter>, clip-path a <clipPath>, marker-* a <marker>.
  • External references: any url() or href that is not a same-document #id fails; a non-fragment <feImage> href is a raster error.
  • xlink prefix: href is honoured unprefixed or under any prefix bound to the xlink namespace; namespace scope is tracked, so x:href can no longer bypass the raster ban or the reference check.
  • Editor namespaces: detected by URI (Inkscape, Sodipodi, http://ns.adobe.com/, adobe:ns:meta/) under any prefix, including values declared through <!ENTITY>.

Every case reproduced against @resvg/resvg-js 2.6.2; failing fixtures added in Suite 8 of tools/test/vector-linter.test.mjs (193/193). CONTRIBUTING.md and the changelog entry now describe only what is enforced.

1174805 moves the changelog entry to clear a conflict with main.

@ninja6-agent ninja6-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blockers

None.

Nits

  • tools/lib/vector-linter.mjs:L330-L334: idTargets keeps the first element for a duplicated id, but resvg 2.6.2 resolves url() to the last one. With two <linearGradient id="g">, resvg paints the second gradient. A <clipPath id="c"> followed by <rect id="c"> lints clean, and resvg drops the clipped element. Two <mask id="m"> behave the same way. No master has a duplicate today, but ore derivatives copy the stone base section, so a collision is a realistic mistake. Reject duplicate id values.
  • tools/lib/vector-linter.mjs:L58-L67, L400-L402, CONTRIBUTING.md:L113: resvg 2.6.2 does apply the marker shorthand when it is written in a <style> rule. .m{marker:url(#mk)} draws the marker. It only ignores the shorthand as a presentation attribute or in a style attribute. The linter rejects all three forms, and the doc says resvg ignores the shorthand. Either accept marker for declarations that come from <style>, or describe the rejection as a policy choice, not resvg behaviour.
  • tools/lib/vector-linter.mjs:L177, L403: the root namespace is never checked. For an SVG with no xmlns, every url() target fails with "is not a <linearGradient> ... (found <linearGradient>)". A master with no references lints clean, even though resvg refuses to parse it. Add an explicit check that the root <svg> is in the SVG namespace, so the error names the actual cause.
  • PR description: the rule list still describes the first round: namespace detection by xmlns: prefix, and a target-type check for clip-path only. Verification still reports 117/117, but the suite now has 193 assertions. Update the description to match the code.

@BharathASL
BharathASL added this pull request to the merge queue Sep 24, 2026
Merged via the queue into main with commit fd648d0 Sep 24, 2026
5 checks passed
@BharathASL
BharathASL deleted the feat/compiler-vector-linter branch September 24, 2026 03:00
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