Skip to content
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.

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

Platoon movement #3134

Merged
merged 23 commits into from
Aug 24, 2020
Merged

Platoon movement #3134

merged 23 commits into from
Aug 24, 2020

Conversation

Garanas
Copy link
Member

@Garanas Garanas commented Jul 14, 2020

Introducing an alternative approach to move platoons around. The current approach forces the platoon to orient to the south at the end of every move order. The new approach either aligns them to the line between the current and next node (...ByFuture) or between the current and past node (...ByPast).

To showcase, see the following video:
https://www.youtube.com/watch?v=e4LH9oaKwTQ

The video becomes especially interesting about 30 seconds in, when the platoons are moving to the north.

From the outer to the inner line:

  • (new) platoon:MovePathOrientedByFuture(...)
  • (new) platoon:MovePathOrientedByPast(...)
  • (old) platoon:MoveToLocation(...)
  • (old) platoon:Patrol(...)

Especially platoon:MovePathOrientedByPast(...) (2nd from the outer one) is promising: it appears to be very smooth and is able to keep up with one of the older approaches that has a lower total distance. They finish at the same time.

I intend to make similar functions for:

  • IssueFormAggressiveMove(...)
  • IssueFormAttack(...)
  • IssueFormPatrol(...)

Waiting for feedback on the coding style, naming and / or the taken approach before I continue.

@shalkya
Copy link
Member

shalkya commented Jul 14, 2020

hello,
I've trouble replicating the issue of unit pointing to the south.
when you talk about platoon, it's something related about AI movement ?

Copy link
Member

@speed2CZ speed2CZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lua doesnt require ; at the end of the line

@Garanas
Copy link
Member Author

Garanas commented Jul 14, 2020

It happens when you tell a platoon to move through scripting. For platoons moved by players this behavior does not happen because you either implicitly or explicitly orientate the platoon as a player. For example, using the following in a script:

  • platoon:MoveToLocation(...)
  • platoon:Patrol(...)
  • IssueFormMove(...)

This is the example map from the video:

If you start the map, you should be able to observe the same behavior. Take note that the inner circles use the original behavior, the outer circles use the new behavior.

When you open up the campaign, and start the first mission on normal you'll see that the Percivals that run around your base show the same behavior: on each way point they first orientate themselves south before they continue the patrol.

Edit: I will remove all the semicolons.

@shalkya shalkya added this to the 3713 milestone Jul 14, 2020
…sivePath' and 'IssuePatrolPath' to match the IssueFormMove, IssueFormAggressiveMove and IssueFormPatrol commands which are available globally and used internally
@Garanas
Copy link
Member Author

Garanas commented Jul 14, 2020

Renamed the functions slightly to match convention of other functions. Implemented the patrol and aggressive move.

Another showcase:

From the outer to the inner platoon:

  • (new) platoon:IssueMovePath(...)
  • (old) platoon:MoveToLocation(...)
  • (new) platoon:IssuePatrolPath(...)
  • (old) platoon:Patrol(...)

The corresponding test map from the video:

I am going to do a read through of the comments tomorrow, remove semicolons that I introduced again by habit and look for other quirky things.

@Garanas
Copy link
Member Author

Garanas commented Jul 15, 2020

I cannot see anything to change or add further, suggestions are welcome. I've also checked whether functions such as CPlatoon:IsMoving(squad) and function CPlatoon:IsPatrolling(squad) correctly pick up that the platoon is moving / patrolling with the new approach. They do.

Copy link
Contributor

@Uveso Uveso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice Work.
Do you want this only for mapscripts or do you also want to patch Skirmish AI ?

If you do so, have in mind there are not only GPG ans Sorian AI using this.
Swarm AI, RNGAI and my AI. (AI-Uveso) will be affected by this.

lua/platoon.lua Outdated
-- check if we have a formation, IssueFormMove doesn't work if the formation argument is 'NoFormation'.
if formation == 'NoFormation' then
WARN('MovePathOrientedByPast: No platoon formation provided, defaulting to GrowthFormation.')
formation = 'GrowthFormation'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't force a formation, there are some platoons that are meant to move without formation.
(they move faster without a formation)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The functions IssueFormMove, IssueFormAggressiveMove, etc, do not work without a formation. I can add an if statement at the bottom to use IssueMove instead of IssueFormMove when no formation is provided.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking into it, how do you retrieve the desired formation of a platoon? I see the following line throughout platoon.lua:
local formation = self.PlatoonData.UseFormation or 'NoFormation

But that appears to be always nil. Calling LOG(repr(self)), which is the platoon in this context, doesn't show me a lot of information either.

lua/platoon.lua Outdated Show resolved Hide resolved
lua/platoon.lua Outdated Show resolved Hide resolved
@Garanas
Copy link
Member Author

Garanas commented Jul 18, 2020

That is a good question, if I would make these changes more 'global' in that sense: where would I start doing this? I haven't looked a lot into the code surrounding the AI's.

Edit: I just checked to be sure, platoons from AI's (of the base game from Steam) have the same issues.

@Uveso
Copy link
Contributor

Uveso commented Jul 19, 2020

If you want to see whats inside a table, you can't simply use repr() because inside classes are too many sub tables.
Use this function to only display the main entries inside a table:

function DebugArray(Table)
    for Index, Array in Table do
        if type(Array) == 'thread' or type(Array) == 'userdata' then
            LOG('Index['..Index..'] is type('..type(Array)..'). I won\'t print that!')
        elseif type(Array) == 'table' then
            LOG('Index['..Index..'] is type('..type(Array)..'). I won\'t print that!')
        else
            LOG('Index['..Index..'] is type('..type(Array)..'). "', repr(Array),'".')
        end
    end
end

then call it with DebugArray(self) or DebugArray(self.PlatoonData)

When we get this to work, then you don't need to implement this into the normal AI.
I will do it in my next AI game patch or at least share the information to impement this into
our custom AIs mods for testing.

@Garanas
Copy link
Member Author

Garanas commented Jul 20, 2020

With your function, it shows:

calling: LOG(repr(self)) in platoon.lua

NFO: Special debug:
INFO: Index[_c_object] is type(userdata). I won't print that!
INFO: Index[Trash] is type(table). I won't print that!
INFO: Index[PartOfAttackForce] is type(boolean). "\000false\000".
INFO: Index[PlatoonData] is type(table). I won't print that!
INFO: Index[CreationTime] is type(number). "\0000\000".
INFO: Index[EventCallbacks] is type(table). I won't print that!

With the LOG(repr()) it shows:

calling DebugArray(self) in platoon.lua

INFO: { <metatable=table: 1B285938>
INFO:   CreationTime=0,
INFO:   EventCallbacks={ OnDestroyed={ } },
INFO:   PartOfAttackForce=false,
INFO:   PlatoonData={ },
INFO:   Trash={ },
INFO:   _c_object=userdata: CScriptObject* at 1C350C68 = [CScriptObject at 0x1B2DE180]
INFO: }

Regardless of the information, the formation information is not in there.

The platoons (of the video) are made with:

local u1 = ScenarioUtils.CreateArmyGroup(army, "p4", false);
local p1 = brain:MakePlatoon('', '');
brain:AssignUnitsToPlatoon(p1, u1, 'Attack', 'GrowthFormation')

The formation is applied / given to a squad. Looking through the definitions of the platoon.lua and cplatoonlua files, I cannot seem to be able to retrieve a squad (and then, in turn, its formation).

I did find:
self:GetSquadUnits('Scout')
function CPlatoon:SetPlatoonFormationOverride(formation) (...) end

But that doesn't allow me to retrieve the formation.

Given this information: my code doesn't look for squads of any kind - should I add that in?
And perhaps add a formation parameter?

@Uveso
Copy link
Contributor

Uveso commented Jul 20, 2020

INFO: Index[_c_object] is type(userdata). I won't print that!
INFO: Index[Trash] is type(table). I won't print that!
INFO: Index[PartOfAttackForce] is type(boolean). "\000false\000".
INFO: Index[PlatoonData] is type(table). I won't print that!
INFO: Index[CreationTime] is type(number). "\0000\000".
INFO: Index[EventCallbacks] is type(table). I won't print that!```

My Debugarray only shows you the main or root values, as you can see there is a table called PlatoonData.
Now you need look into the PlatoonData table:
`DebugArray(self.PlatoonData)`
or
LOG( repr(self.PlatoonData) )



@Garanas
Copy link
Member Author

Garanas commented Jul 22, 2020

Writing the following:

LOG("debug: platoon")
DebugArray(self);
LOG("debug: platoon data")
DebugArray(self.PlatoonData)

LOG("debug: end of debugging")

Produces in the console:

INFO: debug: platoon
INFO: Index[_c_object] is type(userdata). I won't print that!
INFO: Index[Trash] is type(table). I won't print that!
INFO: Index[PartOfAttackForce] is type(boolean). "\000false\000".
INFO: Index[PlatoonData] is type(table). I won't print that!
INFO: Index[CreationTime] is type(number). "\0000\000".
INFO: Index[EventCallbacks] is type(table). I won't print that!
INFO: debug: platoon data
INFO: debug: end of debugging

The table is empty. That is why I was looking at C functions in the faf repository, perhaps the formation data is stored somewhere else.

Looking into it further shows the function:

-- inside platoon.lua
SetPlatoonData = function(self, dataTable)
    self.PlatoonData = table.deepcopy(dataTable)
end,

Given that the code from the repository attempts to use:

local PlatoonFormation = self.PlatoonData.UseFormation or 'NoFormation'
self:SetPlatoonFormationOverride(PlatoonFormation)

I started looking for where 'UseFormation' is defined, which is in the files:

  • fa\lua\AI\AIBuilders\AILandAttackBuilders.lua (18 occurences)
  • fa\lua\AI\AIBuilders\AISeaAttackBuilders.lua (5 occurences)
  • fa\lua\AI\AIBuilders\SorianLandAttackBuilders.lua (18 occurences)
  • fa\lua\AI\AIBuilders\SorianSeaAttackBuilders.lua (10 occurences)

These are 'Buildergroups', of which I am know they are related to platoons but unsure whether the data is being transferred to the platoon data table. If they are being transferred, then all the squads typically lose their original formation because they get overridden whenever this value is set.

In the GPG editor you can also define, or at least view, the platoon data manually. I am not sure whether that is being used given that one drop down mentions that it is being generated dynamically.

That being said, this would make the function worthless for map-related code unless you specifically define the platoon data on your platoon, to pass the formation. Which would be quite hidden. Therefore I propose the following:

--- Moves the platoon along the path, orientating at each node to match the line from the previous node to the current node.
-- @param self The platoon itself.
-- @param path A table of positions, preferably of type Vector. Converted otherwise.
-- @param opts A table of optional values, containing:
--        opts.First the index of the first node the path
--        opts.Last the index of the last node of the path
--        opts.UseFormation the formation to use for the platoon, overrides the formation of the squads
-- @return Table of commands.
IssueMovePath = function(self, path, opts)
    -- check for optional / default values
    local first = opts.First or 1 
    local last = opts.Last or table.getn(path)
    local formation = opts.UseFormation or self.PlatoonData.UseFormation or 'NoFormation'

    -- This is done elsewhere too after checking for self.PlatoonData.UseFormation. Should we do this here?
    -- self:SetPlatoonFormationOverride(formation) 

   -- (the rest of the function)
end

If you manually define a formation, that formation is used. This is useful when the function is called through a map-based script. If that information is not provided, the typical place for a formation is checked. This is useful for AI-related calls. And if there is no formation there either, then no formation is used at all. Down below, the function IssueMove is called instead of IssueFormMove when no formation is provided.

In the gpg-code the call for overriding the squad formations always follows after a manual formation is set. Should we do this here too, when we override the platoons formation as a whole?

That being said:

  • If a formation is provided to a squad, that formation is still unknown / not retrieveable with the current implementation
  • In general, should I implement moving a set of squads with similar behavior to CPlatoon:Patrol(position, [squad])?

…roposing a solution to the formation issue. Separated the generic code into because of the loss of generality after implementing the suggestions
@Garanas Garanas requested a review from Uveso August 8, 2020 14:52
@Garanas
Copy link
Member Author

Garanas commented Aug 8, 2020

@Uveso : I'd like to know what to add / change on the current code, or whether I can consider this to be done 😄 .

@Uveso
Copy link
Contributor

Uveso commented Aug 8, 2020

Hey Garanas :)
I already talked to keyser about this PR.
My girl is visiting me at this weekend so i will do the check on next Tuesday (11.August)

Copy link
Contributor

@Uveso Uveso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a newer test map available ?
I had to modify the old one to add the options table

lua/platoon.lua Outdated Show resolved Hide resolved
lua/platoon.lua Outdated Show resolved Hide resolved
lua/platoon.lua Outdated

-- base orientation when the angle is 0 for the function IssueFormMove
local base = Vector( 0, 0, 1 )
local direction = Utilities.GetDirectionVector(next, curr)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those function are called in a loop, please make them local for better performance
local GetDirectionVector = Utilities.GetDirectionVector

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Do you have an article as to why this is faster? I tried to find one myself, but wasn't able to

@Garanas
Copy link
Member Author

Garanas commented Aug 16, 2020

Link for the latest version of the map:

Went through all the changes Uvseo recommended. I decided to remove the first / last parameter because those operations can also be done outside this function. Also removed the formation parameter to make it fit more with how platoon (AI) functions are called generally in the code.

Added in some error handling. Should these be errors or warnings? I believe there is no stack trace with warnings.

If no more changes are required, then this is the final version.

Copy link
Contributor

@Uveso Uveso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The map is no longer available as download, if someone want to test this.
Overall everything is running.
Please change the functions to accept a formation override argument.
Also if possible, can you rebase the PR to the latest game version please ?

lua/ScenarioPlatoonAI.lua Outdated Show resolved Hide resolved
lua/platoon.lua Outdated Show resolved Hide resolved
lua/platoon.lua Outdated Show resolved Hide resolved
lua/platoon.lua Show resolved Hide resolved
lua/platoon.lua Show resolved Hide resolved
@Garanas
Copy link
Member Author

Garanas commented Aug 24, 2020

I'll do the changes again, I think its best if we sit down some day this week to finish it up with direct and quick communication!

On the map: I'll upload it to my drive instead of WeTransfer.

edit: This is the map:

@Uveso
Copy link
Contributor

Uveso commented Aug 24, 2020

Yes sure, but its already finished.
The platoon-override is for mission scripting and for Azraels, Relentors and my AI.
I already tested the implementation to the normal AI, but i will patch this to the default AI in one of the next AI patches.
It's a bit more work, since not all platoons uses PlatoonData.UseFormation.

@Uveso Uveso merged commit b12426e into FAForever:deploy/fafdevelop Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants