From 59df9b6ca53288e88ab56f7517a5719eb596ba80 Mon Sep 17 00:00:00 2001
From: Paul Kaplan <pkaplan@media.mit.edu>
Date: Tue, 27 Nov 2018 11:25:04 -0500
Subject: [PATCH] Do not auto-start the VM in player mode. Always start before
green flag.
---
src/containers/controls.jsx | 3 ++
src/lib/vm-manager-hoc.jsx | 25 ++++++++----
test/unit/util/vm-manager-hoc.test.jsx | 56 +++++++++++++++++++++++---
3 files changed, 72 insertions(+), 12 deletions(-)
diff --git a/src/containers/controls.jsx b/src/containers/controls.jsx
index 2869a047a..1d4a666d2 100644
--- a/src/containers/controls.jsx
+++ b/src/containers/controls.jsx
@@ -20,6 +20,9 @@ class Controls extends React.Component {
if (e.shiftKey) {
this.props.vm.setTurboMode(!this.props.turbo);
} else {
+ if (!this.props.vm.started) {
+ this.props.vm.start();
+ }
this.props.vm.greenFlag();
analytics.event({
category: 'general',
diff --git a/src/lib/vm-manager-hoc.jsx b/src/lib/vm-manager-hoc.jsx
index 404b7e3c8..159e5c5c0 100644
--- a/src/lib/vm-manager-hoc.jsx
+++ b/src/lib/vm-manager-hoc.jsx
@@ -27,12 +27,16 @@ const vmManagerHOC = function (WrappedComponent) {
]);
}
componentDidMount () {
- if (this.props.vm.initialized) return;
- this.audioEngine = new AudioEngine();
- this.props.vm.attachAudioEngine(this.audioEngine);
- this.props.vm.setCompatibilityMode(true);
- this.props.vm.start();
- this.props.vm.initialized = true;
+ if (!this.props.vm.initialized) {
+ this.audioEngine = new AudioEngine();
+ this.props.vm.attachAudioEngine(this.audioEngine);
+ this.props.vm.setCompatibilityMode(true);
+ this.props.vm.initialized = true;
+ }
+ if (!this.props.isPlayerOnly && !this.props.vm.started) {
+ this.props.vm.start();
+ this.props.vm.started = true;
+ }
}
componentDidUpdate (prevProps) {
// if project is in loading state, AND fonts are loaded,
@@ -41,6 +45,11 @@ const vmManagerHOC = function (WrappedComponent) {
(!prevProps.isLoadingWithId || !prevProps.fontsLoaded)) {
this.loadProject();
}
+ // Start the VM if entering editor mode with an unstarted vm
+ if (!this.props.isPlayerOnly && !this.props.vm.started) {
+ this.props.vm.start();
+ this.props.vm.started = true;
+ }
}
loadProject () {
return this.props.vm.loadProject(this.props.projectData)
@@ -79,6 +88,7 @@ const vmManagerHOC = function (WrappedComponent) {
cloudHost: PropTypes.string,
fontsLoaded: PropTypes.bool,
isLoadingWithId: PropTypes.bool,
+ isPlayerOnly: PropTypes.bool,
loadingState: PropTypes.oneOf(LoadingStates),
onError: PropTypes.func,
onLoadedProject: PropTypes.func,
@@ -94,7 +104,8 @@ const vmManagerHOC = function (WrappedComponent) {
isLoadingWithId: getIsLoadingWithId(loadingState),
projectData: state.scratchGui.projectState.projectData,
projectId: state.scratchGui.projectState.projectId,
- loadingState: loadingState
+ loadingState: loadingState,
+ isPlayerOnly: state.scratchGui.mode.isPlayerOnly
};
};
diff --git a/test/unit/util/vm-manager-hoc.test.jsx b/test/unit/util/vm-manager-hoc.test.jsx
index 8c065d83d..4cee7a49c 100644
--- a/test/unit/util/vm-manager-hoc.test.jsx
+++ b/test/unit/util/vm-manager-hoc.test.jsx
@@ -16,7 +16,8 @@ describe('VMManagerHOC', () => {
beforeEach(() => {
store = mockStore({
scratchGui: {
- projectState: {}
+ projectState: {},
+ mode: {} // Mocked by using override props in tests when needed
}
});
vm = new VM();
@@ -24,34 +25,79 @@ describe('VMManagerHOC', () => {
vm.setCompatibilityMode = jest.fn();
vm.start = jest.fn();
});
- test('when it mounts, the vm is initialized', () => {
+ test('when it mounts in player mode, the vm is initialized but not started', () => {
const Component = () => (<div />);
const WrappedComponent = vmManagerHOC(Component);
mount(
<WrappedComponent
+ isPlayerOnly
+ store={store}
+ vm={vm}
+ />
+ );
+ expect(vm.attachAudioEngine.mock.calls.length).toBe(1);
+ expect(vm.setCompatibilityMode.mock.calls.length).toBe(1);
+ expect(vm.initialized).toBe(true);
+
+ // But vm should not be started automatically
+ expect(vm.start.mock.calls.length).toBe(0);
+ expect(vm.started).not.toBe(true);
+ });
+ test('when it mounts in editor mode, the vm is initialized and started', () => {
+ const Component = () => (<div />);
+ const WrappedComponent = vmManagerHOC(Component);
+ mount(
+ <WrappedComponent
+ isPlayerOnly={false}
store={store}
vm={vm}
/>
);
expect(vm.attachAudioEngine.mock.calls.length).toBe(1);
expect(vm.setCompatibilityMode.mock.calls.length).toBe(1);
- expect(vm.start.mock.calls.length).toBe(1);
expect(vm.initialized).toBe(true);
+
+ expect(vm.start.mock.calls.length).toBe(1);
+ expect(vm.started).toBe(true);
});
- test('if it mounts with an initialized vm, it does not reinitialize the vm', () => {
+ test('if it mounts with an initialized vm, it does not reinitialize the vm but will start it', () => {
const Component = () => <div />;
const WrappedComponent = vmManagerHOC(Component);
vm.initialized = true;
+ vm.started = false;
mount(
<WrappedComponent
+ isPlayerOnly={false}
store={store}
vm={vm}
/>
);
expect(vm.attachAudioEngine.mock.calls.length).toBe(0);
expect(vm.setCompatibilityMode.mock.calls.length).toBe(0);
- expect(vm.start.mock.calls.length).toBe(0);
expect(vm.initialized).toBe(true);
+
+ expect(vm.start.mock.calls.length).toBe(1);
+ expect(vm.started).toBe(true);
+ });
+
+ test('if it mounts without starting the VM, it can be started by switching to editor mode', () => {
+ const Component = () => <div />;
+ const WrappedComponent = vmManagerHOC(Component);
+ vm.initialized = true;
+ vm.started = false;
+ const mounted = mount(
+ <WrappedComponent
+ isPlayerOnly
+ store={store}
+ vm={vm}
+ />
+ );
+ expect(vm.start.mock.calls.length).toBe(0);
+ mounted.setProps({
+ isPlayerOnly: false
+ });
+ expect(vm.start.mock.calls.length).toBe(1);
+ expect(vm.started).toBe(true);
});
test('if the isLoadingWithId prop becomes true, it loads project data into the vm', () => {
vm.loadProject = jest.fn(() => Promise.resolve());
--
GitLab