Reference properties #1774
Reference properties #1774
Conversation
Old tests all pass, update simple.czml so that it works again. Add ability to reference sub-properties.
Completely rewrite tests and refactor implementation to handle sub-properties and definitionChanged events.
Also make local CZML references work (though implementation is a little hacky right now)
Conflicts: CHANGES.md Source/DynamicScene/ReferenceProperty.js
| @@ -1724,6 +1761,7 @@ define([ | |||
| * @param {Object} packetData The CZML packet being processed.y | |||
| * @param {TimeInterval} [interval] A constraining interval for which the data is valid. | |||
| * @param {String} [sourceUri] The originating uri of the data being processed. | |||
| * @param {DynamicObjectCollection} [dynamicObjectCollection] The collection being processsed. | |||
There was a problem hiding this comment.
This parameter is not really optional, is it? It will crash if you don't provide it and you process a packet with a reference in it.
There was a problem hiding this comment.
None of the ones marked optional are actually optional I think. I'll change the doc. This entire file is due for a major cleanup, overhaul before we hit 1.0; I will probably do it as part of the "automatically process custom properties" changes.
|
Your current set of comments have all been addressed. |
| return -1; | ||
| } | ||
|
|
||
| function trySplit(value, delimiter) { |
There was a problem hiding this comment.
It seems like it would be a lot simpler to just walk the reference string, start to finish, and parse as you go, keeping state variables to detect escape sequences. You really don't need to pre-determine all the indices up front.
basically (this is concise enough to just go in fromString)
var identifier;
var values = [];
var inIdentifier = true;
var isEscaped = false;
var token = '';
for (var i = 0; i < referenceString.length; ++i) {
var c = referenceString.charAt(i);
if (isEscaped) {
token += c;
isEscaped = false;
} else if (c === '\\') {
isEscaped = true;
} else if (inIdentifier && c === '#') {
identifier = token;
inIdentifier = false;
token = '';
} else if (!inIdentifier && c === '.') {
values.push(token);
token = '';
} else {
token += c;
}
}
values.push(token);There was a problem hiding this comment.
I had a huge mental block when writing this code, so thanks for this. I agree this is much cleaner.
| * Creates a new instance given the dynamic object collection that will | ||
| * be used to resolve it and a string indicating the target object id and property. | ||
| * The format of the string is "objectId#foo.bar", where # separates the id from | ||
| * property path and . separates sub-properties. |
There was a problem hiding this comment.
Should probably document how to escape # and . and \ in the documentation here too.
|
|
||
| //>>includeStart('debug', pragmas.debug); | ||
| if (parts.length !== 2) { | ||
| throw new DeveloperError('referenceString must contain a single . delineating the target object ID and property name.'); | ||
| if (identifier === '' || values.length === 0) { |
There was a problem hiding this comment.
I think these checks would be appropriate to move to the constructor instead, since they apply just as well there.
|
All good stuff, I added a couple of extra specs to cover the extra checks being done in the constructor as well. Ready to go. |
Conflicts: CHANGES.md Source/DynamicScene/CzmlDataSource.js
This refactors the existing
ReferencePropertyimplementation to fix a plethora of issues it had and make it more useful overall. It also extends CZML capabilities for working with references, as discussed in AnalyticalGraphicsInc/czml-writer#66CzmlDataSource.js is a little unwieldy, but I'm hoping to do a major clean up of the code for 1.0. Hopefully it's not too hard to review the changes currently needed for the above.