Introduce "props" as way of specifying arbitrary data

lazerwalker / twison

A Twine 2 story format that provides JSON export

MIT License

148 stars 34 forks source link

Introduce "props" as way of specifying arbitrary data #18

Closed 1000nettles closed 4 years ago

1000nettles commented 4 years ago

Hey there!

I've been working on a project that needed this capability, and I noticed that you had filed an issue to add this functionality down-the-line. Decided I might as well open a PR so others could use this, too.

Apologies if something is off, this is my first PR on GitHub. Happy to adjust whatever you think needs changing. Thanks!

Resolves issue #3.

Changes:

Previously, there was no way to specify arbitrary data on Twine2 passages. We now have added the ability to specify, from the Twine2 interface, props which will be contained in the exported JSON. For example: "{{foo}}bar{{/foo}}" will add "props": {"foo": "bar"} to the exported JSON document. Multiple props can be specified.
Add JSDoc blocks to functions in src/twison.js for developers to get up-to-speed quicker.
Adjust tests to include testing for props.
Adjust README.md explaining props further.

1000nettles commented 4 years ago

Hey @lazerwalker! Any thoughts on the above? I'd be happy to change anything or even split it into multiple PRs if it's too large. Thank you. :)

lazerwalker commented 4 years ago

Hey @1000nettles! Sorry for the radio silence.

I like the idea of this! Gonna need a few days to find time to look over the implementation, but excited to get this merged in 👍

lazerwalker commented 4 years ago

Sorry again for the delay.

This generally looks great! The only question/comment I have is around nesting.

Say a passage contains the following props:

{{foo}}
    {{bar}}
        baz
    {{/bar}}
{{/foo}}

I'd ideally expect this to translate into {foo: { bar: "baz" }}. Reading through the code and futzing around a bit in the console, I believe the current implementation would result in {foo: "{{bar}}baz{{/bar}}"}

Do you think this could be tweaked to allow arbitrarily nested props?

Nightspeller commented 4 years ago

That would be an awesome feature to have, really waiting for this one! @1000nettles - until it merged, do you have it hosted somewhere so I could use your version with http://twinery.org ? Thanks!

1000nettles commented 4 years ago

@lazerwalker thank you for taking a look! That's a great idea - I've pushed up a commit to address this.

@Nightspeller I don't currently host any Twine format.js files, but potentially this issue will be resolved soon anyhow.

lazerwalker commented 4 years ago

Looks good @1000nettles, thanks so much!

lazerwalker commented 4 years ago

@Nightspeller This should be live on the hosted version as well.

Nightspeller commented 4 years ago

@lazerwalker nice! But to me it seems that the link in the description is not working: https://lazerwalker.com/twison/format.js returns 404

lazerwalker commented 4 years ago

Argh, thanks for noticing/mentioning. I was messing around with deployment settings today to make pushing out a new version more automatic, looks like something got screwed up. Will let you know when it's up and running properly again.

lazerwalker commented 4 years ago

@Nightspeller This should be live again, can you let me know if it's working for you? Sorry about that!

Nightspeller commented 4 years ago

I can open the file, I can add the format to twinary online, but when I try to "Play" the story (expecting to get JSON), I am getting an error in console (the page itself is completely blank): Windows 10, Firefox

lazerwalker commented 4 years ago

Argh, when making an unrelated documentation change, my text editor auto-formatted some code in a way that broke my build script. This is what happens when you tweak a project after not touching it for a few years 😂

A new version has been pushed, and I tested it myself on a new / functionally empty project on twinery.org.

Nightspeller commented 4 years ago

It is working! Thanks for your work!