From f659dfbc3cad4eaa927273516a6b6176292b7c51 Mon Sep 17 00:00:00 2001 From: Paul Kaplan <pkaplan@media.mit.edu> Date: Tue, 27 Nov 2018 11:59:23 -0500 Subject: [PATCH] Move the started flag to the vmStatus reducer --- src/containers/controls.jsx | 5 +++- src/lib/vm-listener-hoc.jsx | 6 +++- src/lib/vm-manager-hoc.jsx | 10 +++---- src/reducers/vm-status.js | 15 ++++++++++ test/unit/util/vm-manager-hoc.test.jsx | 41 ++++++++++++++++++-------- 5 files changed, 58 insertions(+), 19 deletions(-) diff --git a/src/containers/controls.jsx b/src/containers/controls.jsx index 1d4a666d2..db052b494 100644 --- a/src/containers/controls.jsx +++ b/src/containers/controls.jsx @@ -20,7 +20,7 @@ class Controls extends React.Component { if (e.shiftKey) { this.props.vm.setTurboMode(!this.props.turbo); } else { - if (!this.props.vm.started) { + if (!this.props.isStarted) { this.props.vm.start(); } this.props.vm.greenFlag(); @@ -41,6 +41,7 @@ class Controls extends React.Component { render () { const { vm, // eslint-disable-line no-unused-vars + isStarted, // eslint-disable-line no-unused-vars projectRunning, turbo, ...props @@ -58,12 +59,14 @@ class Controls extends React.Component { } Controls.propTypes = { + isStarted: PropTypes.bool.isRequired, projectRunning: PropTypes.bool.isRequired, turbo: PropTypes.bool.isRequired, vm: PropTypes.instanceOf(VM) }; const mapStateToProps = state => ({ + isStarted: state.scratchGui.vmStatus.running, projectRunning: state.scratchGui.vmStatus.running, turbo: state.scratchGui.vmStatus.turbo }); diff --git a/src/lib/vm-listener-hoc.jsx b/src/lib/vm-listener-hoc.jsx index 9dee35d75..58f45e79f 100644 --- a/src/lib/vm-listener-hoc.jsx +++ b/src/lib/vm-listener-hoc.jsx @@ -8,7 +8,7 @@ import {connect} from 'react-redux'; import {updateTargets} from '../reducers/targets'; import {updateBlockDrag} from '../reducers/block-drag'; import {updateMonitors} from '../reducers/monitors'; -import {setRunningState, setTurboState} from '../reducers/vm-status'; +import {setRunningState, setTurboState, setStartedState} from '../reducers/vm-status'; import {showExtensionAlert} from '../reducers/alerts'; import {updateMicIndicator} from '../reducers/mic-indicator'; @@ -39,6 +39,7 @@ const vmListenerHOC = function (WrappedComponent) { this.props.vm.on('TURBO_MODE_OFF', this.props.onTurboModeOff); this.props.vm.on('PROJECT_RUN_START', this.props.onProjectRunStart); this.props.vm.on('PROJECT_RUN_STOP', this.props.onProjectRunStop); + this.props.vm.on('RUNTIME_STARTED', this.props.onRuntimeStarted); this.props.vm.on('PERIPHERAL_DISCONNECT_ERROR', this.props.onShowExtensionAlert); this.props.vm.on('MIC_LISTENING', this.props.onMicListeningUpdate); @@ -110,6 +111,7 @@ const vmListenerHOC = function (WrappedComponent) { onTargetsUpdate, onProjectRunStart, onProjectRunStop, + onRuntimeStarted, onTurboModeOff, onTurboModeOn, onShowExtensionAlert, @@ -128,6 +130,7 @@ const vmListenerHOC = function (WrappedComponent) { onMonitorsUpdate: PropTypes.func.isRequired, onProjectRunStart: PropTypes.func.isRequired, onProjectRunStop: PropTypes.func.isRequired, + onRuntimeStarted: PropTypes.func.isRequired, onShowExtensionAlert: PropTypes.func.isRequired, onTargetsUpdate: PropTypes.func.isRequired, onTurboModeOff: PropTypes.func.isRequired, @@ -158,6 +161,7 @@ const vmListenerHOC = function (WrappedComponent) { }, onProjectRunStart: () => dispatch(setRunningState(true)), onProjectRunStop: () => dispatch(setRunningState(false)), + onRuntimeStarted: () => dispatch(setStartedState(true)), onTurboModeOn: () => dispatch(setTurboState(true)), onTurboModeOff: () => dispatch(setTurboState(false)), onShowExtensionAlert: data => { diff --git a/src/lib/vm-manager-hoc.jsx b/src/lib/vm-manager-hoc.jsx index 159e5c5c0..cbbf10385 100644 --- a/src/lib/vm-manager-hoc.jsx +++ b/src/lib/vm-manager-hoc.jsx @@ -33,9 +33,8 @@ const vmManagerHOC = function (WrappedComponent) { this.props.vm.setCompatibilityMode(true); this.props.vm.initialized = true; } - if (!this.props.isPlayerOnly && !this.props.vm.started) { + if (!this.props.isPlayerOnly && !this.props.isStarted) { this.props.vm.start(); - this.props.vm.started = true; } } componentDidUpdate (prevProps) { @@ -46,9 +45,8 @@ const vmManagerHOC = function (WrappedComponent) { this.loadProject(); } // Start the VM if entering editor mode with an unstarted vm - if (!this.props.isPlayerOnly && !this.props.vm.started) { + if (!this.props.isPlayerOnly && !this.props.isStarted) { this.props.vm.start(); - this.props.vm.started = true; } } loadProject () { @@ -65,6 +63,7 @@ const vmManagerHOC = function (WrappedComponent) { /* eslint-disable no-unused-vars */ fontsLoaded, loadingState, + isStarted, onError: onErrorProp, onLoadedProject: onLoadedProjectProp, projectData, @@ -105,7 +104,8 @@ const vmManagerHOC = function (WrappedComponent) { projectData: state.scratchGui.projectState.projectData, projectId: state.scratchGui.projectState.projectId, loadingState: loadingState, - isPlayerOnly: state.scratchGui.mode.isPlayerOnly + isPlayerOnly: state.scratchGui.mode.isPlayerOnly, + isStarted: state.scratchGui.vmStatus.started }; }; diff --git a/src/reducers/vm-status.js b/src/reducers/vm-status.js index 8e9651992..e99b80368 100644 --- a/src/reducers/vm-status.js +++ b/src/reducers/vm-status.js @@ -1,14 +1,20 @@ const SET_RUNNING_STATE = 'scratch-gui/vm-status/SET_RUNNING_STATE'; const SET_TURBO_STATE = 'scratch-gui/vm-status/SET_TURBO_STATE'; +const SET_STARTED_STATE = 'scratch-gui/vm-status/SET_STARTED_STATE'; const initialState = { running: false, + started: false, turbo: false }; const reducer = function (state, action) { if (typeof state === 'undefined') state = initialState; switch (action.type) { + case SET_STARTED_STATE: + return Object.assign({}, state, { + started: action.started + }); case SET_RUNNING_STATE: return Object.assign({}, state, { running: action.running @@ -22,6 +28,14 @@ const reducer = function (state, action) { } }; +const setStartedState = function (started) { + return { + type: SET_STARTED_STATE, + started: started + }; +}; + + const setRunningState = function (running) { return { type: SET_RUNNING_STATE, @@ -40,5 +54,6 @@ export { reducer as default, initialState as vmStatusInitialState, setRunningState, + setStartedState, setTurboState }; diff --git a/test/unit/util/vm-manager-hoc.test.jsx b/test/unit/util/vm-manager-hoc.test.jsx index 4cee7a49c..798e3a7e4 100644 --- a/test/unit/util/vm-manager-hoc.test.jsx +++ b/test/unit/util/vm-manager-hoc.test.jsx @@ -17,7 +17,8 @@ describe('VMManagerHOC', () => { store = mockStore({ scratchGui: { projectState: {}, - mode: {} // Mocked by using override props in tests when needed + mode: {}, + vmStatus: {} } }); vm = new VM(); @@ -31,6 +32,7 @@ describe('VMManagerHOC', () => { mount( <WrappedComponent isPlayerOnly + isStarted={false} store={store} vm={vm} /> @@ -40,8 +42,7 @@ describe('VMManagerHOC', () => { 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); + expect(vm.start).not.toHaveBeenCalled(); }); test('when it mounts in editor mode, the vm is initialized and started', () => { const Component = () => (<div />); @@ -49,6 +50,7 @@ describe('VMManagerHOC', () => { mount( <WrappedComponent isPlayerOnly={false} + isStarted={false} store={store} vm={vm} /> @@ -57,17 +59,16 @@ describe('VMManagerHOC', () => { expect(vm.setCompatibilityMode.mock.calls.length).toBe(1); expect(vm.initialized).toBe(true); - expect(vm.start.mock.calls.length).toBe(1); - expect(vm.started).toBe(true); + expect(vm.start).toHaveBeenCalled(); }); 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} + isStarted={false} store={store} vm={vm} /> @@ -76,28 +77,44 @@ describe('VMManagerHOC', () => { expect(vm.setCompatibilityMode.mock.calls.length).toBe(0); expect(vm.initialized).toBe(true); - expect(vm.start.mock.calls.length).toBe(1); - expect(vm.started).toBe(true); + expect(vm.start).toHaveBeenCalled(); }); 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 + isStarted={false} store={store} vm={vm} /> ); - expect(vm.start.mock.calls.length).toBe(0); + expect(vm.start).not.toHaveBeenCalled(); mounted.setProps({ isPlayerOnly: false }); - expect(vm.start.mock.calls.length).toBe(1); - expect(vm.started).toBe(true); + expect(vm.start).toHaveBeenCalled(); + }); + test('if it mounts with an initialized and started VM, it does not start again', () => { + const Component = () => <div />; + const WrappedComponent = vmManagerHOC(Component); + vm.initialized = true; + const mounted = mount( + <WrappedComponent + isPlayerOnly + isStarted + store={store} + vm={vm} + /> + ); + expect(vm.start).not.toHaveBeenCalled(); + mounted.setProps({ + isPlayerOnly: false + }); + expect(vm.start).not.toHaveBeenCalled(); }); test('if the isLoadingWithId prop becomes true, it loads project data into the vm', () => { vm.loadProject = jest.fn(() => Promise.resolve()); -- GitLab