symposion / roll20-shaped-scripts

Support script for 5e Shaped Character Sheet on Roll20.net
http://roll20.net/
7 stars 5 forks source link

JSON Format #458

Closed mlenser closed 7 years ago

mlenser commented 7 years ago

I think we should change the JSON format of spells to be more in line with the display. So:

"components": "V, S, M" instead of what we have now. Also for duration and concentration.

I think everything else can stay as is.

symposion commented 7 years ago

Is there a particular reason why this matters other than satisfying a desire for neatness? Every time we change the format it's disruptive for all the users of the script + means we get more questions about locations of files etc. Unless there's a compelling reason to change it I think this should wait until we have other, more significant changes to make to the format that have a functional purpose

mlenser commented 7 years ago

New spells being added to UA.

The format in this regard is really out of sync and annoying to fix.

symposion commented 7 years ago

Following on from our, ahem, discussion the other day + the fact that one of our more enterprising users has gone ahead and built the thing I was thinking of without any prompting (!) I think that maybe we should reconsider the JSON schema option. I experimented with it before but the tools weren't really good enough. Perhaps https://github.com/mozilla-services/react-jsonschema-form or http://schemaform.io/ might do the job better.

If they do, we could publish a JSON schema for the input format. I'd be happy to look at unpicking the connection between the monster JSON and the statblock parser and replacing it with JSON schema validation. That would remove the constraint for the JSON to be limited to being something that maps to a non-ambiguous grammar for statblock parsing, albeit at the cost of having to maintain two separate format definitions. Perhaps it's time to consider ditching the statblock parser entirely and you could just incorporate into a separate tab of the sheet? I'm not sure I see the value of having it in the script any more, particularly as you have a lot of the parsing code already for SRD content.

mlenser commented 7 years ago

Closing this issue. I've converted the suggested format to the current format at compile time of the data.