Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose hard-coded "mode" options more consistently #169

Open
towerofnix opened this issue Mar 8, 2023 · 7 comments
Open

Expose hard-coded "mode" options more consistently #169

towerofnix opened this issue Mar 8, 2023 · 7 comments
Assignees
Labels
discussion Looking for feedback and input

Comments

@towerofnix
Copy link
Member

We've defined a few unions where hard-coded string values are used as mode options:

export type SpeechBubble = "say" | "think";
public touching(target: "mouse" | "edge" | Sprite | Stage ...)
type WatcherStyle = "normal" | "large" | "slider";

However in Trigger.ts, we use static key-symbol properties:

public static readonly GREEN_FLAG = Symbol("GREEN_FLAG");
public static readonly KEY_PRESSED = Symbol("KEY_PRESSED");
public static readonly BROADCAST = Symbol("BROADCAST");
...

We should consistently go one way or the other. I figure the latter, with key/symbols, is the better way, since even though both are equally arbitrary things to remember, property names tend to stick better; symbols also force you to access by property name vs. an enum where you can "cheat" and just use the enum value. (If my understanding of enums is right!)

Regardless what we decide exactly, since this is a 100% public-facing change, we have to think about how it will look and feel in Leopard projects generated from sb-edit and working on your own Leopard project (from scratch or, ahem, from Scratch).

@PullJosh
Copy link
Collaborator

PullJosh commented Mar 9, 2023

I agree that we should pick one and be consistent.

I'll take the controversial stance again and say that I actually prefer opting for using simpler/fewer JS constructs, since we're mostly intending for the library to be used by JavaScript beginners, and using strings works just fine/requires learning one less thing (makes it feel more approachable).

Although, when it comes to triggers, maybe we just prefer something like

this.triggers = [
  Trigger.greenFlag(this.whenGreenFlagClicked),
  Trigger.broadcast("message1", this.whenIReceiveMessage1),
  Trigger.keyPressed("space", this.whenKeySpacePressed),
  Trigger.loudnessGreaterThan(10, this.whenGreaterThan)
];

I'm not sure why we (I?) even made it so complicated in the first place.

@towerofnix
Copy link
Member Author

Nice idea with the Trigger.greenFlag type syntax! It makes sense, since triggers represent a kind of block of their own in Scratch, and it's a clean, consistent syntax to use method names like that.

I also agree with that controversial stance. It's the general perspective I followed with my recent toLeopard rework - where semi-obscure JavaScript syntax could've been effective to ensure compatibility with Scratch (or come close), I still preferred implementing helper functions in Leopard - beginner-friendliness is a major principle for Leopard!

@adroitwhiz, I haven't been 100% certain about this, re: adding type annotations to Leopard: right now generated Leopard projects are still pure JavaScript code, referencing a tsc-compiled copy of Leopard. Was it part of your goal to (eventually) introduce editor integration for Leopard's type annotations into those generated projects? Or are the type annotations only for internal consistency, a helper so we don't write outright incorrect library code ourselves?

It's not hugely pertinent here — there are cleaner user-facing alternatives to enums for everything above — but I'm curious about your thoughts on it. Leopard doesn't have a lot of syntax to memorize - not even much JavaScript, really - so I figure the benefits are mostly our sake's... but I don't think I've asked you your perspective on TypeScript in Leopard and static error catching for Leopard project developers!

@towerofnix
Copy link
Member Author

NB: We don't actually need to stress about SpeechBubbleStyle ("say", "think") - it's internal only. It's exposed via say and think methods, etc.

If we set up triggers to expose through methods too, the only exceptional case is for touching: this.touching("mouse"), this.toucing("edge"). I think it would be OK to break this behavior up into three functions:

this.touchingMouse()
this.touchingEdge()
this.touchingSprite(this.sprites["Sprite2"]) // or whatever

(Double NB: I need to verify but Scratch behavior for "touching [sprite name]" is equivalent to this.touchingSprite(this.sprites[spriteName].andClones). I don't think we're reflecting that in Leopard or toLeopard yet. That's a separate issue, of course!)

@adroitwhiz
Copy link
Collaborator

Was it part of your goal to (eventually) introduce editor integration for Leopard's type annotations into those generated projects? Or are the type annotations only for internal consistency, a helper so we don't write outright incorrect library code ourselves?

I'm not sure how difficult it would be to generate type-safe code, but I definitely think it's worth looking into. Scratch's type system is actually simple and well-defined--all block parameters are allowed to be a string | number | boolean, and most blocks only return a value of a single type (e.g. math blocks always return a number, the "answer" block always returns a string). The only exception I can think of is variable/list blocks, which return a string | number | boolean as well. It should hopefully not be too difficult to ensure type-safety in the generated Leopard code.

If we set up triggers to expose through methods too, the only exceptional case is for touching

I'm 100% in favor of separating those into three different functions. Each case does a completely different thing-- "touching mouse" means "is there an opaque pixel of this sprite over the cursor", "touching sprite" means "do any opaque pixels of this sprite overlap with any opaque pixels of another sprite", and "touching edge" means "does this sprite's bounding box extend offscreen".

@towerofnix
Copy link
Member Author

towerofnix commented Mar 9, 2023

Yeah, I think type-safeness is totally doable in generated code — and has real benefits. My only concern is if tsc-generated errors will be effectively legible to users, or if they'll end up scaring off people new to text-based coding. We might not be able to do much about that without building a completely custom Leopard IDE, which, while potentially awesome, is totally out of scope for now!

If we want to expose TypeScript annotations in CodeSandbox, we'll probably have to work out generating a compatible webpack project. Even if we decide we only want to expose users to pure JavaScript on codesandbox.com (i.e. through leopardjs.com), it'd probably still be a good idea to implement a mode for export which includes a TypeScript and webpack setup - people developing a project with Leopard and TypeScript offline should still have a convenient way to package their project as standalone (and that's not really a skill that is relevant to what Leopard tries to teach).

We don't have a proper offline export built-in yet at all though, so that's still a ways off. IMO it's worth investigating the UI for that before exporting TypeScript from the website.

@towerofnix towerofnix changed the title Use public enums or static symbols in remaining places where strings are hard-coded Expose hard-coded "mode" options more consistently Mar 10, 2023
@towerofnix
Copy link
Member Author

Another case of existing enums which should probably be changed:

Sprite.RotationStyle = Object.freeze({
  ALL_AROUND: Symbol("ALL_AROUND"),
  LEFT_RIGHT: Symbol("LEFT_RIGHT"),
  DONT_ROTATE: Symbol("DONT_ROTATE")
});

More cases which are currently lowercase strings: audio effect names, visual effect names.

The touching function also accepts Color inputs, so should be expanded to touchingColor as well. (Something to consider is colorTouching which is for "does color X on my costume touch color Y on some other sprite's costume / pen trails?" It's named similarly and we may want to rename it — colorTouchingColor would be a simple change that delineates it from other "touching" blocks more clearly.)

Those are the only new special cases I noted going over toLeopard.ts - I checked pretty carefully but I might have missed some.

@towerofnix
Copy link
Member Author

I'm going to go ahead with the changes discussed here so far, we can discuss individual naming decisions and implementation details in a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Looking for feedback and input
Projects
None yet
Development

No branches or pull requests

3 participants