-
Notifications
You must be signed in to change notification settings - Fork 21
Fix symbolic state of Door
Objects
#106
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
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the symbolic state representation of Door objects to correctly encode their open/locked status as integers (0 for open, 1 for closed unlocked, 2 for closed locked). The implementation adds a symbolic_state
property to all entities and updates the observation system to use this new property instead of the previous hardcoded logic.
- Introduces a
symbolic_state
property to the base Entity class and implements it for all entity types - Replaces hardcoded state calculation in the observation system with the new property-based approach
- Adds comprehensive tests to verify the correct symbolic state encoding for different door configurations
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
tests/issue_95.py | Adds test cases verifying correct symbolic state values for all door state combinations |
navix/observations.py | Simplifies state layer calculation by using the new symbolic_state property |
navix/entities.py | Implements symbolic_state property for all entities with Door having the complex state logic |
navix/_version.py | Bumps version from 0.7.2 to 0.7.3 |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Fixes #95
This implements the correct symbolic state of doors, which is currently incorrect, as highlighted in #95 -- thank you @rikifunt for the great catch.
As a byproduct, entities now have a
symbolic_state
property.