Skip to content

DRAFT: Use console_engine instead of crossterm#3

Draft
VincentFoulon80 wants to merge 1 commit intojawline:masterfrom
VincentFoulon80:vfoulon80/console_engine
Draft

DRAFT: Use console_engine instead of crossterm#3
VincentFoulon80 wants to merge 1 commit intojawline:masterfrom
VincentFoulon80:vfoulon80/console_engine

Conversation

@VincentFoulon80
Copy link

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,

@jawline
Copy link
Owner

jawline commented Oct 10, 2021 via email

@VincentFoulon80
Copy link
Author

VincentFoulon80 commented Oct 10, 2021

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 ...

@jawline
Copy link
Owner

jawline commented Oct 28, 2021

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.

@VincentFoulon80
Copy link
Author

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
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments