<!--
> [!WARNING]
> **This is a DRAFT PR.**
> The structure is not definitive. The code might not be optimised yet.
It might not even start nor compile yet.
> **Don't panic. ✋ It's going to be ok. 👌 We can make modifications, we
can fix things.** 😁
-->
# Description
This PR aims to refactor the NextAction declaration to achieve two
goals:
## Eliminate C-style sentinel arrays
Currently, a double pointer (`NextAction**`) approach is being used.
This an old pre-C++11 (< 2011) trick before `std::vector<>` became a
thing.
This approach is painful for developers because they constantly need to
declare their `NextAction` arrays as:
```cpp
NextAction::array(0, new NextAction("foo", 1.0f), nullptr)
```
Instead of:
```cpp
{ new NextAction("foo", 1.0f) }
```
The first argument of `NextAction::array` is actually a hack. It is used
to have a named argument so `va_args` can find the remaining arguments.
It is set to 0 everywhere but in fact does nothing. This is very
confusing to people unfamiliar with this antiquated syntax.
The last argument `nullptr` is what we call a sentinel. It's a `nullptr`
because `va_args` is looking for a `nullptr` to stop iterating. It's
also a hack and also leads to confusion.
## Eliminate unnecessary pointers for `NextAction`
Pointers can be used for several reasons, to cite a few:
- Indicate strong, absolute identity.
- Provide strong but transferable ownership (unlike references).
- When a null value is acceptable (`nullptr`).
- When copy is expensive.
`NextAction` meets none of these criteria:
- It has no identity because it is purely behavioural.
- It is never owned by anything as it is passed around and never fetched
from a registry.
- The only situations where it can be `nullptr` are errors that should
in fact throw an `std::invalid_argument` instead.
- They are extremely small objects that embark a single `std::string`
and a single `float`.
Pointers should be avoided when not strictly necessary because they can
quickly lead to undefined behaviour due to unhandled `nullptr`
situations. They also make the syntax heavier due to the necessity to
constantly check for `nullptr`. Finally, they aren't even good for
performance in that situation because shifting a pointer so many times
is likely more expensive than copying such a trivial object.
# End goal
The end goal is to declare `NextAction` arrays this way:
```cpp
{ NextAction("foo", 1.0f) }
```
> [!NOTE]
> Additional note: `NextAction` is nothing but a hacky proxy to an
`Action` constructor. This should eventually be reworked to use handles
instead of strings. This would make copying `NextAction` even cheaper
and remove the need for the extremely heavy stringly typed current
approach. Stringly typed entities are a known anti-pattern so we need to
move on from those.
PlayerbotAI::FindOilFor was making the server randomly crashing
ChooseTravelTargetAction::getNewTarget: when active bot groupping was making the server crash as looking for unexisting params
Several bug fixes and tweak to Quest and Group
New fucntionnality:
Bots will now share quests randomly to their party
Bots will try to accomplish group member quest before moving on to new target
Bots will try to sells items only after few levels ( 5 ) when in group
When dropping a quest bots will try to select a new one they are on instead of idling for few time
Bots will no longuer try to invite themselfs to group or if group is full
Bots are now allowed to leave party by themself
Bots in groupe if not leader are forbbiden to tag in bgs
Bots in bot-groups no have a more limited range to look for grind target
Polish logs