Mimic icon indicating copy to clipboard operation
Mimic copied to clipboard

Published 20 hours ago • verified publisher icon jawline

DRAFT: Use console_engine instead of crossterm

Open VincentFoulon80 opened this issue 3 years ago • 5 comments

Hi !

I noticed one of your dependencies included the crate console_engine (I'm the creator of it, by the way). Out of curiosity, and for fun, I wanted to try out converting the actual terminal application to use this crate instead of crossterm (which my crate is based of).

I just pushed this PR as a draft so you can take a quick look at it. There's sadly a dirty hack somewhere to add compatibility with the excellent drawille crate. Aside from that, it seems to work pretty well. I'm curious if the recent "flashing" issue is still showing with this change.
One thing that console_engine does is printing only differences between frames, to gain performances since drawing on some terminals can be really slow.

Feel free to close this PR if you're not interested. 😄
I used this project to improve, fix and release the event feature of my crate, so it's not totally a loss 😉

By the way, while working on this project, I noticed a bunch of warnings from rust-analyzer. If you don't mind, I'll soon enough create a new PR to fix all of these. Anyway, Mimic is a really nice project you got, good job!

Kind regards,

VincentFoulon80 avatar Oct 10 '21 09:10 VincentFoulon80

This looks awesome! I was originally using console engine, and I've used it in a project for a previous emulator (https://github.com/jawline/CHIP-9). I would be glad to use it here too.

I think adjustments we made to the clear behaviour have already largely solved the flashing but there were still some issues with the window size being wrong, perhaps console engine will solve these.

Are there any other caveats to using console engine here, particularly, how does it handle inputs? I've been have a bunch of trouble getting input dropoffs balanced enough to be able to play input heavy games. I think there's also a bug in my current implementation I didn't get around to fixing that would prevent two inputs being registered concurrently which was annoying.

Thanks for a great library!

On Sun, 10 Oct 2021, 17:54 Vincent Foulon, @.***> wrote:

Hi !

I noticed one of your dependencies included the crate console_engine (I'm the creator of it, by the way). Out of curiosity, and for fun, I wanted to try out converting the actual terminal application to use this crate instead of crossterm (which my crate is based of).

I just pushed this PR as a draft so you can take a quick look at it. There's sadly a dirty hack somewhere to add compatibility with the excellent drawille crate. Aside from that, it seems to work pretty well. I'm curious if the recent "flashing" issue is still showing with this change. One thing that console_engine does is printing only differences between frames, to gain performances since drawing on some terminals can be really slow.

Feel free to close this PR if you're not interested. 😄 I used this project to improve, fix and release the event feature of my crate, so it's not totally a loss 😉

By the way, while working on this project, I noticed a bunch of warnings from rust-analyzer. If you don't mind, I'll soon enough create a new PR to fix all of these. Anyway, Mimic is a really nice project you got, good job!

Kind regards,

You can view, comment on, or merge this pull request online at:

https://github.com/jawline/Mimic/pull/3 Commit Summary

File Changes

Patch Links:

  • https://github.com/jawline/Mimic/pull/3.patch
  • https://github.com/jawline/Mimic/pull/3.diff

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jawline/Mimic/pull/3, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABYJPZTRJ2NG5VDSGMAG6DUGFPDRANCNFSM5FWJDZAA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

jawline avatar Oct 10 '21 10:10 jawline

This looks awesome! I was originally using console engine, and I've used it in a project for a previous emulator (https://github.com/jawline/CHIP-9). I would be glad to use it here too.

Oh it was you! I starred this project a while ago as well, you got pretty far now!

I think adjustments we made to the clear behaviour have already largely solved the flashing but there were still some issues with the window size being wrong, perhaps console engine will solve these.

I'm not sure for the flashing but I'm curious to know if someone could check that. None of the terminals I use seem to have this behavior (Window's CMD, conemu, VSCode's console and WSL, 2 of which didn't support the braille characters)

Are there any other caveats to using console engine here, particularly, how does it handle inputs? I've been have a bunch of trouble getting input dropoffs balanced enough to be able to play input heavy games. I think there's also a bug in my current implementation I didn't get around to fixing that would prevent two inputs being registered concurrently which was annoying.

That's something I'd want to know too to be honest. It's rare I get any feedback on my crate, I'd be glad to discuss and work out these issues if you got some to share. As for how it handle inputs now, (thanks to your project) I just released an event system through the "event" feature so we can poll events alike how we poll events with crossterm.

The regular method to get keys through wait_frame was sensible to the amount of time you take per frame. That means that if you takes 90% of a frame time to work your logic, the engine will only poll keys on the remaining 10% of time. If you combine this with a high requested framerate, some quick key presses can be lost in between. In short : this first method is fine if you don't make heavy calculations between frames. For simple games it's fine, for an emulator... it's kinda "meh"

That's why there's now an "event" feature: I believe this way we have better handling of key presses. It's only asking confirmation though.

Thanks for a great library!

Thank you too for using it ! It was first a learning project for the Rust language that got some efforts to make it as serious as possible and now I'm happy to see it being used in various projects 😄

EDIT: Having played a bit of tetris, I noticed (with crossterm as well as with console_engine) that sometimes we got lag spike, I don't know if that's linked to the input or if it's just some frames that take much longer to calculate for some reason ...

VincentFoulon80 avatar Oct 10 '21 11:10 VincentFoulon80

Hey!, sorry it's taken so long to get back around to this, I've not had much time for personal things lately.

I've had a look through your change and run it locally and I think everything is good. I know you mentioned some lag spikes (with crossterm as well) but I think if you're also happy it's best to merge this change now and just address any further issues later.

If you are also happy feel free to mark it as in review rather than draft and I'll happily merge it.

Thanks again for your submission, I'm stoked you noticed the project.

As an aside, while it's fine to be a bit hacky with the drawille integration I was wondering if you could think of any better way to support it than an ascii character substitution. I guess shy of integrating it into your library that's probably the best we'll get though and I think it's worth it for the much nicer input handler.

jawline avatar Oct 28 '21 15:10 jawline

Hey!, sorry it's taken so long to get back around to this, I've not had much time for personal things lately.

No problems 😄 there's nothing to rush about

I've had a look through your change and run it locally and I think everything is good. I know you mentioned some lag spikes (with crossterm as well) but I think if you're also happy it's best to merge this change now and just address any further issues later.

If you are also happy feel free to mark it as in review rather than draft and I'll happily merge it.

Thanks again for your submission, I'm stoked you noticed the project.

As an aside, while it's fine to be a bit hacky with the drawille integration I was wondering if you could think of any better way to support it than an ascii character substitution. I guess shy of integrating it into your library that's probably the best we'll get though and I think it's worth it for the much nicer input handler.

To be honest, I'd like to address the drawille issue before considering merging. I've already looked at a way to create a compatibility between drawille and console_engine, first by trying to extend drawille within console_engine (without success), and then walking through the internet one of the recommended way is to propose a PR to the drawille crate directly.

I'm a bit disappointed about the fact that we need to change a remote crate in order to add features to handle compatibility.

Another approach I'd like to give a try is to wrap a drawille Canvas inside a feature of console_engine where I'm just daisy-chaining function calls to the Canvas through a Screen structure. This may work as well

What do you think about these ideas ? Maybe you have something better in mind ?

VincentFoulon80 avatar Oct 28 '21 16:10 VincentFoulon80

Referencing two pull requests:

VincentFoulon80 avatar Oct 31 '21 11:10 VincentFoulon80