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

Update Bee API methods to go directly through executionInfo #20890

Merged
merged 3 commits into from Mar 2, 2018

Conversation

Hamms
Copy link
Contributor

@Hamms Hamms commented Feb 26, 2018

Rather than calling a subtype method which THEN queues an action which
then calls a subtype method, this PR untwitsts things to make the flow
of execution more linear.

Okay, so Subtypes that support Quantum maps need to provide two versions of their major methods:

  1. A "headless" version that doesn't care about animation but will call executionInfo.terminateWithValue if it can't complete successfully. This is used for the validation of the Quantum maps to see if the user's code passes.
  2. An "animation" version that does care about animation (and audio) but doesn't need to care about validation. This is used to animate the execution of the user's code after we've already validated it with the headless version.

We have so far been calling those methods like this:

  • Make an API call
    • API call calls the subtype headless method
      • subtype headless method does its validation and, if the validation passes, uses executionInfo.queueAction to queue up the animation method

But this means that subtypes need to know about execution info and action queuing. I instead propose we change our expectations to add a requirement that the headless version will return a boolean representing whether or not the validation passed. We can then:

  • Make an API call
    • API call calls the subtype headless method
    • API call conditionally queues up the animation method based on the result of the headless method

This means that subtypes no longer need to know about action queuing, and just need to know enough about executionInfo to be able to terminate if something goes wrong.

Then, in a future PR, I can refactor that away, too, and subtypes will no longer need to have any concept of a user's code being executed.

@Hamms
Copy link
Contributor Author

Hamms commented Feb 27, 2018

PTAL

Copy link
Contributor

@balderdash balderdash left a comment

Choose a reason for hiding this comment

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

I like it.

@@ -258,11 +258,15 @@ exports.loopHighlight = API_FUNCTION(function (id) {
* whether or not we're a Bee level
*/
exports.getNectar = API_FUNCTION(function (id) {
Maze.subtype.getNectar(id);
if (Maze.subtype.getNectar()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should be tryGetNectar, just so it's more clear that it attempts to get nectar?

Perhaps that would change back when you pull the termination value out of the subtype, so I'll leave it up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, maybe I should update the comment.

It not only attempts to get nectar, but also does so if it can, via this line.

The way the maze code execution works (which is probably a bit more complicated than it needs to be) is that is makes all the api calls which as a side effect runs all the headless stuff, then resets the environment and runs the queued animation actions.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's the behavior I'd expect from a tryFoo method (i.e. actually do foo, or return false if foo would throw/actually does throw an exception), but I suppose that expectation is not as universal as I had thought.

I think the comment is fine as is.

@Hamms Hamms force-pushed the bee-api-executionInfo branch 3 times, most recently from 26ac017 to 80d958e Compare March 1, 2018 00:24
Rather than calling a subtype method which THEN queues an action which
then calls a subtype method, this PR untwitsts things to make the flow
of execution more linear
…, make the api responsible for conditionally connecting them
@Hamms Hamms merged commit 39c2a5a into staging Mar 2, 2018
@Hamms Hamms deleted the bee-api-executionInfo branch March 2, 2018 22:09
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.

None yet

2 participants