Implement NonSerialized template for Json #5

Merged
merged 2 commits into from Mar 31, 2012

Conversation

Projects
None yet
2 participants
@ghost

ghost commented Mar 26, 2012

Okay I finally got around to doing this. Now instead of using hashes I just generate enum fields. The toArray function was taken from the Orange library (Boost), maybe there should be a note of that.

@CyberShadow

This comment has been minimized.

Show comment
Hide comment
@CyberShadow

CyberShadow Mar 26, 2012

Owner

Why serialize .init instead of just skipping the field?

Owner

CyberShadow commented Mar 26, 2012

Why serialize .init instead of just skipping the field?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Mar 26, 2012

I've assumed it would break compatibility when changing which fields are to be serialized, but that doesn't happen. Some other serialization libraries depend on the exact order and length of the fields, but apparently not ae.json. So .init isn't necessary.

ghost commented Mar 26, 2012

I've assumed it would break compatibility when changing which fields are to be serialized, but that doesn't happen. Some other serialization libraries depend on the exact order and length of the fields, but apparently not ae.json. So .init isn't necessary.

CyberShadow added a commit that referenced this pull request Mar 31, 2012

Merge pull request #5 from AndrejMitrovic/master
Implement NonSerialized template for Json

@CyberShadow CyberShadow merged commit 040a8c1 into CyberShadow:master Mar 31, 2012

@CyberShadow

This comment has been minimized.

Show comment
Hide comment
@CyberShadow

CyberShadow Mar 31, 2012

Owner

Why the heck does toArray need static? I assume that's a DMD bug workaround?

Owner

CyberShadow commented Mar 31, 2012

Why the heck does toArray need static? I assume that's a DMD bug workaround?

@CyberShadow

This comment has been minimized.

Show comment
Hide comment
@CyberShadow

CyberShadow Mar 31, 2012

Owner

Refactored things a bit in b84334b, let me know if anything broke.

Owner

CyberShadow commented Mar 31, 2012

Refactored things a bit in b84334b, let me know if anything broke.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Mar 31, 2012

Why the heck does toArray need static? I assume that's a DMD bug workaround?

Yup. When I took it from Orange I tried removing static but it caused a compilation error.

ghost commented Mar 31, 2012

Why the heck does toArray need static? I assume that's a DMD bug workaround?

Yup. When I took it from Orange I tried removing static but it caused a compilation error.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Apr 18, 2012

Hey Vlad, an unrelated question: Is there a specific reason why the read template requires hashes to have a string key type? Is this forced by the json spec? Otherwise it's fairly easy to work around it, simply by removing the is(typeof(T.init.keys[0])==string)) check and making readAA use the read template instead of calling readString when reading the key of the hash.

Sorry to post this here but I can't open an issue here and notifications are gone from github.

ghost commented Apr 18, 2012

Hey Vlad, an unrelated question: Is there a specific reason why the read template requires hashes to have a string key type? Is this forced by the json spec? Otherwise it's fairly easy to work around it, simply by removing the is(typeof(T.init.keys[0])==string)) check and making readAA use the read template instead of calling readString when reading the key of the hash.

Sorry to post this here but I can't open an issue here and notifications are gone from github.

@CyberShadow

This comment has been minimized.

Show comment
Hide comment
@CyberShadow

CyberShadow Apr 18, 2012

Owner

Yeah, JSON is a subset of JavaScript, and JSON hashes are JavaScript object literals - and the field names have to be strings. The change to allow keys of any type would be interesting and map well to AA arrays, but it would no longer be JSON.

P.S. I'm on IRC, you can just ask me there ;)

Owner

CyberShadow commented Apr 18, 2012

Yeah, JSON is a subset of JavaScript, and JSON hashes are JavaScript object literals - and the field names have to be strings. The change to allow keys of any type would be interesting and map well to AA arrays, but it would no longer be JSON.

P.S. I'm on IRC, you can just ask me there ;)

FeepingCreature pushed a commit to FeepingCreature/ae that referenced this pull request Jan 12, 2018

Merge pull request #8 from MWumpusZ/master
Suggested fix for #5 (Inheritance)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment