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

Added the laser hockey env #52

Merged
merged 9 commits into from
Jan 20, 2024
Merged

Added the laser hockey env #52

merged 9 commits into from
Jan 20, 2024

Conversation

PacoSchatz
Copy link
Collaborator

@PacoSchatz PacoSchatz commented Jan 9, 2024

I had a first go at adding the laser hockey env.

As far as I can tell it works. The agents each send a random action and play against each other.
I changed the _observation in IGame to also receive a player-index, as the observation is inverted for the second player.
While I was at it I also changed the Game class to know its game_ID and protocol to properly transmit this ID to the clients.

@Cari1111 and I both ran into the following error while installing the laser-hockey-env.:
'Failed building wheel for box2d-py'
for me, this was fixed by creating a new venv, installing swig with pip install swig, and then pip install . to install the laser-hockey-env.

Things that can still be improved/are worked on :

  • I couldn't find how to retrieve the score of the game from obs or info, so the _player_stats function doesn't return anything useful yet.
    --> moved to Return score in laser-hockey game class #56.

  • I didn't change any of the parameters when creating the env. These may be changed to best fit our use case.

  • Currently the errors for the laser-hockey package about missing library stubs or py.typed marker are suppressed in the pyproject.toml . Further information can be found here.
    Maybe we can have a go at automatic stub generation for the laser-hockey package

  • Add swig in the pyproject.toml file, as it might be required for the laser-hockey-env

@PacoSchatz PacoSchatz linked an issue Jan 10, 2024 that may be closed by this pull request
@PacoSchatz PacoSchatz marked this pull request as ready for review January 15, 2024 13:44
@Cari1111 Cari1111 self-requested a review January 18, 2024 18:01
Copy link
Collaborator

@Cari1111 Cari1111 left a comment

Choose a reason for hiding this comment

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

Thanks for your implementation. Comments on the changes:

  1. Laser hockey
    I tested the implementation locally and it works realy good. I would only change the name of the class name of the agent class.

  2. game_id
    The game ID is now send to the client from the game class instead of a permanent dummy value. This change was important and was implemented well.

  3. General cleannup
    The updated and corrected comments are very good.

I have two little changes that I requested below.

@PacoSchatz PacoSchatz merged commit ae684fc into main Jan 20, 2024
4 checks passed
@PacoSchatz PacoSchatz deleted the paco/laser-hockey branch January 20, 2024 11:28
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.

add Hockey gym env
2 participants