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

Struct.create broken on Chrome Canary #2

Open
inolen opened this issue Aug 18, 2012 · 2 comments
Open

Struct.create broken on Chrome Canary #2

inolen opened this issue Aug 18, 2012 · 2 comments

Comments

@inolen
Copy link

inolen commented Aug 18, 2012

Heya,

First off, thank you so much for all your great WebGL blog posts and projects, they're absolutely top-notch work all around.

Tonight I was toying around with bringing js-struct into your webgl-quake3 project but noticed Chrome Canary wasn't playing well with Struct.create. Basically, as you're well aware Struct.create will end up creating read functions for each structure and its properties that may look something like:

(function(o) { var st = Object.create(Struct.struct_id_0);
st.ident = (function(o) {
   var str = "";
   for(var j = 0; j < 4; ++j) {
       var char = v.getUint8(o+j, true);
       if(char === 0) { break; }
       str += String.fromCharCode(char);
   }
   return str;
})(o);

And, while in the middle of generating this function it'll call Object.defineProperty to define the ident property on Struct.struct_id_0 (here).

This seems to work fine in the current Chrome Stable, but in Chrome Canary the attempt to set ident on the new object will fail if it was defined on the prototype through Object.defineProperty (regardless of the writeable and configurable flags).

To perhaps illustrate this better, the following will print new value on Chrome Stable but old value on Chrome Canary:

var struct_proto = Object.create(Object.prototype);
Object.defineProperty(struct_proto, 'test', { value: 'original value', enumerable: true, configurable: true, writeable: true });

var struct_new = Object.create(struct_proto);
struct_new.test = 'new value';

console.log(struct_new.test);

At the end of the day, after reading about I'm not sure what the correct behavior here should be, but it doesn't seem the Object.defineProperty is necessary in the first place.

Any thoughts?

@inolen
Copy link
Author

inolen commented Aug 18, 2012

Doh, just realized the Object.defineProperty is necessary for default values, and realized the problem was due to writable being spelled writeable.

Furthermore, I realized this issue is fixed in this repo, and I should have checked before continuing to use the one from webgl-source :)

@inolen
Copy link
Author

inolen commented Aug 22, 2012

As a follow up, there still appears to be one issue.

When you pass the object of extra properties to createStruct (e.g. dtexdata_t in webgl-source) you're not explicitly passing enumerable, writable and configurable properties but using them as such later on (which works fine in Chrome Stable, but not in Canary).

Perhaps those flags should be defaulted to true in createStruct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant