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

Clean the script handler a bit. #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"no-shadow": 0,
"no-loop-func": 0,
"eol-last": 0,
"no-use-before-define": 2,
"no-use-before-define": 0,
"new-cap": 0,
"no-constant-condition": 0,
"no-redeclare": 0,
Expand All @@ -37,4 +37,4 @@
"node": true,
"amd": true
}
}
}
125 changes: 22 additions & 103 deletions src/goo/scriptpack/ScriptHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -537,54 +537,11 @@ define([
});
};

// The allowed types for the script parameters.
var PARAMETER_TYPES = [
'string',
'int',
'float',
'vec2',
'vec3',
'vec4',
'boolean',
'texture',
'sound',
'camera',
'entity',
'animation'
];

// Specifies which controls can be used with each type.
var TYPE_CONTROLS = (function () {
var typeControls = {
'string': ['key'],
'int': ['spinner', 'slider', 'jointSelector'],
'float': ['spinner', 'slider'],
'vec2': [],
'vec3': ['color'],
'vec4': ['color'],
'boolean': ['checkbox'],
'texture': [],
'image': [],
'sound': [],
'camera': [],
'entity': [],
'animation': []
};

// Add the controls that can be used with any type to the mapping of
// controls that ca be used for each type.
for (var type in typeControls) {
Array.prototype.push.apply(typeControls[type], ['dropdown', 'select']);
}

return typeControls;
})();

/**
* Load an external script
*/
function loadExternalScript(script, scriptElem, url) {
return PromiseUtils.createPromise(function (resolve, reject) {
return PromiseUtils.createPromise(function (resolve) {
var timeoutHandler;
var handled = false;

Expand Down Expand Up @@ -624,75 +581,37 @@ define([
});
}

var TYPE_VALIDATORS = {
string: function (key, value) {
if (typeof value !== 'string' || value.length === 0) {
return { message: 'Property "' + key + '" must be a non-empty string' };
}
},
number: function (key, value) {
if (typeof value !== 'number') {
return { message: 'Property "' + key + '" must be a number' };
}
},
boolean: function (key, value) {
if (typeof value !== 'boolean') {
return { message: 'Property "' + key + '" must be a boolean' };
}
},
array: function (key, value) {
if (!(value instanceof Array)) {
return { message: 'Property "' + key + '" must be an array' };
}
}
};

var PROPERTY_VALIDATORS = [
{ name: 'key', validator: TYPE_VALIDATORS.string },
{ name: 'name', validator: TYPE_VALIDATORS.string },
{ name: 'control', validator: TYPE_VALIDATORS.string },
{ name: 'min', validator: TYPE_VALIDATORS.number },
{ name: 'max', validator: TYPE_VALIDATORS.number },
{ name: 'scale', validator: TYPE_VALIDATORS.number },
{ name: 'decimals', validator: TYPE_VALIDATORS.number },
{ name: 'precision', validator: TYPE_VALIDATORS.number },
{ name: 'exponential', validator: TYPE_VALIDATORS.boolean }
];

/**
* Validates every property of a parameter defined by a user script.
* Exposed as a static method only for testing purposes.
* @hidden
* @param parameter
* @returns {{message: string}|undefined} May return an error
*/
ScriptHandler.validateParameter = function (parameter) {
// treat key separately; this needs to always be defined
if (typeof parameter.key !== 'string' || parameter.key.length === 0) {
return { message: 'Property "key" must be a non-empty string' };
}
ScriptHandler.validateParameter = function validateParameter(parameter) {
for (var i = 0; i < ScriptUtils.PROPERTY_TYPES.length; ++i) {
var entry = ScriptUtils.PROPERTY_TYPES[i];
var propValue = parameter[entry.prop];
var isPropDefined = typeof propValue !== 'undefined';

// check for types
for (var i = 0; i < PROPERTY_VALIDATORS.length; i++) {
var entry = PROPERTY_VALIDATORS[i];
var msgStart = 'Property "' + entry.prop + '" must be ';

if (typeof parameter[entry.name] !== 'undefined') {
var maybeError = entry.validator(entry.name, parameter[entry.name]);
if (maybeError) {
return maybeError;
if (entry.mustBeDefined || isPropDefined) {
var validator = ScriptUtils.TYPE_VALIDATORS[entry.type];
var allowedValues = entry.getAllowedValues ? entry.getAllowedValues(parameter) : null;

if (isPropDefined && entry.minLength && propValue.length < entry.minLength) {
return { message: msgStart + 'longer than ' + (entry.minLength - 1) };
}
}
}

// check for type in a list of allowed types; must be defined
if (PARAMETER_TYPES.indexOf(parameter.type) === -1) {
return { message: 'Property "type" must be one of: ' + PARAMETER_TYPES.join(', ') };
}
if (allowedValues && allowedValues.indexOf(propValue) === -1) {
return { message: msgStart + 'one of: ' + allowedValues.join(', ') };
}

// check for controls in a list of controls; this depends on type
var allowedControls = TYPE_CONTROLS[parameter.type];
if (parameter.control !== undefined && allowedControls.indexOf(parameter.control) === -1) {
return { message: 'Property "control" must be one of: ' + allowedControls.join(', ') };
if (!validator(propValue)) {
return { message: msgStart + 'of type ' + entry.type };
}
}
}
};

Expand All @@ -703,7 +622,7 @@ define([
* @param script
* @param outScript
*/
ScriptHandler.validateParameters = function (script, outScript) {
ScriptHandler.validateParameters = function validateParameters(script, outScript) {
var errors = script.errors || [];
if (typeof script.externals !== 'object') {
outScript.externals = {};
Expand Down Expand Up @@ -780,4 +699,4 @@ define([
ScriptHandler.DOM_ID_PREFIX = '_script_';

return ScriptHandler;
});
});
81 changes: 79 additions & 2 deletions src/goo/scripts/ScriptUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ define([

function ScriptUtils() {}


ScriptUtils.DEFAULTS_BY_TYPE = {
'float': 0,
'int': 0,
Expand Down Expand Up @@ -64,7 +63,9 @@ define([
};

return {
array: Array.isArray,
'float': _.isNumber,
'number': _.isNumber,
'string': _.isString,
'boolean': _.isBoolean,
'int': _.isInteger,
Expand All @@ -80,6 +81,82 @@ define([
};
})();

// The allowed types for the script parameters.
ScriptUtils.PARAMETER_TYPES = [
'string',
'int',
'float',
'vec2',
'vec3',
'vec4',
'boolean',
'texture',
'sound',
'camera',
'entity',
'animation'
];

// Specifies which controls can be used with each type.
ScriptUtils.PARAMETER_CONTROLS = (function () {
var typeControls = {
'string': ['key'],
'int': ['spinner', 'slider', 'jointSelector'],
'float': ['spinner', 'slider'],
'vec2': [],
'vec3': ['color'],
'vec4': ['color'],
'boolean': ['checkbox'],
'texture': [],
'image': [],
'sound': [],
'camera': [],
'entity': [],
'animation': []
};

// Add the controls that can be used with any type to the mapping of
// controls that ca be used for each type.
for (var type in typeControls) {
Array.prototype.push.apply(typeControls[type], ['dropdown', 'select']);
}

return typeControls;
})();

ScriptUtils.PROPERTY_TYPES = [
{
prop: 'key',
type: 'string',
mustBeDefined: true,
minLength: 1
},
{
prop: 'type',
type: 'string',
mustBeDefined: true,
minLength: 1,
getAllowedValues: function () {
return ScriptUtils.PARAMETER_TYPES;
}
},
{
prop: 'control',
type: 'string',
getAllowedValues: function (parameter) {
// Allowed controls depend on the parameter type.
return ScriptUtils.PARAMETER_CONTROLS[parameter.type];
}
},
{ prop: 'name', type: 'string' },
{ prop: 'min', type: 'number' },
{ prop: 'max', type: 'number' },
{ prop: 'scale', type: 'number' },
{ prop: 'decimals', type: 'number' },
{ prop: 'precision', type: 'number' },
{ prop: 'exponential', type: 'boolean' }
];

/**
* Fill a passed parameters object with defaults from spec
* @hidden
Expand Down Expand Up @@ -277,4 +354,4 @@ define([
};

return ScriptUtils;
});
});
8 changes: 5 additions & 3 deletions test/unit/loaders/handlers/ScriptHandler-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,19 @@ define([
it('returns an error object if the key is missing or is a non-string', function () {
expect(validateParameter({
type: 'float'
})).toEqual({ message: 'Property "key" must be a non-empty string' });
})).toEqual({ message: 'Property "key" must be of type string' });

expect(validateParameter({
key: 123,
type: 'float'
})).toEqual({ message: 'Property "key" must be a non-empty string' });
})).toEqual({ message: 'Property "key" must be of type string' });
});

it('returns an error object if the key is an empty string', function () {
expect(validateParameter({
key: '',
type: 'float'
})).toEqual({ message: 'Property "key" must be a non-empty string' });
})).toEqual({ message: 'Property "key" must be longer than 0' });
});

it('returns an error object if the type is missing or bad', function () {
Expand Down