Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accept engines that require an interpreter #1023

Merged
merged 18 commits into from
Oct 1, 2024

Conversation

AttackingOrDefending
Copy link
Member

Type of pull request:

  • Bug fix
  • Feature
  • Other

Description:

Allow users to provide arguements before and after the engine name (e.g. when a user is running a jar file as a chess engine).
engine_options could be removed as after_name_parameters does the same thing but for a more general use case. On the other hand, engine_options is easier to use and some people may depend on it, so if we remove it, we should probably deprecate it first.

Related Issues:

closes #1022

Checklist:

  • I have read and followed the contribution guidelines.
  • I have added necessary documentation (if applicable).
  • The changes pass all existing tests.

Screenshots/logs (if applicable):

@MarkZH
Copy link
Collaborator

MarkZH commented Sep 24, 2024

I don't think before_name_parameters and after_name_parameters are good names. The problem that this PR should solve is when a user's chess engine requires an interpreter to run. So, I think two engine subsections are needed:

engine:
  interpreter: java
  interpreter_options:
    - "-jar"

The other engine subsections (dir, name, engine_options) remain the same. The command within engine_wrapper.create_engine() is then built like this:

commands = []
if cfg.interpreter:
  commands.append(cfg.interpreter)
  commands.extend(cfg.interpreter_options or [])
commands.append(engine_path)
if cfg.engine_options:
  for k, v in cfg.engine_options.items():
    commands.append(f"--{k}={v}" if v is not None else f"--{k}")

I don't think there are any other cases besides interpreters for things going before the file containing the engine.

@MarkZH
Copy link
Collaborator

MarkZH commented Sep 27, 2024

Good. Now all that's needed is to document this in the wiki.

I would also comment out interpreter_options: in config.yml.default.

@AttackingOrDefending
Copy link
Member Author

For some reason the test fails, while I can run buggy_engine.py normally as a bot.

@AttackingOrDefending AttackingOrDefending changed the title Accept parameters before and after the engine name Accept engines that require an interpreter Sep 28, 2024
test_bot/test_bot.py Outdated Show resolved Hide resolved
@MarkZH MarkZH merged commit f1b31b8 into lichess-bot-devs:master Oct 1, 2024
15 checks passed
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.

Enable Running engines which are runnable jar files
2 participants