erkyrath / lectrote

The IF interpreter in an Electron shell
Other
248 stars 28 forks source link

Switch from Parchment to ZVM #77

Closed curiousdannii closed 7 years ago

curiousdannii commented 7 years ago

I've got a basic port of ZVM to Glk working now (excluding save/restore and half the IO functions) in the master branch.

I'd recommend switching from embedding Parchment to using ZVM with glkote. This would remove a lot of the inconsistencies, and make the Lectrote options apply to both VMs. I don't know if you'd want to add ZVM to Quixe itself, or only add it to Lectrote. Up to you!

ZVM should be built for the browser with browserify: (other bundling systems would work too, but browserify is super simple when you only need to bundle one file)

browserify src/ifvms.js/src/zvm.js --standalone ZVM > lib/zvm.debug.js

Loading ZVM is very similar to Quixe. An instance of ZVM will need to be created (because it's not a singleton like Quixe), and an extra reference to glk added to the options object. (I set window.engine just for testing, it is not needed.)

I'll keep working on filling in the functionality gaps, but it seems stable enough to start integrating now.

erkyrath commented 7 years ago

Thanks. I will pull this in for the next major release. (I'm hesitant to rock the boat during IFComp.)

curiousdannii commented 7 years ago

This now seems to be quite stable. The IO stuff is essentially done. I'll need help with the save/restore code (https://github.com/erkyrath/glkote/issues/13), but if you wanted to start integrating it I think it's good to go.

FYI, I'm planning on publishing this via npm following semver, so it would be possible to add it as a dependency rather than adding the files directly (or adding it as a submodule) and then you wouldn't need to worry about pulling in updates, if that's a workflow you wanted. inkjs is also on npm btw.

erkyrath commented 7 years ago

Ok, I'll look at it sometime. Er, in the next couple of months.

erkyrath commented 7 years ago

Reminder to myself that IFVMS is ready to go. https://github.com/curiousdannii/ifvms.js/issues/18

curiousdannii commented 7 years ago

Just note that Version 3 is still super broken. But hopefully by the time you're ready to incorporate this it won't be :). Edit: It's fixed! 😄

One thing I'd like your opinion on is the version 3 upper window (distinct from the status window.) I open it at the beginning but set it to 0 height, but this can mean that Quixe has a lot of padding, and in the default orange style it looks pretty bad. I haven't even been able to find a version of Seastalker (or any Inform games) which use the upper window. Should I instead hold off opening it until the VM tries to set its height?

Oh, and we'll need to decide what to do with gli_fileref_create_by_prompt_callback before you can switch to ZVM.

erkyrath commented 7 years ago

Yes, it's better to close the grid window when the Z-machine considers it to have zero height. (Or not open it until the VM sets it to a nonzero height.)

I will look at the create_by_prompt situation when I've got the integration going. I suspect I'll wind up creating a "dispatch layer" shim.

erkyrath commented 7 years ago

Notes for @curiousdannii ...

I started trying to integrate this, in the https://github.com/erkyrath/lectrote/tree/ifvms branch. I am using the current master of ifvms.js, generated with browserify as you describe.

I hit one minor bug (https://github.com/curiousdannii/ifvms.js/issues/26). With that patched, I now get an exception: "RangeError: ZVM start: Offset is outside the bounds of the DataView"

EDIT: sorry, I'm just misunderstanding the situation here. I will try passing in a typed array.

EDIT-2: okay! That is working with Advent.z5. I have not tested other files yet.

Right now it's set up with a forked version of gi_load.js. I will get those changes integrated back.

curiousdannii commented 7 years ago

I will look at the create_by_prompt situation when I've got the integration going. I suspect I'll wind up creating a "dispatch layer" shim.

I noticed that your current integration of Parchment just talks to Dialog directly. That could be another option, although it would have to manually flush to GlkOte, and it would also mean that ZVM couldn't be used in a remote setup.

curiousdannii commented 7 years ago

ZVM needs zvm.css for the output styles to work properly. (They won't be terrible as is, but they are not ideal.) Sorry if I hadn't mentioned that before.

erkyrath commented 7 years ago

Ok.

Is it really worth setting hard black/white colors on the styles at the bottom? I'd rather stay consistent with the status-line colors in Quixe. Also, I just added low-contrast themes to the app.

curiousdannii commented 7 years ago

No, by all means change them to theme appropriate colours (same with the monospaced font). I'm not sure if there are any Z-code games which need reverse styles to be distinguished in the status window - if you don't know of any then you can probably just leave it how it is.

But games like Jigsaw will need reverse style in the main window. So you could change the selectors to:

.BufferWindow .Style_header, .BufferWindow .Style_note, .BufferWindow .Style_user1, .BufferWindow .Style_user2

It will get more tricky when we get to adding colour, because the reverse style will need to be based on current colours. But lets cross that bridge when we get there.

curiousdannii commented 7 years ago

v1.0.0 has just been published. Would you like me to make a pull request?

v1.0.1 will hopefully shortly follow to handle the quote boxes.

erkyrath commented 7 years ago

I got it. Thanks.

curiousdannii commented 7 years ago

How big a priority do you think version 4 support is? I hadn't intended to add support for it, but seeing the games mentioned in the readme is making me reconsider.

erkyrath commented 7 years ago

I didn't mean to be passive-aggressive about it! But I think v4 is worthwhile. Historically, every interpreter that implements 5 also implements 4, so it should be a reasonable gain for a small amount of work.

Plus, Trinity is how you test your quote-box support outside the Inform 6 library implementation. :)