jmoenig / Snap

a visual programming language inspired by Scratch
http://snap.berkeley.edu
GNU Affero General Public License v3.0
1.51k stars 748 forks source link

Error Serializing Reporters as Commands #942

Open cycomachead opened 9 years ago

cycomachead commented 9 years ago

I'm trying to investigate this one, but this file is type-error-ing when I import it, and I'm not sure why...

File: https://s3.amazonaws.com/uploads.hipchat.com/121233/884207/ct4BnKiTCNeaYgS/relkhoury__Encriptify__2015-09-21%2001%3A34%3A57.xml

cycomachead commented 9 years ago

Ok, I've been able to fix the file.

The issue occurs because the block user selects encrypt (ec) is being used as both a command and a predicate in the project. Somehow the XML is saying the type is a predicate but the following is also seen:

                <script>
                    <custom-block s="encrypt or decrypt?"/>
                    <custom-block s="user selects encrypt (ec)"/>
                    <custom-block s="user selects decrypt (ec)"/>
                </script>

Not sure how that happened, but worth investigating.

cycomachead commented 9 years ago

So, I'll add that this same issue happened in a second separate case where the block types mismatched during import, and manually editing the file fixed it..weird.

cycomachead commented 9 years ago

@jmoenig This occurred again in this file: https://s3.amazonaws.com/uploads.hipchat.com/121233/1137141/HFkvlwkHwocyTwS/lelandhw3nelly--HW3_LelandJones_NelidaAlmeida-Bryant--2015-10-12%2019-44-02.xml

There's an issue with the "murge" (their name) block and the "shift up" blocks which are reporters being serialized in a script that makes it look like they are command blocks.

cycomachead commented 8 years ago

See this project: http://snap.berkeley.edu/snapsource/snap.html#present:Username=chaft&ProjectName=Monarch%20Mayhem

Here is the XML: https://s3.amazonaws.com/uploads.hipchat.com/121233/1137141/XxG02YUDnbGJ4Xp/chaft--Monarch%20Mayhem--2015-12-01%2012-03-58.xml

Haven't had a chance to look at this.

Cc @cs10/tas so you all see this have might take a look.

cycomachead commented 8 years ago

Oops. @cs10/staff -- this should work!

cycomachead commented 8 years ago

In the above project, we have another block is is a reporter, but appears to be stuck in the middle of a stack of commands. I was able to fix the issue, but we still have a weird problem in Snap! :(

jmoenig commented 8 years ago

Wow, looks like we now have enough material so we should be able to reproduce this ourselves...

cycomachead commented 8 years ago

The problem is that I'm only able to test the deserialization, but everything so far seems to be this isolated case.

I could even "fix" project loading to ignore this error (and warn the user) but that doesn't really solve the underlying issue. My usual solution is to just pullout the reporter from the stack of command blocks, and just tell them what script I modified. So far, these scripts have been non-essential test scripts when students are working.

cycomachead commented 8 years ago

So, we asked the student what he did, and this was the response:

Hello, Thanks so much for retrieving my project! Also to answer your questions: I did indeed turn my "enemy select group" from a command to a reporter block, I wanted to have the block report the group to be chosen. I kind of remember having the editor for the "enemy select group" block open when saving the project on cloud. Again, thank you so much for getting my project back! [name-...]

jmoenig commented 8 years ago

Aha! Now that's an excellent lead, because it actually could make sense. Maybe I can even manage to reproduce it now. Thank you so much for investigating into this issue!

cycomachead commented 8 years ago

AH HA! I have discovered a bug, not sure if it's related, but probably!

  1. Make a block (say a command)
  2. Change the type (say to a reporter) 2.1 Do not click apply
  3. Drag out block to script area
  4. Click apply 4.1 Command block still exists
jmoenig commented 8 years ago

Yep! Excellent, Michael!! I'm almost sure this is at the bottom of the issue we're seeing here. Good work catching this problem.

Come to think of it I'm not sure how to solve this actually. Geez. My first inclination is to disable changing bock types altogether, but everybody is going to protest against that. Another idea would be to force "apply" whenever a block type gets changed after the initial create. That way there could be no in-between states. That's probably the sensible thing to do.

Good work!

cycomachead commented 8 years ago

Come to think of it I'm not sure how to solve this actually. Geez.

Yeah, that's why I didn't have a solution! :stuck_out_tongue:

But I do think selectively "force-apply-ing" would be a good idea in this case. (Somewhat unrelated, but it would be nice to be able to go reporter <--> predicate even with blocks on the stage, since that doesn't break serialization or the semantics of scripts.)

jmoenig commented 8 years ago

Good point about reporter <--> predicate!

jmoenig commented 5 years ago

is this still happening?

cycomachead commented 5 years ago

I'll test later.