Software icon indicating copy to clipboard operation
Software copied to clipboard

Tactic and play refactor

Open GrayHoang opened this issue 4 months ago • 13 comments

Description

468327453-3a286cdb-9ca7-47f4-bfdc-d7df4b30e00a The main idea is to encapsulate all of our tactics and tactic-FSMs under a common FSM base class (and tactic class for the tactics). I'm mainly making this pr so I can track my overall changes because I'm starting to lose track between forward and backward progress. Feel free to comment on design choices and the like Accompanying docs: https://www.notion.so/ubcthunderbots/FSM-Tactic-and-Play-Framework-2481e25196bd80f9b339e880004929d6

Testing Done

472839956-6676172b-bdc3-4531-a5ff-c4f67c16c1bd

Resolved Issues

Length Justification and Key Files to Review

This overhauls the entire tactic and play framework. It doesn't make sense to split it since none of the parts can work without being together. Please look at tactic.h, tactic_fsm.h, play_base.h, and play_fsm.h

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

GrayHoang avatar Aug 02 '25 05:08 GrayHoang

uh something weird happened when I was trying to resolve the merge conflicts but it should be fixed now?

We may want to take another closer look at the shadow play and defense play files since those seemed to have the issues

GrayHoang avatar Aug 02 '25 06:08 GrayHoang

Also looks like the FSM diagram generation got a bit cooked? Not sure how to fix that script

GrayHoang avatar Aug 02 '25 06:08 GrayHoang

Ok so ideally this passes and we get this reviewed quickly cause if I have to deal with any more merge conflicts I'm going to lose track of what I have been doing.

GrayHoang avatar Aug 03 '25 21:08 GrayHoang

ye idk whats going on with autofixes

GrayHoang avatar Aug 05 '25 19:08 GrayHoang

Oh also the pre commit thing is making the names of the fsm diagrams incorrect and idk how to fix that

GrayHoang avatar Aug 07 '25 02:08 GrayHoang

https://www.notion.so/ubcthunderbots/FSM-Tactic-and-Play-Framework-2481e25196bd80f9b339e880004929d6 some docs here (idk whats going on with the images, never used notion before)

GrayHoang avatar Aug 12 '25 19:08 GrayHoang

Alright, now that the scrim is passed and we are getting back into things, I'd like to wrap this one up before we get hella merge conflicts.

@Andrewyx do you mind doing a code review and determining if this will cause any major merge conflicts with your Bazel 8 PR? We should probably prioritize yours first, since I feel like mine is far simpler to debug than yours?

@williamckha something is happening with Pre-commit and FSM diagram generation, do you know where that stuff is run so I can try to fix it? I read through precommit.yml but don't see anything, and the command log isn't realyl telling me anything I understand

GrayHoang avatar Sep 07 '25 19:09 GrayHoang

@GrayHoang look at https://github.com/UBC-Thunderbots/Software/blob/master/scripts/fsm_diagram_generator.py

williamckha avatar Sep 07 '25 19:09 williamckha

I hope you are happy andrew. I am very sad.

GrayHoang avatar Sep 09 '25 23:09 GrayHoang

idk why the test is failing

GrayHoang avatar Sep 10 '25 22:09 GrayHoang

idk why the test is failing

Maybe try to push an empty commit. It happens sometime.

Mr-Anyone avatar Sep 10 '25 22:09 Mr-Anyone

Chat, I think the target is to get this done either this weekend or the next, so that our new interns can work with the new structure (and I don't have to deal with 50 merge conflicts).

GrayHoang avatar Sep 28 '25 23:09 GrayHoang

We should probably merge this soon

Mr-Anyone avatar Nov 09 '25 23:11 Mr-Anyone

What do you think about this @GrayHoang ? Is there a strong reason to use a packed parameter here?

diff --git a/src/software/ai/hl/stp/play/play_base.hpp b/src/software/ai/hl/stp/play/play_base.hpp
index 139b41ede..83c324ff3 100644
--- a/src/software/ai/hl/stp/play/play_base.hpp
+++ b/src/software/ai/hl/stp/play/play_base.hpp
@@ -8,7 +8,7 @@
  * @tparam PlayFsm the Play FSM to base this class off of (e.g. HaltPlay needs HaltFSM)
  * @tparam PlaySubFsms the sub FSMs this play uses (none use this yet)
  */
-template <class PlayFsm, class... PlaySubFsms>
+template <class PlayFsm>
 class PlayBase : public Play
 {
    public:
@@ -33,11 +33,11 @@ class PlayBase : public Play
     PlayFsm::ControlParams control_params;
 };
 
-template <class PlayFsm, class... PlaySubFsms>
-PlayBase<PlayFsm, PlaySubFsms...>::PlayBase(
+template <class PlayFsm>
+PlayBase<PlayFsm>::PlayBase(
     std::shared_ptr<const TbotsProto::AiConfig> ai_config_ptr, bool requires_goalie)
     : Play(ai_config_ptr, requires_goalie),
-      fsm{PlayFsm{ai_config_ptr}, PlaySubFsms{ai_config_ptr}...},
+      fsm{PlayFsm{ai_config_ptr}},
       control_params()
 {
 }

It compiles thunderloop.

Mr-Anyone avatar Nov 18 '25 22:11 Mr-Anyone

Does thunderscope compile as well? Iirc with the new refactor since every FSM class needs an ai_config_ptr you have to explicitly construct each SubFSM that a tactic uses (E.g. AttackerFSM uses a bunch of SubFSMs and if they are not properly initialized then the thing fails to compile.

The reason for the parameter packing is that it allows us to declare the FSMs that a tactic needs is dependent on (the direct FSM and any subFSMs) and initialize them with a general constructor, rather than having to declare a separate constructor for each Tactic.

GrayHoang avatar Nov 18 '25 23:11 GrayHoang

Thunderscope compiles, and you can try it I think.

It could be I am missing something?

Get Outlook for iOShttps://aka.ms/o0ukef


From: Grayson Hoang @.> Sent: Tuesday, November 18, 2025 3:33:24 PM To: UBC-Thunderbots/Software @.> Cc: Vincent @.>; Comment @.> Subject: Re: [UBC-Thunderbots/Software] Tactic and play refactor (PR #3481)

[https://avatars.githubusercontent.com/u/63363206?s=20&v=4]GrayHoang left a comment (UBC-Thunderbots/Software#3481)https://github.com/UBC-Thunderbots/Software/pull/3481#issuecomment-3549878687

Does thunderscope compile as well? Iirc with the new refactor since every FSM class needs an ai_config_ptr you have to explicitly construct each SubFSM that a tactic uses (E.g. AttackerFSM uses a bunch of SubFSMs and if they are not properly initialized then the thing fails to compile.

The reason for the parameter packing is that it allows us to declare the FSMs that a tactic needs is dependent on (the direct FSM and any subFSMs) and initialize them with a general constructor, rather than having to declare a separate constructor for each Tactic.

— Reply to this email directly, view it on GitHubhttps://github.com/UBC-Thunderbots/Software/pull/3481#issuecomment-3549878687, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AMVMSMELPVM3N44DVYFUVMD35OUEJAVCNFSM6AAAAACC6IVJRGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTKNBZHA3TQNRYG4. You are receiving this because you commented.Message ID: @.***>

Mr-Anyone avatar Nov 18 '25 23:11 Mr-Anyone

Ok I remember now, we don't technically need the template packing because none of the plays include other plays as subFSMs. However, I was building for the future, if we ever needed that kind of feature for something else (E.g. in the tactics).

GrayHoang avatar Nov 18 '25 23:11 GrayHoang

https://github.com/GrayHoang/Software/tree/T-and-P-Refactor-Static-Assert @Mr-Anyone Assuming I'm checking the thing properly, you can see that it doesn't build even if the static assert passes.

I understand the value in the static assert, as I can read the error log and know what it wants, but I would rather a more readable message get thrown first. However, I'm not sure how to setup the check, as the way you and I assumed would work doesn't seem to do anything

GrayHoang avatar Nov 19 '25 00:11 GrayHoang

https://github.com/GrayHoang/Software/tree/T-and-P-Refactor-Static-Assert @Mr-Anyone Assuming I'm checking the thing properly, you can see that it doesn't build even if the static assert passes.

I understand the value in the static assert, as I can read the error log and know what it wants, but I would rather a more readable message get thrown first. However, I'm not sure how to setup the check, as the way you and I assumed would work doesn't seem to do anything

Backtrace:

 no actions running
WARNING: For repository 'zlib', the root module requires module version [email protected], but got [email protected] in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 86 packages loaded
    currently loading: software/embedded/robot_diagnostics_cli ... (35 packages)
Loading: 86 packages loaded
    currently loading: software/embedded/robot_diagnostics_cli ... (35 packages)
Loading: 0 packages loaded
Found target //software/thunderscope:thunderscope_main with confidence 90
bazel run //software/thunderscope:thunderscope_main --copt=-O3 -- --disable_vsync --save_fps
INFO: Invocation ID: 08ed8a87-fc88-48f5-8e17-078045f145fa
INFO: Analyzed target //software/thunderscope:thunderscope_main (290 packages loaded, 29542 targets configured).
INFO: From Compiling upb/mini_table/extension_registry.c [for tool]:
external/protobuf+/upb/mini_table/extension_registry.c:85:1: warning: 'retain' attribute directive ignored [-Wattributes]
   85 | UPB_LINKARR_DECLARE(upb_AllExts, const upb_MiniTableExtension);
      | ^~~~~~~~~~~~~~~~~~~
INFO: From Compiling upb/message/copy.c [for tool]:
In file included from external/protobuf+/upb/message/accessors.h:16,
                 from external/protobuf+/upb/message/copy.c:17:
external/protobuf+/upb/message/copy.c: In function '_upb_Message_Copy':
external/protobuf+/upb/message/internal/accessors.h:309:10: warning: '*((void *)&ret+8)' may be used uninitialized in this function [-Wmaybe-uninitialized]
  309 |   return ret;
      |          ^~~
external/protobuf+/upb/message/internal/accessors.h:309:10: warning: '*((void *)&ret+8)' may be used uninitialized in this function [-Wmaybe-uninitialized]
external/protobuf+/upb/message/internal/accessors.h:309:10: warning: '*((void *)&ret+8)' may be used uninitialized in this function [-Wmaybe-uninitialized]
external/protobuf+/upb/message/internal/accessors.h:309:10: warning: '*((void *)&ret+8)' may be used uninitialized in this function [-Wmaybe-uninitialized]
external/protobuf+/upb/message/internal/accessors.h:309:10: warning: '*((void *)&ret+8)' may be used uninitialized in this function [-Wmaybe-uninitialized]
external/protobuf+/upb/message/internal/accessors.h:309:10: warning: '*((void *)&ret+8)' may be used uninitialized in this function [-Wmaybe-uninitialized]
INFO: From Compiling absl/strings/cord.cc:
In file included from external/abseil-cpp+/absl/container/inlined_vector.h:54,
                 from external/abseil-cpp+/absl/strings/internal/str_format/bind.h:24,
                 from external/abseil-cpp+/absl/strings/str_format.h:84,
                 from external/abseil-cpp+/absl/crc/crc32c.h:32,
                 from external/abseil-cpp+/absl/crc/internal/crc_cord_state.h:23,
                 from external/abseil-cpp+/absl/strings/cord.h:80,
                 from external/abseil-cpp+/absl/strings/cord.cc:15:
external/abseil-cpp+/absl/container/internal/inlined_vector.h: In function 'bool absl::lts_20250127::VerifyNode(absl::lts_20250127::Nonnull<absl::lts_20250127::cord_internal::CordRep*>, absl::lts_20250127::Nonnull<absl::lts_20250127::cord_internal::CordRep*>)':
external/abseil-cpp+/absl/container/internal/inlined_vector.h:411:28: warning: 'worklist.absl::lts_20250127::inlined_vector_internal::Storage<absl::lts_20250127::cord_internal::CordRep*, 2, std::allocator<absl::lts_20250127::cord_internal::CordRep*> >::data_.absl::lts_20250127::inlined_vector_internal::Storage<absl::lts_20250127::cord_internal::CordRep*, 2, std::allocator<absl::lts_20250127::cord_internal::CordRep*> >::Data::allocated.absl::lts_20250127::inlined_vector_internal::Storage<absl::lts_20250127::cord_internal::CordRep*, 2, std::allocator<absl::lts_20250127::cord_internal::CordRep*> >::Allocated::allocated_capacity' may be used uninitialized in this function [-Wmaybe-uninitialized]
  411 |     return data_.allocated.allocated_capacity;
      |                            ^~~~~~~~~~~~~~~~~~
INFO: From Compiling absl/strings/cord.cc [for tool]:
In file included from external/abseil-cpp+/absl/container/inlined_vector.h:54,
                 from external/abseil-cpp+/absl/strings/internal/str_format/bind.h:24,
                 from external/abseil-cpp+/absl/strings/str_format.h:84,
                 from external/abseil-cpp+/absl/crc/crc32c.h:32,
                 from external/abseil-cpp+/absl/crc/internal/crc_cord_state.h:23,
                 from external/abseil-cpp+/absl/strings/cord.h:80,
                 from external/abseil-cpp+/absl/strings/cord.cc:15:
external/abseil-cpp+/absl/container/internal/inlined_vector.h: In function 'bool absl::lts_20250127::VerifyNode(absl::lts_20250127::Nonnull<absl::lts_20250127::cord_internal::CordRep*>, absl::lts_20250127::Nonnull<absl::lts_20250127::cord_internal::CordRep*>)':
external/abseil-cpp+/absl/container/internal/inlined_vector.h:411:28: warning: 'worklist.absl::lts_20250127::inlined_vector_internal::Storage<absl::lts_20250127::cord_internal::CordRep*, 2, std::allocator<absl::lts_20250127::cord_internal::CordRep*> >::data_.absl::lts_20250127::inlined_vector_internal::Storage<absl::lts_20250127::cord_internal::CordRep*, 2, std::allocator<absl::lts_20250127::cord_internal::CordRep*> >::Data::allocated.absl::lts_20250127::inlined_vector_internal::Storage<absl::lts_20250127::cord_internal::CordRep*, 2, std::allocator<absl::lts_20250127::cord_internal::CordRep*> >::Allocated::allocated_capacity' may be used uninitialized in this function [-Wmaybe-uninitialized]
  411 |     return data_.allocated.allocated_capacity;
      |                            ^~~~~~~~~~~~~~~~~~
ERROR: /home/vhe/vscode/Software/src/software/ai/hl/stp/tactic/attacker/BUILD:3:11: Compiling software/ai/hl/stp/tactic/attacker/attacker_tactic.cpp failed: (Exit 1): linux_gcc failed: error executing CppCompile command (from target //software/ai/hl/stp/tactic/attacker:attacker_tactic) toolchains/cc/wrapper/linux_gcc -MD -MF bazel-out/k8-fastbuild/bin/software/ai/hl/stp/tactic/attacker/_objs/attacker_tactic/attacker_tactic.pic.d ... (remaining 482 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
In file included from ./software/util/sml_fsm/sml_fsm.h:4,
                 from ./software/ai/hl/stp/tactic/tactic_fsm.h:11,
                 from ./software/ai/hl/stp/tactic/tactic_base.hpp:7,
                 from ./software/ai/hl/stp/tactic/get_behind_ball/get_behind_ball_fsm.h:3,
                 from ./software/ai/hl/stp/tactic/chip/chip_fsm.h:3,
                 from ./software/ai/hl/stp/tactic/attacker/attacker_fsm.h:6,
                 from ./software/ai/hl/stp/tactic/attacker/attacker_tactic.h:3,
                 from software/ai/hl/stp/tactic/attacker/attacker_tactic.cpp:1:
external/+_repo_rules+sml/include/boost/sml.hpp: In instantiation of 'boost::ext::sml::v1_1_3::aux::missing_ctor_parameter<T>::operator TMissing&() const [with TMissing = KeepAwayFSM; typename boost::ext::sml::v1_1_3::aux::enable_if<(! boost::ext::sml::v1_1_3::aux::integral_constant<bool, __is_base_of(boost::ext::sml::v1_1_3::aux::pool_type_base, U)>::value), int>::type <anonymous> = 0; T = KeepAwayFSM]':
external/+_repo_rules+sml/include/boost/sml.hpp:389:117:   required from 'boost::ext::sml::v1_1_3::aux::pool<Ts>::pool(boost::ext::sml::v1_1_3::aux::init, const boost::ext::sml::v1_1_3::aux::pool<TArgs ...>&) [with TArgs = {AttackerFSM, DribbleFSM, PivotKickFSM}; Ts = {boost::ext::sml::v1_1_3::back::process<TacticFSM<PivotKickFSM>::Update>, boost::ext::sml::v1_1_3::back::process<TacticFSM<KeepAwayFSM>::Update>, AttackerFSM, DribbleFSM, PivotKickFSM, KeepAwayFSM, boost::ext::sml::v1_1_3::back::process<TacticFSM<DribbleFSM>::Update>}]'
external/+_repo_rules+sml/include/boost/sml.hpp:1650:122:   required from 'boost::ext::sml::v1_1_3::back::sm< <template-parameter-1-1> >::sm(TDeps&& ...) [with TDeps = {AttackerFSM, DribbleFSM, PivotKickFSM}; typename boost::ext::sml::v1_1_3::aux::enable_if<((sizeof... (TDeps) > 1) && boost::ext::sml::v1_1_3::aux::is_unique<boost::ext::sml::v1_1_3::aux::type<>, TDeps ...>::value), int>::type <anonymous> = 0; TSM = boost::ext::sml::v1_1_3::back::sm_policy<AttackerFSM, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> >]'
/usr/include/c++/10/bits/unique_ptr.h:962:30:   required from 'typename std::_MakeUniq<_Tp>::__single_object std::make_unique(_Args&& ...) [with _Tp = boost::ext::sml::v1_1_3::back::sm<boost::ext::sml::v1_1_3::back::sm_policy<AttackerFSM, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> > >; _Args = {AttackerFSM, DribbleFSM, PivotKickFSM}; typename std::_MakeUniq<_Tp>::__single_object = std::unique_ptr<boost::ext::sml::v1_1_3::back::sm<boost::ext::sml::v1_1_3::back::sm_policy<AttackerFSM, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> > >, std::default_delete<boost::ext::sml::v1_1_3::back::sm<boost::ext::sml::v1_1_3::back::sm_policy<AttackerFSM, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> > > > >]'
./software/ai/hl/stp/tactic/tactic_base.hpp:229:44:   required from 'std::unique_ptr<boost::ext::sml::v1_1_3::back::sm<boost::ext::sml::v1_1_3::back::sm_policy<T, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> > > > TacticBase<TacticFsm, TacticSubFsms>::fsmInit() [with TacticFsm = AttackerFSM; TacticSubFsms = {DribbleFSM, PivotKickFSM}]'
software/ai/hl/stp/tactic/attacker/attacker_tactic.cpp:38:53:   required from here
external/+_repo_rules+sml/include/boost/sml.hpp:353:53: error: static assertion failed: State Machine is missing a constructor parameter! Check out the `missing_ctor_parameter` error to see the missing type.
  353 |     static_assert(missing_ctor_parameter<TMissing>::value,
      |                                                     ^~~~~
external/+_repo_rules+sml/include/boost/sml.hpp: In instantiation of 'boost::ext::sml::v1_1_3::back::sm_impl< <template-parameter-1-1> >::sm_impl(const TPool&, boost::ext::sml::v1_1_3::aux::false_type) [with TPool = boost::ext::sml::v1_1_3::aux::pool<AttackerFSM, DribbleFSM, PivotKickFSM>; TSM = boost::ext::sml::v1_1_3::back::sm_policy<KeepAwayFSM, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> >; boost::ext::sml::v1_1_3::aux::false_type = boost::ext::sml::v1_1_3::aux::integral_constant<bool, false>]':
external/+_repo_rules+sml/include/boost/sml.hpp:1329:72:   required from 'boost::ext::sml::v1_1_3::back::sm_impl< <template-parameter-1-1> >::sm_impl(boost::ext::sml::v1_1_3::aux::init, const TPool&) [with TPool = boost::ext::sml::v1_1_3::aux::pool<AttackerFSM, DribbleFSM, PivotKickFSM>; TSM = boost::ext::sml::v1_1_3::back::sm_policy<KeepAwayFSM, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> >]'
external/+_repo_rules+sml/include/boost/sml.hpp:339:54:   required from 'boost::ext::sml::v1_1_3::aux::pool_type<T>::pool_type(boost::ext::sml::v1_1_3::aux::init, TObject) [with TObject = boost::ext::sml::v1_1_3::aux::pool<AttackerFSM, DribbleFSM, PivotKickFSM>; T = boost::ext::sml::v1_1_3::back::sm_impl<boost::ext::sml::v1_1_3::back::sm_policy<KeepAwayFSM, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> > >]'
external/+_repo_rules+sml/include/boost/sml.hpp:391:59:   required from 'boost::ext::sml::v1_1_3::aux::pool<Ts>::pool(const boost::ext::sml::v1_1_3::aux::pool<TArgs ...>&) [with TArgs = {AttackerFSM, DribbleFSM, PivotKickFSM}; Ts = {boost::ext::sml::v1_1_3::back::sm_impl<boost::ext::sml::v1_1_3::back::sm_policy<AttackerFSM, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> > >, boost::ext::sml::v1_1_3::back::sm_impl<boost::ext::sml::v1_1_3::back::sm_policy<DribbleFSM, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> > >, boost::ext::sml::v1_1_3::back::sm_impl<boost::ext::sml::v1_1_3::back::sm_policy<PivotKickFSM, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> > >, boost::ext::sml::v1_1_3::back::sm_impl<boost::ext::sml::v1_1_3::back::sm_policy<KeepAwayFSM, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> > >}]'
external/+_repo_rules+sml/include/boost/sml.hpp:1650:122:   required from 'boost::ext::sml::v1_1_3::back::sm< <template-parameter-1-1> >::sm(TDeps&& ...) [with TDeps = {AttackerFSM, DribbleFSM, PivotKickFSM}; typename boost::ext::sml::v1_1_3::aux::enable_if<((sizeof... (TDeps) > 1) && boost::ext::sml::v1_1_3::aux::is_unique<boost::ext::sml::v1_1_3::aux::type<>, TDeps ...>::value), int>::type <anonymous> = 0; TSM = boost::ext::sml::v1_1_3::back::sm_policy<AttackerFSM, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> >]'
/usr/include/c++/10/bits/unique_ptr.h:962:30:   required from 'typename std::_MakeUniq<_Tp>::__single_object std::make_unique(_Args&& ...) [with _Tp = boost::ext::sml::v1_1_3::back::sm<boost::ext::sml::v1_1_3::back::sm_policy<AttackerFSM, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> > >; _Args = {AttackerFSM, DribbleFSM, PivotKickFSM}; typename std::_MakeUniq<_Tp>::__single_object = std::unique_ptr<boost::ext::sml::v1_1_3::back::sm<boost::ext::sml::v1_1_3::back::sm_policy<AttackerFSM, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> > >, std::default_delete<boost::ext::sml::v1_1_3::back::sm<boost::ext::sml::v1_1_3::back::sm_policy<AttackerFSM, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> > > > >]'
./software/ai/hl/stp/tactic/tactic_base.hpp:229:44:   required from 'std::unique_ptr<boost::ext::sml::v1_1_3::back::sm<boost::ext::sml::v1_1_3::back::sm_policy<T, boost::ext::sml::v1_1_3::back::policies::process_queue<std::queue> > > > TacticBase<TacticFsm, TacticSubFsms>::fsmInit() [with TacticFsm = AttackerFSM; TacticSubFsms = {DribbleFSM, PivotKickFSM}]'
software/ai/hl/stp/tactic/attacker/attacker_tactic.cpp:38:53:   required from here
external/+_repo_rules+sml/include/boost/sml.hpp:1331:98: error: call of overloaded 'KeepAwayFSM(<brace-enclosed initializer list>)' is ambiguous
 1331 |   sm_impl(const TPool &p, aux::false_type) : sm_t{aux::try_get<sm_t>(&p)}, transitions_{(*this)()} {
      |                                                                                                  ^
In file included from ./software/ai/hl/stp/tactic/attacker/attacker_fsm.h:7,
                 from ./software/ai/hl/stp/tactic/attacker/attacker_tactic.h:3,
                 from software/ai/hl/stp/tactic/attacker/attacker_tactic.cpp:1:
./software/ai/hl/stp/tactic/keep_away/keep_away_fsm.h:22:14: note: candidate: 'KeepAwayFSM::KeepAwayFSM(std::shared_ptr<const TbotsProto::AiConfig>)'
   22 |     explicit KeepAwayFSM(std::shared_ptr<const TbotsProto::AiConfig> ai_config_ptr);
      |              ^~~~~~~~~~~
./software/ai/hl/stp/tactic/keep_away/keep_away_fsm.h:10:8: note: candidate: 'KeepAwayFSM::KeepAwayFSM(const KeepAwayFSM&)'
   10 | struct KeepAwayFSM : TacticFSM<KeepAwayFSM>
      |        ^~~~~~~~~~~
Target //software/thunderscope:thunderscope_main failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 139.844s, Critical Path: 52.00s
INFO: 1435 processes: 441 action cache hit, 1375 disk cache hit, 20 internal, 40 processwrapper-sandbox.
ERROR: Build did NOT complete successfully
ERROR: Build failed. Not running target
make: *** [Makefile:3: all] Error 1

I think you are right. It seems that this trips the assertion inside boost instead of the we want. I wonder if there is a nicer way to handle this; this is probably nature/challenge of c++ meta programming.

Mr-Anyone avatar Nov 19 '25 00:11 Mr-Anyone

It's weird because the static assert is handled before the boost construction is called, but I guess is_constructible_v considers well-formed rather than actually compilable? Technically I think it is well-formed, because the minimum number of arguments is passed, but boost::SML has some weird requirements on construction of FSM<TacticFSM> that is not evaluated until the object is actually constructed. Anyways, I've put this into the docs so someone in the future who reads docs should be able to understand the backtrace in shorter time.

GrayHoang avatar Nov 19 '25 01:11 GrayHoang