[Thunderscope] Remove Extra Robots and Add Missing Robots during Simulator + Fixes #3454

Andrewyx · 2025-03-30T20:39:11Z

Please fill out the following before requesting review on this PR

Description

This PR provides support for dynamic robot removal & re-addition during simulation per acquired cards during play. Addition is then called after cards expire (yellow), and robots are placed back onto the field along the field margin along the halfway line (per RoboCup rules).
Also addresses the following:

Introduces a new ThreadSafeCircularBuffer for more efficient proto handling
this ticket also resolves a few other Thunderscope bugs. See below.

Testing Done

Modifying GC during sims works.

Screencast.from.2025-03-30.13-19-52.webm

Resolved Issues

Resolves #3116

Extras:

Small fix to Remove all instances of MessageToDict #3405
Cleans up ThreadSafeBuffer usage
Fixes fullsystem termination upon AutoRef goal score
Refactors ThreadSafeBuffer for better OOP

Length Justification and Key Files to Review

Review Checklist

It is the reviewers responsibility to also make sure every item here has been covered

Function & Class comments: All function definitions (usually in the .h file) should have a javadoc style comment at the start of them. For examples, see the functions defined in thunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.
Remove all commented out code
Remove extra print statements: for example, those just used for testing
Resolve all TODO's: All TODO (or similar) statements should either be completed or associated with a github issue

…into Andrewyx/Remove_extra_robots

Copilot

Pull Request Overview

This PR enhances the Thunderscope simulator by supporting dynamic robot removal and re-addition during simulation, while also refactoring buffer implementations and updating referee-related commands and tests. Key changes include:

Updating the Gamecontroller to pass in a simulator-specific proto IO and introducing a ThreadSafeCircularBuffer.
Refactoring various modules to replace direct queue access with uniform buffer get/size methods.
Updating test cases and referee command mappings from indirect to direct commands.

Reviewed Changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/software/thunderscope/thunderscope_main.py	Updated Gamecontroller instantiation to include simulator_proto_unix_io.
src/software/thunderscope/thread_safe_buffer.py	Replaced direct queue usage with a new buffer attribute and updated exception handling.
src/software/thunderscope/log/g3log_widget.py	Modified log retrieval to use the buffer's get method.
src/software/thunderscope/gl/layers/*	Refactored graphics layers to use buffer.size() and get methods instead of direct queue access.
src/software/thunderscope/common/thread_safe_circular_buffer.py	Added a new circular buffer for improved proto handling.
src/software/thunderscope/common/proto_plotter.py	Adjusted buffer iteration to use the new size() method.
src/software/thunderscope/binary_context_managers/game_controller.py	Enhanced robot count updating and integrated new referee handling.
src/software/sensor_fusion/sensor_fusion_test.cpp	Updated tests to use direct referee commands instead of indirect ones.
src/proto/message_translation/*	Updated deprecated referee commands and removed indirect command tuples.

Files not reviewed (2)

src/software/thunderscope/binary_context_managers/BUILD: Language not supported
src/software/thunderscope/common/BUILD: Language not supported

Comments suppressed due to low confidence (1)

src/software/thunderscope/common/thread_safe_circular_buffer.py:53

MIN_DROPPED_BEFORE_LOG is referenced but not defined in this class; consider defining it as a class-level constant if it is not inherited from the parent class.

and self.protos_dropped > self.MIN_DROPPED_BEFORE_LOG

Copilot · 2025-04-18T07:55:27Z

src/software/thunderscope/gl/layers/gl_tactic_layer.py

+                    {tactic_assignments[robot.id].tactic_name} - 
+                    {tactic_assignments[robot.id].tactic_fsm_state}


The earlier version accessed tactic assignments using string keys via a function call, but the updated code uses robot.id directly; ensure that the key types are consistent (e.g., converting robot.id to string if required) to avoid potential mismatches.

Suggested change

{tactic_assignments[robot.id].tactic_name} -

{tactic_assignments[robot.id].tactic_fsm_state}

{tactic_assignments[str(robot.id)].tactic_name} -

{tactic_assignments[str(robot.id)].tactic_fsm_state}

probably a bad take here

itsarune · 2025-04-18T07:58:04Z

src/proto/message_translation/ssl_referee.cpp

@@ -62,6 +70,8 @@ const static std::unordered_map<SSLProto::Referee::Command, RefereeCommand>
 RefereeCommand createRefereeCommand(const SSLProto::Referee &packet,
                                    TeamColour team_colour)
 {
+    if (deprecated_commands.contains(packet.command()))


didn't we notice the gamecontroller randomly generate these? couldn't this cause our robots to randomly halt during the game (which is bad).

itsarune · 2025-04-18T07:58:56Z

src/proto/message_translation/ssl_referee_test.cpp

@@ -124,17 +124,6 @@ INSTANTIATE_TEST_CASE_P(
        std::make_tuple(RefereeCommand::DIRECT_FREE_THEM,
                        SSLProto::Referee_Command_DIRECT_FREE_YELLOW, TeamColour::BLUE),

-        // indirect free


depending on above comment, you might need to change this

itsarune · 2025-04-18T08:00:03Z

src/software/thunderscope/binary_context_managers/game_controller.py

+        self,
+        suppress_logs: bool = False,
+        use_conventional_port: bool = False,
+        simulator_proto_unix_io: ProtoUnixIO = None,


simulated_proto_unix_io needs docs

also you can probably pass this in via setup_proto_unix_io

itsarune · 2025-04-18T08:02:50Z

src/software/thunderscope/binary_context_managers/game_controller.py

+            return
+        for _ in range(robots_diff):
+            try:
+                robot_states[removed_robot_ids.get_nowait()].CopyFrom(place_state)


do multiple robots being spawned in the same location cause any issues?

itsarune · 2025-04-18T08:03:51Z

src/software/thunderscope/binary_context_managers/game_controller.py

+            except queue.Empty:
+                return
+
+    def handle_referee(self, referee: Referee) -> None:


handle_referee is not a clear enough name here. I suggest handle_fouls

itsarune · 2025-04-18T08:04:48Z

src/software/thunderscope/binary_context_managers/game_controller.py

@@ -168,6 +281,7 @@ def __send_referee_command(data: Referee) -> None:

            :param data: The referee command to send
            """
+            self.handle_referee(data)


you can probably move this down to the if autoref_proto_unix_io is not None check below.

or you should probably have a new argument for this class called enable_auto_remove_robots that will enable this feature

itsarune · 2025-04-18T08:05:29Z

src/software/thunderscope/binary_context_managers/game_controller.py

@@ -187,6 +301,9 @@ def __send_referee_command(data: Referee) -> None:
        yellow_full_system_proto_unix_io.register_observer(
            ManualGCCommand, self.command_override_buffer
        )
+        blue_full_system_proto_unix_io.register_observer(


why only register the World for blue?

itsarune · 2025-04-18T08:07:24Z

src/software/thunderscope/common/thread_safe_circular_buffer.py

+from typing import override
+
+
+class ThreadSafeCircularBuffer(ThreadSafeBuffer):


make an issue to collapse both ThreadSafeBufer and ThreadSafeCircularBuffer into better OOP-style

or we could completely replace ThreadSafeBuffer into a deque implementation which I think will work

itsarune · 2025-04-18T08:09:07Z

src/software/thunderscope/gl/layers/gl_cost_vis_layer.py

-            cost_vis = self.cost_visualization_buffer.queue.get_nowait()
-        except queue.Empty:
-            cost_vis = None
+        cost_vis = self.cost_visualization_buffer.get(block=False, return_cached=False)


does it not raise a queue.Empty error anymore?

itsarune · 2025-04-18T08:09:35Z

src/software/thunderscope/gl/layers/gl_passing_layer.py

-            pass_vis = self.pass_visualization_buffer.queue.get_nowait()
-        except queue.Empty:
-            pass_vis = None
+        pass_vis = self.pass_visualization_buffer.get(block=False, return_cached=False)


does it not raise a queue.Empty error anymore?

itsarune · 2025-04-18T08:10:43Z

src/software/thunderscope/log/g3log_widget.py

-            log = self.log_buffer.queue.get_nowait()
-        except queue.Empty:
+
+        log = self.log_buffer.get(block=False, return_cached=False)


no queue.Empty exception?

itsarune · 2025-04-18T08:23:32Z

src/software/thunderscope/binary_context_managers/game_controller.py

@@ -168,6 +281,7 @@ def __send_referee_command(data: Referee) -> None:

            :param data: The referee command to send
            """
+            self.handle_referee(data)


pass in simulator_proto_unix_io as an argument to this setup_proto_unix_io function and then you can call self.handle_data(simulator_proto_unix_io and avoid modifying the constructor

itsarune

great work! this will help us loads in figuring out gameplay issues before robocup!

Andrewyx added 7 commits March 26, 2025 23:05

init

c0ccafa

tests

b208b59

working

5a1b327

changes

44d15c6

Merge branch 'master' of https://github.com/UBC-Thunderbots/Software …

e994921

…into Andrewyx/Remove_extra_robots

changes

0b52eb5

finished

8938700

Andrewyx requested review from itsarune, williamckha and sauravbanna as code owners March 30, 2025 20:39

pre-commit-ci-lite bot and others added 5 commits March 30, 2025 20:48

[pre-commit.ci lite] apply automatic fixes

f8b50c7

cleanup

d780dc4

changes

5d7d0cf

changes

785ce1a

removed tests

268379b

itsarune requested review from Copilot, a team, DimitriK24, someone2060 and AmyKawa and removed request for a team April 18, 2025 07:54

Copilot AI reviewed Apr 18, 2025

View reviewed changes

itsarune reviewed Apr 18, 2025

View reviewed changes

itsarune reviewed Apr 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Thunderscope] Remove Extra Robots and Add Missing Robots during Simulator + Fixes #3454

[Thunderscope] Remove Extra Robots and Add Missing Robots during Simulator + Fixes #3454

Andrewyx commented Mar 30, 2025 •

edited

Loading

Copilot AI left a comment

Copilot AI Apr 18, 2025

itsarune Apr 18, 2025

itsarune Apr 18, 2025

itsarune Apr 18, 2025

itsarune Apr 18, 2025

itsarune Apr 18, 2025

itsarune Apr 18, 2025

itsarune Apr 18, 2025

itsarune Apr 18, 2025

itsarune Apr 19, 2025

itsarune Apr 18, 2025

itsarune Apr 18, 2025

itsarune Apr 18, 2025

itsarune Apr 18, 2025

itsarune Apr 18, 2025

itsarune Apr 18, 2025

itsarune Apr 18, 2025

itsarune left a comment

		{tactic_assignments[robot.id].tactic_name} -
		{tactic_assignments[robot.id].tactic_fsm_state}

		from typing import override


		class ThreadSafeCircularBuffer(ThreadSafeBuffer):

[Thunderscope] Remove Extra Robots and Add Missing Robots during Simulator + Fixes #3454

Are you sure you want to change the base?

[Thunderscope] Remove Extra Robots and Add Missing Robots during Simulator + Fixes #3454

Conversation

Andrewyx commented Mar 30, 2025 • edited Loading

Please fill out the following before requesting review on this PR

Description

Testing Done

Resolved Issues

Length Justification and Key Files to Review

Review Checklist

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Copilot AI Apr 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itsarune left a comment

Choose a reason for hiding this comment

Andrewyx commented Mar 30, 2025 •

edited

Loading