Skip to content

Fixed Sokoban crash bug#7

Merged
samvelyan merged 3 commits intoNetHack-LE:mainfrom
BartekCupial:fix/sokoban_crash
Mar 3, 2025
Merged

Fixed Sokoban crash bug#7
samvelyan merged 3 commits intoNetHack-LE:mainfrom
BartekCupial:fix/sokoban_crash

Conversation

@BartekCupial
Copy link

Associated issue: #4

Description

This PR fixes a crash in the Sokoban environments when the agent character (@) disappears from observation. The crash occurs because _object_positions() returns an empty list when the agent is not found, causing an IndexError when accessing the first element

Changes Made

  • fixed index error which occured in death screen agent_pos = next((pos for pos in self._object_positions(observation, "@")), None)
  • added food to all maps (the same amount as in nle), because some maps are impossible to solve without first starving to death
  • changed action space so we can interact with food (PICKUP, EAT)

Testing

  • Verified crash is fixed with MiniHack-Sokoban1b-v0
import minihack
import gymnasium as gym

env = gym.make("MiniHack-Sokoban1b-v0")

env.reset()
env.render()
done = False

while not done:
    obs, rew, term, trun, info = env.step(env.action_space.sample())
    done = term or trun
    env.render()

Closes #4

@github-actions github-actions bot added the envs label Feb 13, 2025
@BartekCupial
Copy link
Author

@samvelyan can we merge this?

Copy link
Collaborator

@samvelyan samvelyan left a comment

Choose a reason for hiding this comment

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

Hi @BartekCupial. Many thanks for the PR. This looks great. Just have a few small points:

  1. In sokoban.py, could you please rename the version of the environments from -v0 to -v1, given considerable change to their dynamics.

  2. It would be wonderful to also update the documentation to reflect these changes.

@BartekCupial
Copy link
Author

@samvelyan tell me if there sth more to be done

Copy link
Collaborator

@samvelyan samvelyan left a comment

Choose a reason for hiding this comment

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

LGTM

@samvelyan samvelyan merged commit 8b7d3ea into NetHack-LE:main Mar 3, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Sokoban envs crash consistently

2 participants