Improve FreeKickPlay behaviour and convert it to use FSMs #2953

raymond212 · 2023-06-30T06:20:56Z

Please fill out the following before requesting review on this PR

Description

Improved FreeKickPlay:

Reimplemented it using FSMs
Used KickTactic instead of AttackerTactic to ensure that we do not dribble
Deleted CornerKickPlay

Todo:

Add validation for free kick play

Testing Done

Resolved Issues

resolves #2950, resolves #3205

Length Justification and Key Files to Review

free_kick_play_fsm.h and free_kick_play_fsm.cpp.

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

…mond/fix_free_kick

williamckha

Sweet

williamckha · 2023-07-01T03:48:50Z

src/software/ai/hl/stp/play/free_kick/free_kick_play.cpp

+    /**
+     * This play is basically:
+     * - One robot attempts to shoot first. If there is no good shot, it will attempt to
+     *   pass, and finally chips towards the enemy goal if it can't find a pass in time
+     * - Two robots try to get in good positions near the enemy net to receive a pass
+     * - Two robots crease defend
+     * - One robot is goalie
+     */


We should include this comment at the top of free_kick_play.h since this function might be removed later

williamckha · 2023-07-01T04:24:32Z

src/software/ai/hl/stp/play/free_kick/free_kick_play_fsm.cpp

+      MAX_TIME_TO_COMMIT_TO_PASS(Duration::fromSeconds(
+          5)),  // Spend at most 5 seconds looking for a pass. The rule for division B
+                // teams is at most 10 seconds spent on the entire free kick


we might want to make this a configurable param in parameters.proto

I think this parameter is quite specific to free kick, so it might be fine to just declare it in FreeKickPlayFSM.h as a static const.

itsarune · 2023-07-01T16:35:56Z

src/software/ai/hl/stp/play/free_kick/free_kick_play_fsm.cpp

+      MAX_TIME_TO_COMMIT_TO_PASS(Duration::fromSeconds(
+          5)),  // Spend at most 5 seconds looking for a pass. The rule for division B
+                // teams is at most 10 seconds spent on the entire free kick
+      MIN_ACCEPTABLE_PASS_SCORE(0.1),
+      MIN_OPEN_ANGLE_FOR_SHOT(
+          5)  // Only attempt shots with an opening angle greater than 5 degrees


for these constants, i'd prefer them to be declared as static const and declared directly in the header file. I believe there are some compiler optimizations to doing constants in that way

nimazareian · 2023-10-06T04:34:37Z

@raymond212 is this PR still good to be merged (not sure if we changed this on the robocup branch)? Is it implemented the way we want it? (cc: @itsarune @williamckha)

raymond212 · 2023-10-07T18:57:37Z

@nimazareian This PR will probably get somewhat overhauled when we rewrite our gameplay architecture but it definitely improves upon the current implementation and I think it can be merged for now.

src/software/ai/hl/stp/play/free_kick/free_kick_play.h

itsarune

LGTM, minor nit

nimazareian

Skimmed through the changes and it looks good. Just pending Arun's comment and the failing CI (fixed once you pull upstream master)

itsarune · 2023-11-20T21:21:18Z

src/software/ai/hl/stp/play/free_kick/free_kick_play_fsm.cpp

+      chip_tactic(std::make_shared<ChipTactic>()),
+      passer_tactic(std::make_shared<KickTactic>()),
+      receiver_tactic(std::make_shared<ReceiverTactic>()),
+      offensive_positioning_tactics(std::vector<std::shared_ptr<MoveTactic>>(2)),


Not blocking this merge but I think free kick play FSM is only handling two robot behaviour atm (can be fixed later) Or am I wrong?

itsarune

I tested it on my computer and it seems that during the free kick process, several robots are being assigned a StopTactic. I don't think that is ideal.

nimazareian · 2024-03-10T05:14:30Z

@raymond212 What's the status of this PR? Do you think you could update it? I think it could be nice to get it in and not wait on the gameplay overhaul since we don't have an exact timeline on that. What do you think @itsarune?

…into raymond/fix_free_kick

nimazareian · 2024-06-01T07:37:03Z

@raymond212 Once this PR is ready, similar to to #3146, could you also include some videos of what the new gameplay looks like?

raymond212 · 2024-06-01T17:43:25Z

@raymond212 Once this PR is ready, similar to to #3146, could you also include some videos of what the new gameplay looks like?

Yes, I'll be sure to do that.

…into raymond/fix_free_kick

itsarune · 2024-06-03T03:42:50Z

src/proto/play.proto

-    DribblingParcourPlay               = 21;
-    ScoringFromContestedPossessionPlay = 22;
-    PassEndurancePlay                  = 23;
+    ShootOrPassPlay                    = 5;


you should reserve 5 and keep the existing proto numbers so that we don't break previous replays

itsarune · 2024-06-03T03:44:08Z

src/software/ai/hl/stp/play/free_kick/free_kick_play.h

+class FreeKickPlay : public Play
+{
+   public:
+    FreeKickPlay(TbotsProto::AiConfig config);


the constructor should have docs

itsarune · 2024-06-03T03:49:58Z

src/software/ai/hl/stp/play/free_kick/free_kick_play_fsm.cpp

+
+    shoot_tactic->updateControlParams(
+        ball_pos, (shot->getPointToShootAt() - ball_pos).orientation(),
+        BALL_MAX_SPEED_METERS_PER_SECOND - 0.5);


maybe we should refactor attacker_tactic instead to add the padding within attacker tactic.

my concern: sometime in the future, we decide we want to lower/increase the shot speed and we have to find all instances of attacker tactic performing a shot and change it which isn't maintainable.

itsarune · 2024-06-03T03:50:59Z

src/software/ai/hl/stp/play/free_kick/free_kick_play_fsm.cpp

+    LOG(DEBUG) << "Time to look for pass expired. Chipping ball...";
+    PriorityTacticVector tactics_to_run = {{}};
+
+    double fallback_chip_target_x_offset = 1.5;


maybe this should be a static constexpr

itsarune · 2024-06-03T04:03:17Z

src/software/ai/hl/stp/play/free_kick/free_kick_play_test.py

could you supplement the these tests to demonstrate free kicks shooting & chipping as well?

itsarune

looking good 🔥!

It still seems some defenders seem to be assigned StopTactics unintentionally. Let's try to explicitly assign robots to all tactics

raymond212 added 10 commits June 26, 2023 13:07

Fix free kick passing

e8a71e3

Implement shooting behavior if possible

8a19f00

Move free kick play to new folder

b75ca77

Fix build file

d8674ed

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

36b7990

…mond/fix_free_kick

Convert FreeKickPlay to use FSMs

13fec3f

Remove unused imports

f4ee834

wip

1290070

wip

3f94ad0

Revert changes to free kick test

a7a1a72

raymond212 requested review from williamckha and germchow June 30, 2023 06:20

raymond212 requested review from nimazareian and itsarune as code owners June 30, 2023 06:20

Add docs

2f75bd7

williamckha reviewed Jul 1, 2023

View reviewed changes

itsarune reviewed Jul 1, 2023

View reviewed changes

raymond212 added 6 commits July 3, 2023 03:16

Improve commenting

33d1506

Delete extra newline

c3695f4

Initialize constants in header file

e227cdc

Improve log for free kick passing

8645098

Start moving pass receivers earlier

99eceba

Fix formatting

36f2f04

nimazareian mentioned this pull request Jul 3, 2023

Merging PRs into Robocup 2023 #2954

Merged

4 tasks

williamckha previously approved these changes Jul 3, 2023

View reviewed changes

raymond212 requested a review from itsarune October 9, 2023 04:11

itsarune reviewed Oct 17, 2023

View reviewed changes

src/software/ai/hl/stp/play/free_kick/free_kick_play.h Show resolved Hide resolved

itsarune reviewed Oct 17, 2023

View reviewed changes

nimazareian reviewed Oct 27, 2023

View reviewed changes

Add docs

fc30414

raymond212 dismissed williamckha’s stale review via fc30414 October 28, 2023 03:55

Undo change in pytest

37422e6

itsarune reviewed Nov 20, 2023

View reviewed changes

itsarune reviewed Nov 22, 2023

View reviewed changes

Bvasilchikov added the Gameplay label Feb 8, 2024

raymond212 marked this pull request as draft May 19, 2024 18:44

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

fa9cbcb

…into raymond/fix_free_kick

raymond212 mentioned this pull request May 19, 2024

Integrate FreeKickPlay and KickoffPlay into OffencePlay #2798

Closed

2 tasks

raymond212 added 3 commits May 25, 2024 13:46

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

bbd3c03

…into raymond/fix_free_kick

Delete CornerKickPlay

ab56c66

Use dynamic params, fix formatting

af5d7b2

raymond212 added 3 commits June 2, 2024 13:55

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

d2fd4ff

…into raymond/fix_free_kick

Fix formatting

ed26163

Fix build file

38c52f2

itsarune reviewed Jun 3, 2024

View reviewed changes

nimazareian mentioned this pull request Jun 12, 2024

New passing and receiving architecture and tuned offense strategy #3200

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve FreeKickPlay behaviour and convert it to use FSMs #2953

Improve FreeKickPlay behaviour and convert it to use FSMs #2953

raymond212 commented Jun 30, 2023 •

edited

williamckha left a comment

williamckha Jul 1, 2023

raymond212 Jul 3, 2023

williamckha Jul 1, 2023

raymond212 Jul 3, 2023

itsarune Jul 1, 2023

raymond212 Jul 3, 2023

nimazareian commented Oct 6, 2023 •

edited

raymond212 commented Oct 7, 2023 •

edited

itsarune left a comment

nimazareian left a comment

itsarune Nov 20, 2023

itsarune left a comment •

edited

nimazareian commented Mar 10, 2024

nimazareian commented Jun 1, 2024

raymond212 commented Jun 1, 2024

itsarune Jun 3, 2024

itsarune Jun 3, 2024

itsarune Jun 3, 2024

itsarune Jun 3, 2024

itsarune Jun 3, 2024

itsarune left a comment

Improve FreeKickPlay behaviour and convert it to use FSMs #2953

Are you sure you want to change the base?

Improve FreeKickPlay behaviour and convert it to use FSMs #2953

Conversation

raymond212 commented Jun 30, 2023 • edited

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

williamckha left a 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

nimazareian commented Oct 6, 2023 • edited

raymond212 commented Oct 7, 2023 • edited

itsarune left a comment

Choose a reason for hiding this comment

nimazareian left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itsarune left a comment • edited

Choose a reason for hiding this comment

nimazareian commented Mar 10, 2024

nimazareian commented Jun 1, 2024

raymond212 commented Jun 1, 2024

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

raymond212 commented Jun 30, 2023 •

edited

nimazareian commented Oct 6, 2023 •

edited

raymond212 commented Oct 7, 2023 •

edited

itsarune left a comment •

edited