fitztrev / lichess-tauri

Desktop app for Lichess external engine
GNU Affero General Public License v3.0
75 stars 10 forks source link

Reuse engine process #17

Open fitztrev opened 1 year ago

fitztrev commented 1 year ago

Don't start a new engine process on each request

Disservin commented 11 months ago

I think I've already messaged you about my engine process fix on discord, but i'll mention it here also ;)

The way I implemented it in my own is like this:

The communication is basically implemented in js (see https://github.com/Disservin/fast/blob/master/src/ts/ChessProcess.ts), by using the tauri shell and command api.
This js class object is simply persisted in the appropiate place.
There is a small problem though, tauri does not allow the execution of unknown shell commands, all commands have to be predefined in the config. This obviously wont work since we dont know the engine name at build time, thus I had to modify tauri itself and disable this security feature see: https://github.com/Disservin/tauri/commit/00d961e46d9ce429f3bd62a69f1c9508e5d821fa it's just a small workaround. There exists an issue in tauri already https://github.com/tauri-apps/tauri/issues/5910 and an open but currently stale pr https://github.com/tauri-apps/tauri/pull/7165.

I hope this helps ;)

joshka commented 11 months ago

Hey, I was thinking about implementing this if you're interested, by keeping the engine running in the background properly in rust. I wasn't sure what was happening with the engine settings PR which shakes a couple of things in that area up. Would you mind a PR for this, or prefer to get the other PR merged first?

fitztrev commented 11 months ago

@joshka That would be great! Ah yeah, that branch can be disregarded. This is a higher priority and I'll happily deal with any conflicts if this lands first.

Disservin commented 11 months ago

Tbf disabling that check and implementing it js side is probably way simpler and easier to do than to reimplement everything in rust :D

joshka commented 11 months ago

I did see your rationale on this. I have a general view that hacking around security controls is something that leads to suboptimal results when things change. So I think it's still worth doing in Rust.

Disservin commented 11 months ago

The security controls in this case anyway don’t offer much value… but yeah it’s a nasty workaround I admit and once the mentioned pr gets merged everything will be simpler anyway

franciscoBSalgueiro commented 11 months ago

I've already implemented this in my GUI:

The relevant code is here https://github.com/franciscoBSalgueiro/en-croissant/blob/master/src-tauri/src/chess.rs

The way it works is basically this: