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

Wire sprite info panel to VM #152

Merged
merged 10 commits into from
Mar 2, 2017

Conversation

rschamp
Copy link
Contributor

@rschamp rschamp commented Mar 2, 2017

Sets up sprite info component to reflect data about the selected sprite in the panel.

In the interest of time:

  • Every field is updated as you type, including x/y position. Typing - is interpreted as -1. Eventually this should only happen on blur or enter key pressed. Good luck trying to update position while a sprite is moving 🙃
  • When you select the stage, the sprite info pane becomes disabled, with placeholder values set in the inputs
  • Everything is handled with props passing. This is a good place for redux, but I needed it to be done for the play test tomorrow.
  • I did minimal performance testing. I would not be surprised if some projects perform more slowly after this update. I believe the redux refactor could help with this, since it would reduce the number of parent components that would need to re-render as sprite info changes.

/cc @lifeinchords @cwillisf @carljbowman

Ray Schamp added 8 commits March 1, 2017 17:58
For the time being, update the position onChange. Eventually this will update onBlur/on "enter" key
Very crude, requires a refactor to allow toggling
Refactor sprite info behavior into a container and let it handle the raw events.

TODO: refactor again into redux
@thisandagain
Copy link
Contributor

thisandagain commented Mar 2, 2017

Validation of Sprite Names

One issue that may be ok to resolve later if we file a bug is that it is possible in the PR to have sprites with '' as a name as well as multiple sprites with the same name. This will become important to resolve once we complete work on blocks like point towards [x] and go to [x].

image

Negative Integers in X / Y Position Fields

If you start typing a negative integer (e.g. -50) into the x or y position fields it will always prefix with -1 which leads to unexpected results and can be very frustrating (-50 turns into -150). This issue would probably be good to resolve before testing.

Performance

The performance hit is really quite large and you can feel it while interacting with the GUI (dragging out a block, panning the workspace, adding a sprite, or editing a field) while a simple program is running (forever [turn 15 degrees move 10 steps]). Good example:
https://llk.github.io/scratch-vm/#141490356
https://llk.github.io/scratch-gui/#141490356

Copy link
Contributor

@thisandagain thisandagain left a comment

Choose a reason for hiding this comment

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

I think I'm ok to land this as long as we file bugs for the three issues I listed. I recommend that the negative x/y position issue be resolved before the Friday review.

Ray Schamp added 2 commits March 2, 2017 10:37
These aren't necessary to update in realtime, so throttle them to once every 30ms
@rschamp
Copy link
Contributor Author

rschamp commented Mar 2, 2017

I fixed the performance issues altogether! Hahahaha!

@rschamp
Copy link
Contributor Author

rschamp commented Mar 2, 2017

And agreed on the other two points.

@rschamp rschamp merged commit c0ea247 into scratchfoundation:develop Mar 2, 2017
@rschamp rschamp deleted the wire-sprite-info branch March 2, 2017 15:48
@rschamp rschamp mentioned this pull request Mar 2, 2017
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.

None yet

2 participants