OffensePlayTest checking that there is Never Excessive Dribbling by adrianchan787 · Pull Request #3536 · UBC-Thunderbots/Software

adrianchan787 · 2025-11-30T11:45:32Z

Description

Added a validation status check in offense play test that checks if the friendly team has excessively dribbled
Changed the validation status check (NeverExcessivelyDribbles and EventuallyStartsExcessiveDribbles) to look at the dribble_displacement field in World instead of the previous "brute force" method; this (theoretically) aligns more with the AutoRef's implementation of excessive dribbling
Added a testing file for the validation status check

Testing Done

- Reran all pytests that used NeverExcessivelyDribbles; some of them were flaky for unrelated reasons e.g. pass_defender_test had a robot enter region validation error first time around
- Created a testing file that tests never excessively and eventually starts dribbling, using the dribble_tactic. (since the main dribbling done in offense_play_test is in dribble_tactic). These tests typically pass but are also flaky; I'll expand more in the comments at the end.

Resolved Issues

Resolves #3452

Length Justification and Key Files to Review

Review Checklist

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

[x ] 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.
[ x] Remove all commented out code
[ x] Remove extra print statements: for example, those just used for testing
[ x] Resolve all TODO's: All TODO (or similar) statements should either be completed or associated with a github issue

Additional Comments

Not really an addition and more of a few observations and questions. Wanted to still make this PR since I feel like it's not directly related to my current ticket (and also because I've dragged this on for too long lol).
Was looking at how the Autoref implemented dribble distance, and from what I understood it seems to compare initial position of the robot to the final position of the ball (see the code snippet below, taken from https://github.com/TIGERs-Mannheim/AutoReferee/tree/53063578e38ac4818849df3196b32a856a5fa41d). If I'm mistaken, please tell me and I'll edit the comments in my branch lol

However, because they use robot to ball and not e.g. ball to ball, the dribble displacement field is extremely inconsistent. For example, in my tests, if you start the robot in the same location as the ball, you get a different dribble displacement than if you started the robot a meter way. (which I think makes sense? since the orientation with which you start dribbling will probably be different).
Similarly, that's why the maximum distance between the initial point location of the ball and its location after dribbling is greater than 1 m. From tests I got up to 1.03 m. However, this maximum limit was pretty flaky for me, and fluctuated between 1.03 and 1.01 m. (which is why there's a decent chance my code won't pass my own tests).
So I was wondering if this is an issue? On one hand, (I think?) if this mirrors the Autoref's behaviour, it doesn't matter how inconsistent it is. On the other, I know William said the 1 meter max dribbling was a pretty strict limit, so would it be worthwhile to be conservative and use like 0.98 max distance in testing and tactics?

Sorry if that was a bit unclear, please let me know if it was. Thanks!

adrianchan787 · 2025-11-30T11:53:46Z

(oops really sorry not sure if i was supposed to resolve the merge conflict)

…/adrianchan787/Software into adrian/offense_play_test_dribble

nycrat

Added some comments, just some refactoring changes recommended but the new implementation of excessive_dribbing validation seems good and the test are thorough

src/software/ai/hl/stp/tactic/dribble/excessive_dribble_test.py

src/software/simulated_tests/excessive_dribbling.py

src/software/ai/hl/stp/tactic/dribble/excessive_dribble_test.py

src/software/ai/hl/stp/play/offense/offense_play_test.py

src/software/ai/hl/stp/tactic/dribble/excessive_dribble_test.py

src/software/simulated_tests/excessive_dribbling.py

nycrat

LGTM

nycrat · 2026-02-04T17:24:31Z

Hey @adrianchan787 could you pull from master so that the ci passes?

adrianchan787 · 2026-02-07T19:15:35Z

Hey @adrianchan787 could you pull from master so that the ci passes?

really sorry that it took this long, should be good now

adrianchan787 · 2026-02-07T20:29:02Z

Hey @adrianchan787 could you pull from master so that the ci passes?

really sorry that it took this long, should be good now

Wow I have messed this up horribly one sec (ok i am so incredibly sorry for this cursed git log)

This reverts commit 6d9a363.

Andrewyx

Nice work so far, I left some feedback!

Andrewyx · 2026-02-13T00:42:42Z

docs/getting-started.md

+  - [Installing Software Dependencies](#installing-software-dependencies)
+  - [Installing an IDE](#installing-an-ide)
+    - [Installing an IDE: CLion](#installing-an-ide-clion)
+      - [Getting your Student License](#getting-your-student-license)
+      - [Installing CLion](#installing-clion)
+  - [Installing an IDE: VS Code](#installing-an-ide-vs-code)
+  - [Editing with Vim or NeoVim](#editing-with-vim-or-neovim)
+- [Building and Running the Code](#building-and-running-the-code)
+  - [Building from the command line](#building-from-the-command-line)
+  - [Building from the command line using the fuzzy finder](#building-from-the-command-line-using-the-fuzzy-finder)
+  - [Building with CLion](#building-with-clion)
+  - [Building with VS Code](#building-with-vs-code)
+  - [Running our AI, Simulator, SimulatedTests or Robot Diagnostics](#running-our-ai-simulator-simulatedtests-or-robot-diagnostics)
+- [Debugging](#debugging)
+  - [Debugging with CLion](#debugging-with-clion)
+  - [Debugging from the Command Line](#debugging-from-the-command-line)
+- [Profiling](#profiling)
+  - [Callgrind](#callgrind)
+  - [Tracy](#tracy)
+- [Building for the robot](#building-for-the-robot)
+- [Deploying Robot Software to the robot](#deploying-robot-software-to-the-robot)
+- [Testing Robot Software locally](#testing-robot-software-locally)
+- [Setting up Virtual Robocup 2021](#setting-up-virtual-robocup-2021)
+  - [Setting up the SSL Simulation Environment](#setting-up-the-ssl-simulation-environment)


nit: Probably unintended formatting change

Andrewyx · 2026-02-13T00:43:43Z

docs/getting-started.md

-
-# Software Setup
-
-## Introduction
-
-These instructions assume that you have the following accounts setup:
- [GitHub](https://github.com/login)
- [Discord](https://discord.com). Please contact a Thunderbots lead to receive the invite link.
-
-These instructions assume you have a basic understanding of Linux and the command line. There are many great tutorials online, such as [LinuxCommand](http://linuxcommand.org/). The most important things you'll need to know are how to move around the filesystem and how to run programs or scripts.
-
-## Installation and Setup
-
-### Operating systems
-
-We currently only support Linux, specifically Ubuntu.


Unintended deletion?

Andrewyx · 2026-02-13T00:45:30Z

src/software/simulated_tests/excessive_dribbling.py

+    def get_validation_status(
+        self,
+        world,
+    ) -> ValidationStatus:


nit: is this formatting right?

Andrewyx · 2026-02-13T00:46:24Z

src/software/simulated_tests/excessive_dribbling.py

        """Checks if any friendly robot is excessively dribbling the ball, i.e. for over 1m.

        :param world: The world msg to validate
+               estimate of max dribble distance (effective dribble distance is length - error margin)


Where is this comment directed to?

Andrewyx · 2026-02-13T01:39:55Z

src/software/simulated_tests/excessive_dribbling.py

+        if (
+            world.HasField("dribble_displacement")
+            and world.dribble_displacement is not None
+        ):
+            dribble_disp = world.dribble_displacement
+            dist = tbots_cpp.createSegment(dribble_disp).length()
+            if dist > (
+                DribblingConstants.MAX_DRIBBLING_DISTANCE
+                - DribblingConstants.DRIBBLING_ERROR_MARGIN
+            ):
                return ValidationStatus.FAILING
-            elif self.continous_dribbling_start_point is None:
-                # ball is in dribbler, but previously wasn't in dribbler, so set continuous dribbling start point
-                self.continous_dribbling_start_point = ball_position
+


@sauravbanna @williamckha Can you take a look at this? I am hesitant to think this code is correct since dribble_displacement might be determined by SensorFusion and in that case, our internal logic is also referencing dribble_displacement when deciding when to let go of the ball.

Additionally, suppose a bug arises in SensorFusion where dribble_displacement is None, but in fact we are dribbling. In that case, this test would pass since it is referencing the value passed on by SensorFusion.

I see your concern. Though imo it does not make sense to return FAILING if dribble_displacement is null, since if it is accurate, that would mean we are not dribbling at all and therefore not breaking the excessive dribbling rule. I would suggest adding an extra validation to the excessively dribbling test that checks if the ball is eventually moved to its destination. This would validate that the ball is actually being dribbled.

wait nevermind I didn't read the second part of your comment properly. To address that potential bug, we should check that the dribble_displacement member is eventually non null.

Andrewyx · 2026-02-13T01:40:36Z

src/software/thunderscope/constants.py

+class DribblingConstants:
+    """Constants to be used to check whether the robot has dribbled beyond competition regulations"""
+
+    MAX_DRIBBLING_DISTANCE = 1.0


nit: Displacement not distance

Andrewyx · 2026-02-13T01:41:34Z

src/software/ai/hl/stp/tactic/dribble/excessive_dribble_test.py

+@pytest.mark.parametrize(
+    "initial_location,dribble_destination,final_dribble_orientation, should_excessively_dribble",
+    [
+        # Tests Should not excessively dribble


nit: Unnecessary comment

probably unclear, but my tests are split between not excessively dribbling and excessively dribbling, hence the comment (there's a corresponding comment below for excessively dribbling). I'll try to format it better?

Andrewyx · 2026-02-13T01:45:45Z

src/software/ai/hl/stp/tactic/dribble/excessive_dribble_test.py

+    simulated_test_runner,
+):
+    if should_excessively_dribble:
+        blue_robot_locations = [tbots_cpp.Point(0, 0.0)]


We probably do not want to set the locations in the test, and instead have them parameterized. This clear separation makes reading the logic of the test cases make more sense.

got it makes sense

Andrewyx · 2026-02-13T01:56:17Z

src/software/ai/hl/stp/tactic/dribble/excessive_dribble_test.py

+        ),
+    ],
+)
+def test_excessive_dribbling(


I'm not quite sure what the reasoning was for this test, since the issue this PR is marked for is regarding offense play. Reading through the work here, it appears to be more akin to a DribbleTactic test. Excessive dribbling is a characteristic of Gameplay more than a particular state, so I think it is a bit odd to be testing it explicitly since it is really just testing the implementation of DribbleTactic itself.

Hmm ig that makes sense. I was thinking (please correct me if I'm wrong or really misguided) but the main dribbling tactic used in offense play is DribbleTactic, and if that's the case if it works in DribbleTactic then it should work everywhere else, including in offense play. As it is easier to test (especially boundary test) DribbleTactic directly rather than test offense play, I was thinking it might be the better option.

adrianchan787 added 6 commits November 1, 2025 12:24

Added NeverExcessivelyDribbles functionality into OffensePlayTest

ec7010a

intermediate commit

b690a45

intermediate change

c339388

Switched to world proto subscription measurement

a4f6bc0

Added excessive dribbling functionality and testing

544bc55

Reverted verbosity change which was for print testing previously

d928b5d

adrianchan787 requested review from Andrewyx, sauravbanna and williamckha as code owners November 30, 2025 11:45

Merge branch 'master' into adrian/offense_play_test_dribble

a3929cf

[pre-commit.ci lite] apply automatic fixes

32cc42d

adrianchan787 marked this pull request as draft November 30, 2025 22:10

adrianchan787 added 2 commits November 30, 2025 14:49

Added error margin of 0.05 to max dribbling distance

5b0b98c

Merge branch 'adrian/offense_play_test_dribble' of https://github.com…

2fb03f8

…/adrianchan787/Software into adrian/offense_play_test_dribble

adrianchan787 marked this pull request as ready for review November 30, 2025 23:01

[pre-commit.ci lite] apply automatic fixes

dff8853

nycrat requested changes Dec 2, 2025

View reviewed changes

Andrewyx reviewed Dec 4, 2025

View reviewed changes

StarrryNight reviewed Dec 10, 2025

View reviewed changes

src/software/simulated_tests/excessive_dribbling.py Outdated Show resolved Hide resolved

adrianchan787 and others added 10 commits January 8, 2026 22:59

Made testing file one function

8336df9

Increased max dribble margin to 0.06 to reduce flakiness

8d1354f

Fixed import statements for test file

75141c3

Changed get validation to reflect correct start point

2d235e3

Merge conflict fix

ed1102e

[pre-commit.ci lite] apply automatic fixes

e3afcc5

Added constants to the excessive dribbling

7edc073

Removed allow excessive dribbling for the tests

87eb7b9

Merged master

20f5cfd

Fixed merge conflict

e9d2ff6

adrianchan787 added 2 commits January 31, 2026 11:51

Merge conflicts fixed

7e46bdf

Added overrides

6d9a363

nycrat previously approved these changes Jan 31, 2026

View reviewed changes

pulled from master

9812bc1

adrianchan787 closed this Feb 7, 2026

adrianchan787 reopened this Feb 7, 2026

[pre-commit.ci lite] apply automatic fixes

871d210

pre-commit-ci-lite bot dismissed nycrat’s stale review via 871d210 February 7, 2026 19:22

adrianchan787 added 3 commits February 7, 2026 12:17

test

9022e22

fixed syntax

8a2bf29

Merge conflicts fixed

22ec418

pre-commit-ci-lite bot and others added 12 commits February 7, 2026 20:34

[pre-commit.ci lite] apply automatic fixes

57e14e9

i think it's finally good now?

1d98837

test push to see if it fixes the 59 changed files...

14ef1b0

[pre-commit.ci lite] apply automatic fixes

5e15f1a

Revert "Added overrides"

c0632cd

This reverts commit 6d9a363.

revert

d36bd78

revert

3edb21c

revert merge

6cef848

revert

41344a3

merge

417a094

Revert

6e3cb23

[pre-commit.ci lite] apply automatic fixes

212b4db

Andrewyx requested changes Feb 13, 2026

View reviewed changes

adrianchan787 added 3 commits February 14, 2026 13:06

fixed some of the review stuff

7a55bf4

Revert precommit bot

e77077e

Fixed test formatting

53ed48b

Conversation

adrianchan787 commented Nov 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing Done

Resolved Issues

Length Justification and Key Files to Review

Review Checklist

Additional Comments

Uh oh!

adrianchan787 commented Nov 30, 2025

Uh oh!

nycrat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nycrat left a comment

Choose a reason for hiding this comment

Uh oh!

nycrat commented Feb 4, 2026

Uh oh!

adrianchan787 commented Feb 7, 2026

Uh oh!

adrianchan787 commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Andrewyx left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

williamckha Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andrewyx Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

adrianchan787 commented Nov 30, 2025 •

edited

Loading

adrianchan787 commented Feb 7, 2026 •

edited

Loading

williamckha Feb 14, 2026 •

edited

Loading

Andrewyx Feb 13, 2026 •

edited

Loading