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

Build for Arch and an info property for the Observation protobuf class #79

Merged
merged 9 commits into from
Aug 25, 2023

Conversation

JJJHolscher
Copy link

@JJJHolscher JJJHolscher commented Aug 23, 2023

The Observation protobuf class now has an info property which is a string.
It can be set both in client and server mods. (though as per the design, the client side will be ignored if the sync_port is set).
I mostly copied rkla's code for this.

Usage

git clone https://github.com/JJJHolscher/minetest
cd minetest
python -m venv .venv
. .venv/bin/activate
make linux_deps
make python_build_deps
make repos
make sdl2
make proto
make zmqpp
make minetest
pip install $minetester_wheel --force-reinstall
make demo

Now any arch or debian user should see the demo play and see the rewards, terminates and info (which is the player position in this case) being printed until terminates randomly (intentionally randomly) stops after a few steps.

Changes

  • minetest_env.py and test_loop.py now log and/or print the info property of the Observation protobuf class.
  • the info server mod was added to show that it works, it is also added to test_loop.py
  • the result mods now specify the (empty) info variable, this is necessary else the server errors.
  • c++ version is set to 17 in CMakelist.txt, the previous version 14 was incorrect
  • vim complains if charset=utf8 instead of utf-8 in .editorconfig.
  • cpp and header files are changed only for the sake of extracting the info variable and passing it to python.
  • Added arch dependencies.
  • Added ctags and debug to the Makefile.

Todos

  • src/client/recorder.{h,cpp} uses tabs instead of spaces for indentation, contrary to the rest of the code and .editorconfig.
  • make minetester (still) errors when repairing the wheel. I used an unrepaired version, for testing whether my PR works, as described in Usage.

@JJJHolscher
Copy link
Author

Err, the README.md should be updated to reflect that
make deb_deps is now make linux_deps

@JJJHolscher JJJHolscher reopened this Aug 23, 2023
Copy link

@AI-WAIFU AI-WAIFU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@AI-WAIFU AI-WAIFU merged commit 315cb46 into EleutherAI:develop Aug 25, 2023
1 check 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.

None yet

2 participants