Render every balloon in the vpython renderer#27
Open
thc1006 wants to merge 2 commits into
Open
Conversation
_render_frame's vpython branch created a single sphere and positioned it from _balloon_states[0], so a scenario with N balloons showed only the first. Create one sphere per balloon and update them all, which matches the matplotlib branch that already slices over every balloon. Add tests/test_render.py, which mocks vpython and checks that a reset creates one sphere per balloon. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the vpython renderer so it creates and updates a sphere for each balloon instead of rendering only balloon 0.
Changes:
- Replaces the single vpython balloon sphere with a list of spheres.
- Updates each sphere position from the corresponding balloon state.
- Adds a behavioral test for vpython sphere creation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
BalloonPoppingGymEnv/envs/balloon_world.py |
Updates the vpython render branch to create and position one sphere per balloon. |
tests/test_render.py |
Adds a mocked-vpython regression test for rendering all balloons. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+354
to
+355
| for balloon, state in zip(self.render_balloons, self._balloon_states): | ||
| balloon.pos = vector(state[0], state[1], state[2]) |
Comment on lines
+29
to
+32
| """True when the heavy simulation stack (rocketpy) is installed.""" | ||
| return importlib.util.find_spec("rocketpy") is not None | ||
|
|
||
|
|
The render test checked that num spheres are created but not that all of them are repositioned each frame, so a regression updating only one sphere would still pass. Add a vector() call-count assertion that pins the per-balloon update loop. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #26:
_render_frame'svpythonbranch only ever drew balloon 0.What changed
_render_frame, therender_mode == "vpython"branch:sphereand positioned it fromself._balloon_states[0, 0:3](hardcoded index0), so a scenario with N balloons showed only the first one.sphereper balloon and updates every sphere's.posfrom its own row ofself._balloon_states.This matches the
matplotlibbranch in the same method, which already slices over all balloons (self._balloon_states[:, ...]).Balloon colour stays
magenta, the same as the previous single sphere. Status colouring and rocket rendering are still missing from the vpython branch (thematplotlibbranch has both); those are out of scope here and noted in #26 for a later pass.Verified
tests/test_render.pystarted red (the old code created 1 sphere; scenario 0 has 10 balloons) and went green after the change.vpythonmodule, builds a scenario-0 env withrender_mode="vpython", and asserts that areset()creates one sphere per balloon.ruff checkandruff format --checkpass; the full test suite passes (10 tests).vpython rendering itself needs a display, so it cannot be exercised headless; the test verifies the logic (one sphere per balloon, all updated).
Closes #26