addressed CR feedback

This commit is contained in:
Thijs Wenker 2018-09-25 22:43:58 +02:00
parent 1094b6722f
commit 80beb6c444
3 changed files with 74 additions and 70 deletions

View file

@ -1226,7 +1226,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool
QScriptValue properties = engine->newObject();
EntityItemProperties defaultEntityProperties;
const bool psuedoPropertyFlagsActive = psueudoPropertyFlags & EntityPsuedoPropertyFlag::flagsActive;
const bool psuedoPropertyFlagsActive = psueudoPropertyFlags.test(EntityPsuedoPropertyFlag::FlagsActive);
// Fix to skip the default return all mechanism, when psuedoPropertyFlagsActive
const bool psuedoPropertyFlagsButDesiredEmpty = psuedoPropertyFlagsActive && _desiredProperties.isEmpty();
@ -1235,28 +1235,28 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool
return properties;
}
if (_idSet && (!psuedoPropertyFlagsActive || psueudoPropertyFlags & EntityPsuedoPropertyFlag::id)) {
if (_idSet && (!psuedoPropertyFlagsActive || psueudoPropertyFlags.test(EntityPsuedoPropertyFlag::ID))) {
COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_ALWAYS(id, _id.toString());
}
if (!psuedoPropertyFlagsActive || psueudoPropertyFlags & EntityPsuedoPropertyFlag::type) {
if (!psuedoPropertyFlagsActive || psueudoPropertyFlags.test(EntityPsuedoPropertyFlag::Type)) {
COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_ALWAYS(type, EntityTypes::getEntityTypeName(_type));
}
if (!psuedoPropertyFlagsActive || psueudoPropertyFlags & EntityPsuedoPropertyFlag::created) {
if (!psuedoPropertyFlagsActive || psueudoPropertyFlags.test(EntityPsuedoPropertyFlag::Created)) {
auto created = QDateTime::fromMSecsSinceEpoch(getCreated() / 1000.0f, Qt::UTC); // usec per msec
created.setTimeSpec(Qt::OffsetFromUTC);
COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_ALWAYS(created, created.toString(Qt::ISODate));
}
if ((!skipDefaults || _lifetime != defaultEntityProperties._lifetime) && !strictSemantics) {
if (!psuedoPropertyFlagsActive || psueudoPropertyFlags & EntityPsuedoPropertyFlag::age) {
if (!psuedoPropertyFlagsActive || psueudoPropertyFlags.test(EntityPsuedoPropertyFlag::Age)) {
COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_NO_SKIP(age, getAge()); // gettable, but not settable
}
if (!psuedoPropertyFlagsActive || psueudoPropertyFlags & EntityPsuedoPropertyFlag::ageAsText) {
if (!psuedoPropertyFlagsActive || psueudoPropertyFlags.test(EntityPsuedoPropertyFlag::AgeAsText)) {
COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_NO_SKIP(ageAsText, formatSecondsElapsed(getAge())); // gettable, but not settable
}
}
if (!psuedoPropertyFlagsActive || psueudoPropertyFlags & EntityPsuedoPropertyFlag::lastEdited) {
if (!psuedoPropertyFlagsActive || psueudoPropertyFlags.test(EntityPsuedoPropertyFlag::LastEdited)) {
properties.setProperty("lastEdited", convertScriptValue(engine, _lastEdited));
}
COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_LAST_EDITED_BY, lastEditedBy);
@ -1489,7 +1489,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool
* @property {Vec3} dimensions - The dimensions of the AA box.
*/
if (!skipDefaults && !strictSemantics &&
(!psuedoPropertyFlagsActive || psueudoPropertyFlags & EntityPsuedoPropertyFlag::boundingBox)) {
(!psuedoPropertyFlagsActive || psueudoPropertyFlags.test(EntityPsuedoPropertyFlag::BoundingBox))) {
AABox aaBox = getAABox();
QScriptValue boundingBox = engine->newObject();
@ -1505,7 +1505,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool
}
QString textureNamesStr = QJsonDocument::fromVariant(_textureNames).toJson();
if (!skipDefaults && !strictSemantics && (!psuedoPropertyFlagsActive || psueudoPropertyFlags & EntityPsuedoPropertyFlag::originalTextures)) {
if (!skipDefaults && !strictSemantics && (!psuedoPropertyFlagsActive || psueudoPropertyFlags.test(EntityPsuedoPropertyFlag::OriginalTextures))) {
COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_NO_SKIP(originalTextures, textureNamesStr); // gettable, but not settable
}
@ -1532,7 +1532,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool
// Rendering info
if (!skipDefaults && !strictSemantics &&
(!psuedoPropertyFlagsActive || psueudoPropertyFlags & EntityPsuedoPropertyFlag::renderInfo)) {
(!psuedoPropertyFlagsActive || psueudoPropertyFlags.test(EntityPsuedoPropertyFlag::RenderInfo))) {
QScriptValue renderInfo = engine->newObject();
@ -1559,10 +1559,10 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool
}
// FIXME: These properties should already have been set above.
if (!psuedoPropertyFlagsActive || psueudoPropertyFlags & EntityPsuedoPropertyFlag::clientOnly) {
if (!psuedoPropertyFlagsActive || psueudoPropertyFlags.test(EntityPsuedoPropertyFlag::ClientOnly)) {
properties.setProperty("clientOnly", convertScriptValue(engine, getClientOnly()));
}
if (!psuedoPropertyFlagsActive || psueudoPropertyFlags & EntityPsuedoPropertyFlag::owningAvatarID) {
if (!psuedoPropertyFlagsActive || psueudoPropertyFlags.test(EntityPsuedoPropertyFlag::OwningAvatarID)) {
properties.setProperty("owningAvatarID", convertScriptValue(engine, getOwningAvatarID()));
}

View file

@ -14,22 +14,28 @@
#ifndef hifi_EntityPsuedoPropertyFlag_h
#define hifi_EntityPsuedoPropertyFlag_h
enum class EntityPsuedoPropertyFlag {
none = 0,
flagsActive = 1 << 0,
id = 1 << 1,
type = 1 << 2,
created = 1 << 3,
age = 1 << 4,
ageAsText = 1 << 5,
lastEdited = 1 << 6,
boundingBox = 1 << 7,
originalTextures = 1 << 8,
renderInfo = 1 << 9,
clientOnly = 1 << 10,
owningAvatarID = 1 << 11,
all = (1 << 12) - 1
};
Q_DECLARE_FLAGS(EntityPsuedoPropertyFlags, EntityPsuedoPropertyFlag)
#include <bitset>
#include <type_traits>
namespace EntityPsuedoPropertyFlag {
enum {
None = 0,
FlagsActive,
ID,
Type,
Created,
Age,
AgeAsText,
LastEdited,
BoundingBox,
OriginalTextures,
RenderInfo,
ClientOnly,
OwningAvatarID,
NumFlags
};
}
typedef std::bitset<EntityPsuedoPropertyFlag::NumFlags> EntityPsuedoPropertyFlags;
#endif // hifi_EntityPsuedoPropertyFlag_h

View file

@ -427,51 +427,51 @@ QScriptValue EntityScriptingInterface::getMultipleEntityPropertiesInternal(QScri
EntityPsuedoPropertyFlags psuedoPropertyFlags;
const auto readExtendedPropertyStringValue = [&](QScriptValue extendedProperty) {
const auto extendedPropertyString = extendedProperty.toString();
if (extendedPropertyString == QString("id")) {
psuedoPropertyFlags |= EntityPsuedoPropertyFlag::id;
} else if (extendedPropertyString == QString("type")) {
psuedoPropertyFlags |= EntityPsuedoPropertyFlag::type;
} else if (extendedPropertyString == QString("created")) {
psuedoPropertyFlags |= EntityPsuedoPropertyFlag::created;
} else if (extendedPropertyString == QString("age")) {
psuedoPropertyFlags |= EntityPsuedoPropertyFlag::age;
} else if (extendedPropertyString == QString("ageAsText")) {
psuedoPropertyFlags |= EntityPsuedoPropertyFlag::ageAsText;
} else if (extendedPropertyString == QString("lastEdited")) {
psuedoPropertyFlags |= EntityPsuedoPropertyFlag::lastEdited;
} else if (extendedPropertyString == QString("boundingBox")) {
psuedoPropertyFlags |= EntityPsuedoPropertyFlag::boundingBox;
} else if (extendedPropertyString == QString("originalTextures")) {
psuedoPropertyFlags |= EntityPsuedoPropertyFlag::originalTextures;
} else if (extendedPropertyString == QString("renderInfo")) {
psuedoPropertyFlags |= EntityPsuedoPropertyFlag::renderInfo;
} else if (extendedPropertyString == QString("clientOnly")) {
psuedoPropertyFlags |= EntityPsuedoPropertyFlag::clientOnly;
} else if (extendedPropertyString == QString("owningAvatarID")) {
psuedoPropertyFlags |= EntityPsuedoPropertyFlag::owningAvatarID;
if (extendedPropertyString == "id") {
psuedoPropertyFlags.set(EntityPsuedoPropertyFlag::ID);
} else if (extendedPropertyString == "type") {
psuedoPropertyFlags.set(EntityPsuedoPropertyFlag::Type);
} else if (extendedPropertyString == "created") {
psuedoPropertyFlags.set(EntityPsuedoPropertyFlag::Created);
} else if (extendedPropertyString == "age") {
psuedoPropertyFlags.set(EntityPsuedoPropertyFlag::Age);
} else if (extendedPropertyString == "ageAsText") {
psuedoPropertyFlags.set(EntityPsuedoPropertyFlag::AgeAsText);
} else if (extendedPropertyString == "lastEdited") {
psuedoPropertyFlags.set(EntityPsuedoPropertyFlag::LastEdited);
} else if (extendedPropertyString == "boundingBox") {
psuedoPropertyFlags.set(EntityPsuedoPropertyFlag::BoundingBox);
} else if (extendedPropertyString == "originalTextures") {
psuedoPropertyFlags.set(EntityPsuedoPropertyFlag::OriginalTextures);
} else if (extendedPropertyString == "renderInfo") {
psuedoPropertyFlags.set(EntityPsuedoPropertyFlag::RenderInfo);
} else if (extendedPropertyString == "clientOnly") {
psuedoPropertyFlags.set(EntityPsuedoPropertyFlag::ClientOnly);
} else if (extendedPropertyString == "owningAvatarID") {
psuedoPropertyFlags.set(EntityPsuedoPropertyFlag::OwningAvatarID);
}
};
if (extendedDesiredProperties.isString()) {
readExtendedPropertyStringValue(extendedDesiredProperties);
psuedoPropertyFlags |= EntityPsuedoPropertyFlag::flagsActive;
psuedoPropertyFlags.set(EntityPsuedoPropertyFlag::FlagsActive);
} else if (extendedDesiredProperties.isArray()) {
const quint32 length = extendedDesiredProperties.property("length").toInt32();
for (quint32 i = 0; i < length; i++) {
readExtendedPropertyStringValue(extendedDesiredProperties.property(i));
}
psuedoPropertyFlags |= EntityPsuedoPropertyFlag::flagsActive;
psuedoPropertyFlags.set(EntityPsuedoPropertyFlag::FlagsActive);
}
EntityPropertyFlags desiredProperties = qScriptValueToValue<EntityPropertyFlags>(extendedDesiredProperties);
bool needsScriptSymantics = desiredProperties.getHasProperty(PROP_POSITION) ||
bool needsScriptSemantics = desiredProperties.getHasProperty(PROP_POSITION) ||
desiredProperties.getHasProperty(PROP_ROTATION) ||
desiredProperties.getHasProperty(PROP_LOCAL_POSITION) ||
desiredProperties.getHasProperty(PROP_LOCAL_ROTATION) ||
desiredProperties.getHasProperty(PROP_LOCAL_VELOCITY) ||
desiredProperties.getHasProperty(PROP_LOCAL_ANGULAR_VELOCITY) ||
desiredProperties.getHasProperty(PROP_LOCAL_DIMENSIONS);
if (needsScriptSymantics) {
if (needsScriptSemantics) {
// if we are explicitly getting position or rotation, we need parent information to make sense of them.
desiredProperties.setHasProperty(PROP_PARENT_ID);
desiredProperties.setHasProperty(PROP_PARENT_JOINT_INDEX);
@ -488,7 +488,7 @@ QScriptValue EntityScriptingInterface::getMultipleEntityPropertiesInternal(QScri
const auto& entityID = entityIDs.at(i);
const EntityItemPointer entity = _entityTree->findEntityByEntityItemID(EntityItemID(entityID));
if (entity) {
if (!psuedoPropertyFlags && desiredProperties.isEmpty()) {
if (psuedoPropertyFlags.none() && desiredProperties.isEmpty()) {
// these are left out of EntityItem::getEntityProperties so that localPosition and localRotation
// don't end up in json saves, etc. We still want them here, though.
EncodeBitstreamParams params; // unknown
@ -498,8 +498,8 @@ QScriptValue EntityScriptingInterface::getMultipleEntityPropertiesInternal(QScri
desiredProperties.setHasProperty(PROP_LOCAL_VELOCITY);
desiredProperties.setHasProperty(PROP_LOCAL_ANGULAR_VELOCITY);
desiredProperties.setHasProperty(PROP_LOCAL_DIMENSIONS);
psuedoPropertyFlags = EntityPsuedoPropertyFlag::all;
needsScriptSymantics = true;
psuedoPropertyFlags.set();
needsScriptSemantics = true;
}
auto properties = entity->getProperties(desiredProperties, true);
@ -512,18 +512,16 @@ QScriptValue EntityScriptingInterface::getMultipleEntityPropertiesInternal(QScri
}
QScriptValue finalResult = engine->newArray(resultProperties.size());
quint32 i = 0;
{
if (needsScriptSymantics) {
PROFILE_RANGE(script_entities, "EntityScriptingInterface::getMultipleEntityProperties>Script Semantics");
foreach(auto result, resultProperties) {
finalResult.setProperty(i++, convertPropertiesToScriptSemantics(result.properties, result.scalesWithParent)
.copyToScriptValue(engine, false, false, false, psuedoPropertyFlags));
}
} else {
PROFILE_RANGE(script_entities, "EntityScriptingInterface::getMultipleEntityProperties>Skip Script Semantics");
foreach(auto result, resultProperties) {
finalResult.setProperty(i++, result.properties.copyToScriptValue(engine, false, false, false, psuedoPropertyFlags));
}
if (needsScriptSemantics) {
PROFILE_RANGE(script_entities, "EntityScriptingInterface::getMultipleEntityProperties>Script Semantics");
foreach(const auto& result, resultProperties) {
finalResult.setProperty(i++, convertPropertiesToScriptSemantics(result.properties, result.scalesWithParent)
.copyToScriptValue(engine, false, false, false, psuedoPropertyFlags));
}
} else {
PROFILE_RANGE(script_entities, "EntityScriptingInterface::getMultipleEntityProperties>Skip Script Semantics");
foreach(const auto& result, resultProperties) {
finalResult.setProperty(i++, result.properties.copyToScriptValue(engine, false, false, false, psuedoPropertyFlags));
}
}
return finalResult;