[Bug Report] CarRacing module crashes when executed directly and no longer stops after closing the window

[foxik]

Describe the bug
When running the car_racing.py module directly, it fails with

  File "/tmp/venv/gym/gym/envs/box2d/car_racing.py", line 799, in <module>
    env.render()
  File "/tmp/venv/gym/gym/envs/box2d/car_racing.py", line 568, in render
    return self._render(self.render_mode)
  File "/tmp/venv/gym/gym/envs/box2d/car_racing.py", line 571, in _render
    assert mode in self.metadata["render_modes"]
AssertionError

This is caused by the recent rendering changes, because car_racing.py is constructing CarRacing instance directly in

gym/gym/envs/box2d/car_racing.py

Line 798 in f4d668f

env = CarRacing()

The problem could be fixed by adding render_mode="human" to the CarRacing constructor.

There is also a second problem, however. The main cycle is

    isopen = True
    while isopen:
        env.reset()
        total_reward = 0.0
        steps = 0
        restart = False
        while True:
            register_input()
            s, r, terminated, truncated, info = env.step(a)
            total_reward += r
            if steps % 200 == 0 or terminated or truncated:
                print("\naction " + str([f"{x:+0.2f}" for x in a]))
                print(f"step {steps} total_reward {total_reward:+0.2f}")
            steps += 1
            isopen = env.render()
            if terminated or truncated or restart or isopen is False:
                break

Previously, isopen was set when the user closed the viewer, which is no longer the case with pygame. However:

• currently isopen is not set when closing the window -- the window cannot be closed, actually
• render is called twice -- once inside the step, and also manually in the cycle

I am not sure how to handle this use case:

• instead of

  pygame.event.pump()

  in the render method one can use

  if pygame.event.peek(pygame.QUIT):
      # handle close
  else:
      # original code
      pygame.event.pump()

  to detect that user tried to close the window
• however, it is not clear how to handle the close -- should only a flag be raised? Or the window actually closed?
  • If you only set the isopen flag and return it, then because render is called twice now (once in step), in 50% of the cases this flag will be returned from the render call in step method and will be ignored (because it will not be handled by the above cycle)
  • If you close the window as a response to the QUIT event, then:
    • maybe we could have some API place where this flag will be set, so that something like env.isopen is exported from all environments with a viewer?
    • care must be taken so that a new window is not immediately created (if for example self.surface is None)

Previously there was a single Viewer class handling rendering of all classic_control and box2d environments, but right now the pygame logis is instead replicated in all of them. Maybe we could have again some Viewer class to provide basic functionality, so that changes like "allowing the user to close the window" would be implemented on one place, not identically in many modules.

[RedTachyon]

Thanks, that's a good catch. I'd actually consider removing the whole functionality of running individual files, it seems to be a relic of the old versions of gym, and overall not a great pattern.

[pseudo-rnd-thoughts]

@foxik Would you be able to make a PR to fix it?

[foxik]

@pseudo-rnd-thoughts No problem, I just created it.

The handling of isopen was easy in the end, I just augmented the event cycle in register_input to also handle the pygame.QUIT event. When I was at it, I also added handling of pygame.K_ESCAPE key.