New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Formation #1

Open

cyberium wants to merge 23 commits into master from formation

Owner

cyberium commented Dec 26, 2021

🍰 Pullrequest

Proof

None

Issues

None

How2Test

None

Todo / Checklist

None

killerwife reviewed

View reviewed changes

src/game/AI/PlayerAI/PlayerAI.cpp

+                          charmer->GetFormationSlot()->GetFormationData()->Add(m_player);
+                      }
+                      else
+                          m_player->GetMotionMaster()->MoveFollow(charmer, PET_FOLLOW_DIST, PET_FOLLOW_ANGLE, true);

killerwife Dec 26, 2021 •

edited

Loading

Since there is multiple of these, maybe handle it in MoveFollow? Player based quasi formation also needs the same logic, when you lets say have army of the dead in wotlk or multiple guardians and hunter pet, they should all dynamically calculate angle.

Owner Author

cyberium Dec 26, 2021

Well first at this stage formation is a sub option for creaturegroup meaning that formation need a creature group absolutly and its designed only for creature as leader.
Thus some big change have to be done to be able to make a player as a master and more to have some other kind of group that will be attached to player.

Secondly you are asking for more than just adding it as exception of the movefollow we had a discussion about it with tobi and it seem that movefollow should even been replaced by some 'move in formation' with position computed depending of the player's followers.

killerwife reviewed

View reviewed changes

src/game/Chat/Level2.cpp Outdated

+                          SendSysMessage("Error: unable to retrieve the slot of the creature.");
+                  }
+                  SendSysMessage(gData->to_string().c_str());

killerwife Dec 26, 2021

Non standard way of things. Compiler will scream. Do PSendSys and format specifier %s and pass it as arg 1.

killerwife reviewed

View reviewed changes

src/game/Chat/Level2.cpp

+                      return false;
+                  }
+                  if (formationId >= static_cast<uint32>(SpawnGroupFormationType::SPAWN_GROUP_FORMATION_TYPE_COUNT))

killerwife Dec 26, 2021

primitives are best typecasted as c-style cast, since they arent really heresy. Curious choice of enum class, shauren loves them, buuuut historically in cmangos they seem out of place. Not really opposed to it tho.

killerwife reviewed

View reviewed changes

src/game/DBScripts/ScriptMgr.cpp Outdated

+                                  case 2: // SetFormation
+                                  {
+                                      break;

killerwife Dec 26, 2021

Brackets unnecessary with only break

killerwife reviewed

View reviewed changes

src/game/DBScripts/ScriptMgr.cpp Outdated

+                          {
+                              if (m_script->textId[0] == 1)
+                              {
+                                  auto respPos = creature->GetRespawnPosition();

killerwife Dec 26, 2021 •

edited

Loading

Shauren kindly explained to me not to use auto like this. I know what the type is, but anyone else reviewing wont. So in these cases dont use auto and put Position there. Its bad when doing it in github interface, i.e. bad for OS. I use it mainly for pairs, tuples, iterators and other templated nonsense.

Owner Author

cyberium Dec 26, 2021

Lazyness spotted you are correct

killerwife reviewed

View reviewed changes

src/game/DBScripts/ScriptMgr.cpp Outdated


		Creature* leader = static_cast<Creature*>(pTarget);

		auto groupData = leader->GetCreatureGroup();

killerwife Dec 26, 2021

Same point here, I have no fkin clue what it should be :D

killerwife reviewed

View reviewed changes

src/game/DBScripts/ScriptMgr.cpp Outdated

+                              if (LogIfNotCreature(pSource))
+                                  return false;
+                              CreatureGroup* grpData = nullptr;

killerwife Dec 26, 2021

grpData and later groupData, quite confusing variable choice

killerwife reviewed

View reviewed changes

src/game/DBScripts/ScriptMgr.cpp Outdated

+                          switch (m_script->formationData.command)
+                          {
+                          case 2:                         // set formation

killerwife Dec 26, 2021

missing indent

killerwife reviewed

View reviewed changes

src/game/DBScripts/ScriptMgr.cpp Outdated

+                                  break;
+                              }
+                              FormationEntrySPtr fEntry = std::make_shared<FormationEntry>();

killerwife Dec 26, 2021

Why does this need to be shared ptr at entry level and not at global level. Not inherently wrong just curious.

Owner Author

cyberium Dec 26, 2021

Hum yes maybe not needed too here

killerwife reviewed

View reviewed changes

src/game/DBScripts/ScriptMgr.cpp Outdated

+                                  break;
+                              }
+                              if (m_script->x <= 15)

killerwife Dec 26, 2021

This is a super odd way of using X. Use a different parameter. Do you really need a float?

Owner Author

cyberium Dec 26, 2021

yes i can make it x10 maybe in some data if need but will be even less intuitive

killerwife reviewed

View reviewed changes

src/game/DBScripts/ScriptMgr.h Outdated

@@ @@ -445,6 +447,13 @@ struct ScriptInfo @@
                           uint32 empty2;                                  // datalong2
                       } logKill;
+                      struct                                              // SCRIPT_COMMAND_MOVE_DYNAMIC (37)

killerwife Dec 26, 2021

Wrong comment im pretty sure

killerwife reviewed

View reviewed changes

src/game/Globals/ObjectMgr.cpp Outdated

+                          fEntry->Options = fields[4].GetUInt32();
+                          fEntry->Comment = fields[5].GetCppString();
+                          auto gItr = newContainer->spawnGroupMap.find(fEntry->GroupId);

killerwife Dec 26, 2021

just name it itr, its not like you have several iterators there.

killerwife reviewed

View reviewed changes

src/game/Globals/ObjectMgr.cpp

@@ @@ -1126,6 +1179,7 @@ void ObjectMgr::LoadSpawnGroups() @@
                           SpawnGroupDbGuids guid;
                           guid.Id = fields[0].GetUInt32();
                           guid.DbGuid = fields[1].GetUInt32();
+                          guid.SlotId = fields[2].GetInt32();

killerwife Dec 26, 2021

-1 default?

Owner Author

cyberium Dec 26, 2021

yes we will have to talk about that

killerwife reviewed

View reviewed changes

src/game/Maps/SpawnGroupDefines.h

+                  SPAWN_GROUP_FORMATION_TYPE_FANNED_OUT_IN_FRONT = 5,
+                  SPAWN_GROUP_FORMATION_TYPE_CIRCLE_THE_LEADER   = 6,
+                  SPAWN_GROUP_FORMATION_TYPE_COUNT               = 7

killerwife Dec 26, 2021

The 8th type missing here maybe? Even adding a placeholder would be cool even if its not implemented.

Owner Author

cyberium Dec 26, 2021

what is it?

killerwife reviewed

View reviewed changes

src/game/MotionGenerators/MotionMaster.h

@@ @@ -74,8 +75,39 @@ enum MovementGeneratorType @@
                   EXTERNAL_WAYPOINT_MOVE          = 17,                   // Only used in UnitAI::MovementInform when a waypoint is reached. The pathId >= 0 is added as additonal value
                   EXTERNAL_WAYPOINT_MOVE_START    = 18,                   // Only used in UnitAI::MovementInform when a waypoint is started. The pathId >= 0 is added as additional value
                   EXTERNAL_WAYPOINT_FINISHED_LAST = 19,                   // Only used in UnitAI::MovementInform when the waittime of the last wp is finished The pathId >= 0 is added as additional value
+                  FORMATION_MOTION_TYPE           = 20,                   // TargetedMovementGenerator.h

killerwife Dec 26, 2021

How will it work in creature table, in DB it will be still 2?

Owner Author

cyberium Dec 26, 2021

That have nothing to do with defaut movement type in db.
Its only for movement generator internally and used for follower i should rename it.

cyberium added 15 commits

January 3, 2022 10:00


          Formation code for spawngroup

49dc096


          Improve waypoint movegen

1d2e4b6

Fiw last reached point to previous point after calling
WaypointMovementGenerator<Creature>::SetNextWaypoint(uint32 pointId)


          Some improvement to formation

916a121

Add script command add creature, remove creature, switch formation type
Handle adding and removing player from/to formation (in case of possessning)
Fix a couple of issues


          Enforce movement_template table for spawngroupÃ

50d04ab


          Change script command to group target instead of formation

53e9a80


          Add possibility to assign dynamically a formation to a creature group

2dc7e33

Add a db script command to make it possible
```
datalong = command(2 for formation create)
datalong2 = group id to send this command (0 mean remove the formation of current group)
int[0] = type
int[1] = options
int[2] = MovementType
int[3] = MovementID
spread = x
```


          Fix a little mistake npc formation switch command error message

93ee4ad


          Improve a bit the npc formation info command

0914fb7


          Removing some unneeded ingame command

b3a9b86


          Some more cleaning to formation code

5ccfc7a


          Set keep compact option from formation entry

1858c31

also a bit more cleaning


          Temp hack to fix movetype for static group formation

b95981b


          Merge FormationData constructors

89a2ed5

Better to avoid problems with forgetted variables initialization and duplicate code


          remove duplicate code in FormationMovementGenerator::Update

4adab4d

So normal update will be called


          Add change spread and change option script command

2f1930e

a bit more cleaning and soma error text fixes

cyberium added 6 commits

January 3, 2022 10:00


          some code style

8c8bb06


          Use better method to get best position for followers

10d2067

Remove some of the  custom code not needed anymore


          Implement followers check to master distance

ebc7e0d

So if master respawn very far from its followers they will get teleported or run to it.

also fix formation shape reset after master respawn


          Fix movement type for formation in db

3148be1

We should take care of how we handle movement in core and generalize the fact that movement id should not be tied to an entry or a guid. So everything in creature_movement or creature_movement_template should be moved to new movement_template table.


          Prepare for release

c489723

Specify the change for command MOVE_TO that will now support move to respawn pos for the creature itself or its entire group.


          Prepare SCRIPT_COMMAND_SPAWN_GROUP(51) for release

5c5126b

Only formation code in it for now
Ability to set formation shape, spread and options via script

cyberium force-pushed the formation branch from fd69029 to 62fd840 Compare

January 3, 2022 14:22

cyberium added 2 commits

January 3, 2022 23:47


          [s2439] Add Formation sql

803a198


          Last fixes for release

3db02e9

cyberium force-pushed the formation branch from 62fd840 to 3db02e9 Compare

January 3, 2022 23:02

cyberium force-pushed the master branch 3 times, most recently from 58d70e8 to 5c321aa Compare

February 14, 2022 18:23

cyberium force-pushed the master branch from 5c321aa to ddaa67b Compare

May 28, 2022 16:19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet