Re-implement cat block rendering in Blockly v12 terms

[cwillisf]

Proposed Changes

• Merge latest changes from develop into spork
• Re-implement cat block rendering in Blockly v12 terms

Reason for Changes

• ^•⩊•^

@gonfunko, I'm very interested in any refactorings you think might make future Blockly upgrades smoother, or if there's a better or more official mechanism for any of this. I'm not thrilled with CatBlockSvg, for example.

[Copilot]

Pull request overview

This PR re-implements the cat block rendering feature to work with Blockly v12 APIs, refactoring the renderer architecture to support multiple rendering themes. The changes merge the latest from develop and introduce a theme-based system for selecting between classic Scratch blocks and animated cat blocks.

Key Changes

• Introduced a theme-based renderer selection system with ScratchBlocksTheme enum supporting CLASSIC and CAT_BLOCKS themes
• Refactored renderer architecture to use overridable makeReplacementTop_() and makeBowlerHat() methods for better extensibility
• Implemented cat block renderer with interactive animations including blinking eyes and flickable ears

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/renderer/renderer.ts	Added getClassName() method and renamed renderer registration from "scratch" to "scratch_classic"
src/renderer/render_info.ts	Extracted bowler hat creation into overridable makeBowlerHat() method for subclass customization
src/renderer/drawer.ts	Refactored hat path replacement logic into makeReplacementTop_() method to support subclass overrides
src/renderer/constants.ts	Added BOWLER_HAT_HEIGHT constant and makeBowlerHatPath() method; fixed unnecessary semicolon
src/renderer/bowler_hat.ts	Updated to use BOWLER_HAT_HEIGHT from constants instead of hardcoded value
src/renderer/cat/renderer.ts	New cat renderer that extends ScratchRenderer and registers as "scratch_catblocks"
src/renderer/cat/drawer.ts	New drawer with face rendering and interactive animations for cat blocks
src/renderer/cat/constants.ts	Cat-specific constants including face geometry paths and dynamic cat path generation
src/renderer/cat/cat_block_svg.ts	Interface extending BlockSvg with hasFace flag to track face creation state
src/index.ts	Added theme-based renderer selection with ScratchBlocksOptions interface
src/constants.ts	Added ScratchBlocksTheme enum defining available block themes

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gonfunko]

I'm not positive, but I think it might work a bit better to create a PathObject subclass and move most of what's in Drawer into there. Blocks have one PathObject for their entire lifecycle, vs the drawer being invoked every time the block is rendered, so I think you could just add the face SVG group to the block's root SVG group in the PathObject constructor unconditionally, which would remove the need for the state tracking in CatBlockSvg.

[cwillisf]

I'm not positive, but I think it might work a bit better to create a PathObject subclass and move most of what's in Drawer into there.

I'm just now getting back to this work, and tried to move the face into a new PathObject subclass today. Unfortunately, I don't see a way to know at PathObject construction time whether or not a block is going to be a hat block. I only have the SVG root, the BlocklyStyle (where hat is always an empty string...?), and the ConstantsProvider. Unless there's something I'm missing, it seems like I'd need to wait until the second applyColour() call to add the face (it looks like applyColour() is typically called twice, and only knows about the hat the second time). Any suggestions?

[gonfunko]

Well shoot, right you are...I wonder if PathObject.setStyle() might work? It appears to be the source of one of the applyColour() calls, hopefully the latter.

[cwillisf]

I got bogged down in tracing who calls what, and decided to zoom out. Here's what I'm thinking:

1. The ConstantsProvider knows/generates the path fragments for all the atomic parts of all the blocks
2. The BlockSvg owns its root SVGElement
3. The PathObject owns the SVGElement for the block's main path
4. The Drawer (at least for Zelos) is responsible for inspecting block state (inputs, outputs, etc.) and then calling PathObject.setPath() with the fully assembled path.

One thing I'll admit up front: I don't have a good handle on what the outlines and remainingOutlines members of the Zelos PathObject are for. There could be something key that I'm missing there. But, based on what I currently know...

Given item 3, it seems appropriate for each face to be owned by its associated PathObject, but given item 4, it seems appropriate for the Drawer to be in control of whether or not a particular PathObject has a face. The PathObject can know whether or not it already has a face, so maybe there should be something like PathObject.ensureFace().

By "face" here, I mean the eyes and mouth, which are "extra" items that don't affect the block's outline. The eyes react to mouseenter on the svgPath owned by the PathObject, so it makes sense for the PathObject (or its delegate) to set up those events.

The ears are tricky: their insides are like face parts (strong, independent SVGElements who don't need no Drawer), but their outsides are part of the overall block outline. That is, the inside can be updated (shown/hidden) entirely inside of the PathObject, but the outside can only be updated (lowered & raised) by redrawing the whole path, which means involving the Drawer. (Well, we could update it by replacing a path substring by force, but I'd rather rebuild the path in the official Drawer way.)

Based on all of this, I'm leaning toward moving as much of the face logic as possible into a new, separate object. That object can be created, owned, and partially hooked up by the PathObject, and the Drawer can base its outline decisions on the ear-state owned by the face object.

The one thing that isn't covered is the method I called Drawer.redraw() in the first version of this PR. I get the feeling that y'all were trying to avoid having the PathObject retain a reference to the Drawer, but if the face object is owned by the PathObject and it can call Drawer.redraw() (or Drawer.draw()) then there must be a direct or indirect reference there somewhere. The only way I see to avoid that is to have the Drawer own the face object, which would lead to a structure more or less like the current one, just with a bit more encapsulation.

Thoughts?

[cwillisf]

Update: I'm now thinking that the Drawer should own the FaceDrawer, but the PathObject can hold the drawn face parts. That seems a) more like the way Zelos is set up, and b) like it will fix the draw() / reference issue I mentioned in my previous comment.

[gonfunko]

That broadly sounds reasonable; this is an area of the code I tend to avoid, but I ran it by Rachel and she suggested looking at how Drawer.layoutField_() handles fields/icons as sort of the nearest parallel to the facial features. In general rendering was built around the assumption that blocks are composed of rows of measurable elements, and not really with an affordance for arbitrary row-crossing graphical elements. Probably the most "correct" thing would be to make the eyes, face, inner ears, etc Measurables and update the layout code to take them into account and such, but I think that's pretty overkill and potentially infeasible. What you're suggesting with the Drawer/PathObject split seems like it aligns to their respective purposes reasonably well.

[cwillisf]

I made this for myself to keep track of who holds a reference to whom:

Also: Today I Learned that both Drawer and RenderInfo are ephemeral. I thought it was just Drawer. (I should really spend more time with the Blockly docs... :/)

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 10 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.