TurboWarp / types

Type definitions for the Scratch VM and editor
https://turbowarp.github.io/types/
Apache License 2.0
8 stars 15 forks source link

Suggestion: Using strings instead of an enum for layer group in scratch-renderer #17

Closed Tacodiva closed 1 year ago

Tacodiva commented 1 year ago

In my opinion, it's kind of strange that these typing as they are right now only allow you to use the layer groups that scratch uses. As the comment in the enum says:

  const enum LayerGroup {
    // The renderer can be configured to use any strings as group names, but these are the groups
    // that scratch-vm uses, listed in order.
    Background = 'background',
    Video = 'video',
    Pen = 'pen',
    Sprite = 'sprite'
  }

What you name your layer groups is really a decision made by consumers of the scratch-renderer library and shouldn't be arbitrarily enforced by these typings. Because of this odd restriction, things like the playground example which come directly from scratch-render repo will not compile without casting or changing the layer group name to one of the four prescribed by the enum above.

I think this should be changed, but I though I'd create an issue because it seems to be a very deliberate decision and I was curious to hear your thoughts.

GarboMuffin commented 1 year ago

I think it would make sense to change it to'background' | 'video' | 'pen' | 'sprite' | string

Tacodiva commented 1 year ago

That sounds like a good compromise. Should I add it to the PR?

GarboMuffin commented 1 year ago

Sure