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

[Spritelab] Implement Pause button behind experiment #38457

Merged
merged 6 commits into from Jan 8, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion apps/src/p5lab/P5Lab.js
Expand Up @@ -391,8 +391,10 @@ P5Lab.prototype.init = function(config) {
(!config.hideSource &&
!config.level.debuggerDisabled &&
!config.level.iframeEmbedAppAndCode);
var showPauseButton = experiments.isEnabled(experiments.SPRITELAB_PAUSE);
var showDebugConsole = config.level.editCode && !config.hideSource;
this.debuggerEnabled = showDebugButtons || showDebugConsole;
this.debuggerEnabled =
showDebugButtons || showPauseButton || showDebugConsole;

if (this.debuggerEnabled) {
getStore().dispatch(
Expand Down
4 changes: 4 additions & 0 deletions apps/src/p5lab/P5LabVisualizationColumn.jsx
Expand Up @@ -5,6 +5,7 @@ import {connect} from 'react-redux';
import classNames from 'classnames';
import GameButtons from '@cdo/apps/templates/GameButtons';
import ArrowButtons from '@cdo/apps/templates/ArrowButtons';
import PauseButton from '@cdo/apps/templates/PauseButton';
import BelowVisualization from '@cdo/apps/templates/BelowVisualization';
import experiments from '@cdo/apps/util/experiments';
import {APP_HEIGHT, APP_WIDTH} from './constants';
Expand Down Expand Up @@ -192,6 +193,9 @@ class P5LabVisualizationColumn extends React.Component {
</div>

<GameButtons>
{experiments.isEnabled(experiments.SPRITELAB_PAUSE) && (
Copy link
Contributor

Choose a reason for hiding this comment

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

this checks localStorage every time the component renders, which has caused performance issues in the past, so it should be moved to a piece of state set during componentDidMount or stored in some other way (memoized or something similar would also work)

<PauseButton />
)}
<ArrowButtons />

<CompletionButton />
Expand Down
2 changes: 1 addition & 1 deletion apps/src/p5lab/spritelab/commands/worldCommands.js
Expand Up @@ -10,7 +10,7 @@ export const commands = {

getTime(unit) {
if (unit === 'seconds') {
return this.World.seconds || 0;
return coreLibrary.getAdjustedWorldTime(this) || 0;
} else if (unit === 'frames') {
return this.World.frameCount || 0;
}
Expand Down
29 changes: 21 additions & 8 deletions apps/src/p5lab/spritelab/coreLibrary.js
Expand Up @@ -4,6 +4,7 @@ var inputEvents = [];
var behaviors = [];
var userInputEventCallbacks = {};
var newSprites = {};
var pauseTime = 0;

export var background;
export var title = '';
Expand All @@ -19,6 +20,19 @@ export function reset() {
title = subtitle = '';
userInputEventCallbacks = {};
defaultSpriteSize = 100;
pauseTime = 0;
}

export function addPauseTime(time) {
pauseTime += time;
}

/**
* Returns World.seconds adjusted to exclude time during which the app was paused
*/
export function getAdjustedWorldTime(p5Inst) {
const current = new Date().getTime();
return Math.round((current - p5Inst._startTime - pauseTime) / 1000);
}

/**
Expand Down Expand Up @@ -205,13 +219,11 @@ export function clearCollectDataEvents() {
function atTimeEvent(inputEvent, p5Inst) {
if (inputEvent.args.unit === 'seconds') {
const previousTime = inputEvent.previousTime || 0;
inputEvent.previousTime = p5Inst.World.seconds;
const worldTime = getAdjustedWorldTime(p5Inst);
inputEvent.previousTime = worldTime;
// There are many ticks per second, but we only want to fire the event once (on the first tick where
// World.seconds matches the event argument)
if (
p5Inst.World.seconds === inputEvent.args.n &&
previousTime !== inputEvent.args.n
) {
// the time matches the event argument)
if (worldTime === inputEvent.args.n && previousTime !== inputEvent.args.n) {
// Call callback with no extra args
return [{}];
}
Expand All @@ -227,10 +239,11 @@ function atTimeEvent(inputEvent, p5Inst) {

function collectDataEvent(inputEvent, p5Inst) {
const previous = inputEvent.previous || 0;
inputEvent.previous = p5Inst.World.seconds;
const worldTime = getAdjustedWorldTime(p5Inst);
inputEvent.previous = worldTime;

// Only log data once per second
if (p5Inst.World.seconds !== previous) {
if (worldTime !== previous) {
// Call callback with no extra args
return [{}];
} else {
Expand Down
63 changes: 63 additions & 0 deletions apps/src/templates/PauseButton.jsx
@@ -0,0 +1,63 @@
import React from 'react';
import {connect} from 'react-redux';
import PropTypes from 'prop-types';
import color from '@cdo/apps/util/color';
import {actions, selectors} from '@cdo/apps/lib/tools/jsdebugger/redux';
import * as coreLibrary from '@cdo/apps/p5lab/spritelab/coreLibrary';

const styles = {
button: {
minWidth: 0,
padding: '10px 13px',
backgroundColor: color.orange,
borderColor: color.orange,
color: color.white
}
};

class PauseButton extends React.Component {
static propTypes = {
// from redux
togglePause: PropTypes.func.isRequired,
isPaused: PropTypes.bool.isRequired,
isAttached: PropTypes.bool.isRequired
};

state = {
pauseStart: 0
};

togglePause = () => {
const current = new Date().getTime();
if (this.props.isPaused) {
coreLibrary.addPauseTime(current - this.state.pauseStart);
Copy link
Contributor

Choose a reason for hiding this comment

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

this function should probably be a prop so that PauseButton doesn't depend on coreLibrary and can be reused

this.setState({pauseStart: 0});
} else {
this.setState({pauseStart: current});
}
this.props.togglePause();
};

render() {
return (
<button
type="button"
onClick={this.togglePause}
style={styles.button}
disabled={!this.props.isAttached}
>
<i className={this.props.isPaused ? 'fa fa-play' : 'fa fa-pause'} />
</button>
);
}
}

export default connect(
state => ({
isAttached: selectors.isAttached(state),
isPaused: selectors.isPaused(state)
}),
{
togglePause: actions.togglePause
}
)(PauseButton);
1 change: 1 addition & 0 deletions apps/src/util/experiments.js
Expand Up @@ -33,6 +33,7 @@ experiments.SPRITELAB_INPUT = 'spritelabInput';
experiments.TIME_SPENT = 'time-spent';
experiments.APPLAB_ML = 'applab-ml';
experiments.BYPASS_DIALOG_POPUP = 'bypass-dialog-popup';
experiments.SPRITELAB_PAUSE = 'spritelabPause';

/**
* Get our query string. Provided as a method so that tests can mock this.
Expand Down