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

Miner incorrectly wins game if the agent spawns on the exit due to incorrect diamond count initialization #67

Open
jordan-schneider opened this issue Aug 3, 2021 · 4 comments

Comments

@jordan-schneider
Copy link

I don't know if this counts as something that won't be fixed for backwards compatability, but if the agent starts an episode on the exit (as in seed 1926574277), then

  1. miner.cpp initializes diamonds_remaining as 0
  2. Miner::game_step calls BasicAbstractGame::game_step() first before updating diamonds_remaining
  3. BasicAbstractGame::game_step() calls for collision handling
  4. Miner::handle_agent_collision checks the (unchanged) diamonds_remaining value, sees it's zero, and immediately exits the episode.
@jordan-schneider
Copy link
Author

This is easily fixed by initializing diamonds_remaining to a sentinel value like -1. If this is something y'all are interested in fixing I'd be happy to write the PR.

@christopherhesse
Copy link
Collaborator

Thanks for investigating and letting us know, but unfortunately we don't want to change the behavior of the published environments. I think there has been a couple more reports like this, so if you wanted to make a PR to start a KNOWN_ISSUES.md file with this issue, linked from the README, I'd be happy to approve that.

@christopherhesse
Copy link
Collaborator

Could also note the bug in a code comment in miner.cpp so that people would be more likely to see it.

@christopherhesse
Copy link
Collaborator

Here is the other issue I found: #59

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

No branches or pull requests

2 participants