Object Buffer Wrapper Errors

[Enthalpy]

Initial investigation of object buffer wrappers has revealed inconsistent behavior:

foo = getBufferWrapperFor(["a", "b", "c", "d", "e"])
foo.splice(2, 0, "f", "g", "h")
> foo should "=" ["a", "b", "f", "g", "h", "c", "d", "e"]
foo.splice(6, 1)
> foo should "=" ["a", "b", "f", "g", "h", "c", "e"]
// The shift number on the "c" block SHOULD be 3.
foo.__diff.blockEdits[1]
> {'start_index': 6, 'end_index': 7, 'shift': -1}
// We confirmed a bug... But wait, shouldn't c and e live in separate blocks, anyway?
foo.__diff.blockEdits
> Array[Object, Object]
// Uh-oh. This should be an array of length 3. Let's see what our data looks like, anyway.
foo.length
> 7
foo[0]
> "f"
foo[1]
> "g"
foo[2]
> "h"
foo[3]
> "g"
foo[4]
> "h"
foo[5]
> Inconsistent diff
foo[6]
> Inconsistent diff

These bugs must be fixed, and all object buffer wrapper code must be tested to detect any additional errors. In particular, the arrays need to be tested, due to having much more complex code.

It can be inferred from the lack of reports on this that the current implementation is not causing problems with the saving system, but the objects_buffer_wrappers is planned to be used in many other AAO systems, including the rowmap rewrite #100 and all code amending row templates, such as #94. Failure to catch errors can cause unpredictable errors in other functions, and may allow for currently unknown (and very hard to debug!) errors in the save system, especially where reconciliation with trial edits is concerned.

[Arnaud Vié]

Yes, the reason no bug has been reported yet is because the wrapper is currently only enabled in the player for the game saving mechanism, which does not perform any array operation.

And that's precisely the case because I'm not confident enough in the current "splice" implementation in the wrapper. Once we have a reliable splice method, most of the work will be done : all other methods (push, shift, unshift, etc.) are edge cases of splice.

One thing : a good thing you can do in order to automatically check consistency of the diff is to instantiate a second buffer wrapper on the same original object using the diff of the first wrapper. If the resulting array is the same in the first and second wrappers, it means that the operations you performed in the first wrapper (before taking the diff that was used to initialise the second one) were consistently written in the diff.

[Arnaud Vié]

Oh, by the way, I didn't see you had linked to #94. Issue #94 does not depend on BufferWrapper as far as I can tell. To add new data fields to the trial file format, we only need the ExtensionWrapper : I splitted it away from the BufferWrapper precisely for this reason. The ExtensionWrapper is much simpler and practically ready for use already. It might be missing some array methods, but their implementation is straightforward : apply the original array method and create new accessors on the wrapper if needed.

The wrapper which causes problems during array operations is only the BufferWrapper :-)

[Enthalpy]

Luckily for all of us, I didn't know that when I started testing. Try out the following code:

foo = getExtensionWrapperFor({"bar": "baz"})
foo["bar"]

Surprise, it fails! The extension wrapper only defines properties that are in the model, and ones that were in the object but not the model get skipped over. (I'm assuming you didn't intend for the model to be a complete model over all possible properties.)

That would definitely have caused problems. It's a simple enough fix, and I'm hoping to have this up today.

[Arnaud Vié]

I assumed the model to enumerate all possible properties yes, that's what an object model is supposed to be ;-)

When defined correctly, the same object model should be actually used to create new objects in createDataRow, and to extend existing trials with new properties using the wrapper.

In short, please don't fix that, it's expected behaviour :-P

[Enthalpy]

Fixed by Pull Request #28.