Skip to content

Builtin extensions #687

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

Merged
merged 6 commits into from
Oct 13, 2017
Merged

Conversation

cwillisf
Copy link
Contributor

@cwillisf cwillisf commented Oct 4, 2017

Proposed Changes

This change converts the pen and WeDo 2.0 blocks into "internal extensions" -- extensions allowed to run in the main thread instead of in a worker -- and provides a mechanism for loading those two extensions by ID.

To load a built-in extension, call extensionManager.loadExtensionURL(extensionId). The valid IDs are 'pen' and 'wedo2'; any other ID will be treated as a URL.

Reason for Changes

This will make it easier to experiment with the extensions system before we have all the technical details settled.

Future Work

The extension system doesn't yet have support for drop-down menus, so although the WeDo 2.0 blocks contain the data necessary to generate menus those fields will appear as text inputs for now. Extension menus will be implemented as a followup to this change. (Tracking in #689)

This change also contains a partial fix for #580, though I now believe a complete fix will require changes in the scratch-blocks repository.

The message dispatch system, which is the foundation of the extension system, returns a promise for any call. Unfortunately the runtime is set up such that a call which returns a promise will always yield. I put in a hack to work around this limitation for internal extensions, but we need to do some work in the runtime if we want worker-sandboxed extensions to perform well. (Tracking in #688)

Christopher Willis-Ford added 3 commits October 4, 2017 12:54
WeDo2 and Pen blocks have been converted to internal extensions, and can
now be loaded by giving `loadExtensionURL` the string 'pen' or 'wedo2'
instead of an actual URL.
This prevents generation of invalid XML due to characters like '<' or
'>' in fields' default values. Unfortunately the value comes back in its
escaped form, so there's still more work to be done.
@cwillisf
Copy link
Contributor Author

cwillisf commented Oct 4, 2017

Oops - integration tests are failing because the opcode for pen blocks has changed. Will fix soon...

@cwillisf cwillisf force-pushed the builtin-extensions branch from d6bd1ee to b6727a7 Compare October 4, 2017 23:07
@paulkaplan
Copy link
Contributor

@cwillisf did you have a plan for how to load extensions that are used when loading the project? For example, using this I can't load projects that use pen blocks (I get a number of errors). I think that this change should include some kind of auto-loader for at least the built-in extensions.

@ericrosenbaum
Copy link
Contributor

@cwillisf Cool! Two small changes to get the Wedo to start working:

  • Remove the scratch3_wedo2 require from runtime
  • Add this.connect() to the Scratch3WeDo2Blocks constructor

@cwillisf
Copy link
Contributor Author

cwillisf commented Oct 6, 2017

@paulkaplan that's definitely something we'll need to handle. In Scratch 2.0 we record the extension ID/URL along with enough information to reconstruct the shape of the blocks if the extension can't be loaded. I think we should do the same in 3.0, but obviously it's not there yet. (Tracking in #692)

You should be able to get pen blocks working if you load a Scratch 2.0 project and also load a pen extension. Unfortunately(?) the Scratch 3.0 pen opcodes changed due to extension-ification, so pen blocks in a Scratch 3.0 JSON saved before this change will no longer work correctly after this change.

- Remove WeDo 2 extension from the runtime's default block packages list
- The WeDo 2.0 extension now calls its own `connect` method on startup

I also renamed `EXTENSION_NAME` to `EXTENSION_ID` for consistency with
the rest of the extension system.
@cwillisf cwillisf force-pushed the builtin-extensions branch from c465fa6 to 7297341 Compare October 6, 2017 16:55
Copy link
Contributor

@rschamp rschamp left a comment

Choose a reason for hiding this comment

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

This looks like what we want to me. Although process-wise, I think it would be good to either include the auto-loading of the pen extension for 2.0 projects in this PR, or to have the PR that accomplishes that ready when we merge this PR. Curious what @thisandagain thinks about that.

@cwillisf
Copy link
Contributor Author

Yeah - that's one interesting point that we haven't really discussed yet. It's one thing to auto-load an extension explicitly mentioned in the extensions list of a project, but for pen -- and any other blocks we decide to convert to extensions -- we may have to inspect the opcodes in the project in order to decide whether we should load the pen extension. That, or we just always load the pen extension when importing a Scratch 2.0 project.

@cwillisf
Copy link
Contributor Author

Not that inspecting the opcodes would be difficult or anything. I think it would make sense to add an optional field in sb2_specmap.js indicating which extension a block lives in, and collect/dedupe/load extensions as part of the SB2 import process.

@cwillisf
Copy link
Contributor Author

I'm going to go ahead and merge this so I can file clean PRs for some of my other extension work. I'm also working on auto-loading the pen extension and expect to have that ready today.

@cwillisf cwillisf merged commit 7051ccf into scratchfoundation:develop Oct 13, 2017
@cwillisf cwillisf deleted the builtin-extensions branch October 13, 2017 16:49
stefania11 pushed a commit to mitmedialab/cognimates-vm that referenced this pull request Mar 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants