Edited - Finished the doAsk and answer functionality and brought the project under test.

[brianpilati]

I am still having problems running Scratch HTML5 on my local machine, so ... I have started to bring the project under test.

I decided to go with Karma and Jasmine -- a great combination for Javascript testing. I brought Scratch.js and IO.js under test. Admittedly, there are not "A LOT" of tests now but I wanted to get some feedback before adding more tests (and figuring out what is wrong on my local machine).

In order to move Scratch under test, I needed to make some changes to index.html and Scratch.js. I have updated the README file to help with how to get the Unit Tests running.

Please give me some feedback, direction and vision on the tests.

[brianpilati]

I forgot the compare.html file ...

[brianpilati]

I added some more tests -- not a lot but some. I see that you have not merged my previous pull-request. Before I move forward, I would like to know your thoughts on my contributions.

I would love to start to add more code and tests.

Cheers,
Brian

[brianpilati]

@sclements You should really keep-up on the pull-requests. I have spent some time tonight working on the "doAsk" and "answer" functionality plus added substantial tests only to realize that someone else had already done the work ... but it wasn't merged. Ooops, my bad.

So is this project dead? I started to get committed but I need some reassurance from you.

I did find a few bugs that I fixed --

1- Stage shouldn't have a say or think element.
2- Reporter labels were undefined and I fixed them. I removed the "label" variable. I could not find any references to it.
3- There is still a problem with the ability to name a global and sprite only variable the same name. I will need to think about this problem.
4- There is a fundamental flaw in the use of sprites. For example, two sprites can both have "say" blocks at the same time. However, because the sprite elements are not nested, only one will appear. I think the elements of a sprite should be nested. Thoughts?

Please let me know how active this project is ... I keep saying that I will wait for your word but this time I am going to pause, regardless of how much my children whine!

[brianpilati]

@sclements I reviewed the other pull-request for doAsk and answer. I did not include the thread pause but I noticed it late in the night and project.

I did ensure the z-index was always correct and attempted to match the current code.

Either way, I did not add a "bind" method because of the nested sprite problem. The bind-to-element will not be able to distinguish which doAsk submit button to reference. Or there is only one doAsk element on the stage.

Let me know you thoughts and next steps, so I can contribute.

[brianpilati]

@sclements Oh, btw -- I wanted to compliment you on your code. It is a dream to work with. I have seen a lot of code and this project is right up there with amazing!

If we move forward with the project, it will only take me a few more nights to bring the entire project under test. Then, per your request, we will be able to move much faster.

[nathan]

I did find a few bugs that I fixed

Putting more changes in one pull request only means that it will take longer to merge it—especially when you mix bug fixes and entirely new features.

For example, two sprites can both have "say" blocks at the same time. However, because the sprite elements are not nested, only one will appear.

That does not seem to be the case.

[brianpilati]

@nathan Good point on the combining pull-requests. Once you start to add tests and functionality, your velocity increases and combining features and bugs is one of the by-products. Unfortunately, I still have at least one more commit to get the click-handler working. My comment on pull-requests was more towards now we have two doAsk/answer pull-requests.

I found an example of when two Sprites do not show a "say" at the same time. After further review, it's not that the "say" is not displayed but it is displayed off of the stage (random sprite positioning created this bug). Which brings me to another issue I discovered. In the Flash version, the say and think are positioned above the sprite taking into account transparency and not the top, right corner. This makes sense to my because sometimes, I will create a 480x360 image that is only 25x25. It makes sense for quick positioning, etc. In addition, it appears that if the say will run off of the stage that it is moved down. Below is my example. I will add two issues. The transparency positioning needs some thought and discussion.

http://scratch.mit.edu/projects/18965450

After a semi-good night's sleep, your posted helped my see the light on the nested elements issue. You have a reference to the say and think element in the Sprite. Nice. At this point, the nested sprites would be more of a convenience in readability than functionality.

[brianpilati]

@nathan @sclements Okay guys, the doAsk functionality is complete. Please review this code and let me know any changes I need to make.

I have found a few more issues that I would like to work on but don't want to continue to add to this pull-request.

[sclements]

Here is my output from the test script: (shouldn't the tests be passing?)

Starting Karma Server (http://karma-runner.github.io)

WARN [plugin]: Error during loading "jasmine-jquery" plugin:
Cannot read property '1' of null
INFO [karma]: Karma v0.10.9 server started at http://localhost:9876/
INFO [launcher]: Starting browser Chrome
INFO [Chrome 33.0.1750 (Mac OS X 10.9.2)]: Connected on socket cWsDwNCVVhD_PFjrnsg2
Chrome 33.0.1750 (Mac OS X 10.9.2) Scratch Scratch - Load Project should call the internalapi project FAILED
TypeError: Object # has no method 'promise'
at null. (/Users/shanemc/projects/html5/test/unit/scratchSpec.js:15:17)
Expected spy onSuccess to have been called.
Error: Expected spy onSuccess to have been called.
at null. (/Users/shanemc/projects/html5/test/unit/scratchSpec.js:20:26)
Chrome 33.0.1750 (Mac OS X 10.9.2): Executed 63 of 130 (1 FAILED) (0 secs / 0.042 secs)
Chrome 33.0.1750 (Mac OS X 10.9.2): Executed 130 of 130 (1 FAILED) (0.229 secs / 0.1 secs)

[nathan]

type=… is unnecessary.

[brianpilati]

done

[brianpilati]

I believe I have made all of the necessary corrections. Thanks for the feedback. If I have missed anything, please leave me more comment. I appreciate your thoroughness on the reviews.

The tests should now be passing -- how funny/embarrassing is that to bring the project until test only to have them fail. I had a dependency on mock-ajax but forgot that I submitted a pull-request with new functionality which has not been accepted yet. I have now removed the dependency and the tests "better" ;) pass now. If not, let me know.

I have a branch that I am working on to run the tests headless using PhantomJS. Then the tests could be run on drone.io and embedded into the README.md file. There are two tests failing right now (but they are all passing using Chrome???). I will let you know when I am successful in a new pull-request.

[sclements]

Thanks for reviewing the rest of the code @nathan! It's great to be getting this code into a state to add unit tests. Thanks @brianpilati.

[nathan]

Is there a reason you changed img/playerStartFlag.png in the ask PR?

[brianpilati]

@nathan -- OCD. The flash version has a slight opacity that I attempted to capture.

[nathan]

That was the original asset from the flash version. It might be better to apply the opacity with CSS.

[nathan]

@sclements I feel like this was merged prematurely, as it fails most of the more complex ask cases.

[nathan]

http://scratch.mit.edu/projects/19100561/ demonstrates some of them:

• The stage should be able to ask.
• Enter/return should submit the prompt.
• Sprites should not ask simultaneously.
• There are some white spots on the bottom edge of the bubble: .

[nathan]

http://scratch.mit.edu/projects/19101047/ demonstrates another:

• Hidden sprites should be able to ask.

[sclements]

While I was aware that this code did not complete the Ask feature (and yes
I did notice that the graphic was inappropriately changed), I valued that
ability to bring the project under test. Please do not combine features
like this in the future. We do need to revert the image change since CSS
is the proper way to do this and implements it closest to the Flash version.

I'm currently working on open sourcing the Flash version so that features
like Ask can be implemented in the most compatible way. Right now, the
code committed is incomplete, as you pointed out Nathan, and has other
issues like static CSS is being set in code. Obviously this feature is not
done and needs much more work. Hopefully I can complete the process of
open sourcing the Flash code soon so that it can be used as a reference.

On Tue, Mar 11, 2014 at 9:24 AM, Nathan Dinsmore
notifications@github.comwrote:

http://scratch.mit.edu/projects/19101047/ demonstrates another:

• Hidden sprites should be able to ask.

Reply to this email directly or view it on GitHubhttps://github.com//pull/43#issuecomment-37308233
.

[brianpilati]

@sclements So should I continue to work on parity or should I wait until the flash version is open-sourced? I would like to continue to contribute and it is difficult to determine how everything should work by just using the compare.html file.

Is the compare.html page the "reference" for parity?

@nathan Just let me know the items that I need to change or enhance.

[brianpilati]

I do have the original start flag. Do you want me to add it to my current pull-request of updates to the doAsk code or wait for a future pull-request that is only the icon?

[brianpilati]

I have added support for the following:

Hidden sprites should be able to ask.
The stage should be able to ask.
Enter/return should submit the prompt

I have been looking at simultaneous asks and the stack only persists two of the four append:toList commands. The third one is received after. I have no idea about the fourth.

Debugging that portion of the code will take me some time to bring it under test. I will probably need to review it, think about it and come back to it after some time. I will let you know.

[brianpilati]

I was able to finish: Sprites should not ask simultaneously and I removed the static css.

As I mentioned previously, the order of the "sprite asks" is consistently d,a,c,b. Here is my example: http://scratch.mit.edu/projects/19136054/#editor

I will look at the order another day. I love to TDD but bringing the Intrepreter.js file under tests does not sound like fun today! ;)

I would like to work on #45, #46 and #49 for a few days, if that is okay while you review this code? Once this code is merged, I will create a pull-request for the "green flag" and the other issues, one at a time.

Let me know any issues that I need to address with this pull-request too.

Thanks Guys!