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

Navy separation #13605

Merged
merged 1 commit into from Sep 13, 2017

Conversation

Projects
None yet
5 participants
@forcecore
Contributor

forcecore commented Jul 8, 2017

Naval units are separated from the ground squad, just like aircraft is right now.
Naval units used to prevent ground units from moving as they can never move near the squad leader, which is a ground unit. (or maybe the other way round)

Depends on my other PR, IsEnemyUnit refactor.

@forcecore

This comment has been minimized.

Show comment
Hide comment
@forcecore

forcecore Jul 27, 2017

Contributor

Removed dependency to IsEnemyUnit refactor.

Contributor

forcecore commented Jul 27, 2017

Removed dependency to IsEnemyUnit refactor.

Show outdated Hide outdated OpenRA.Mods.Common/AI/States/NavyStates.cs
Show outdated Hide outdated OpenRA.Mods.Common/AI/States/NavyStates.cs
Show outdated Hide outdated OpenRA.Mods.Common/AI/States/NavyStates.cs
Show outdated Hide outdated OpenRA.Mods.Common/AI/States/NavyStates.cs
Show outdated Hide outdated OpenRA.Mods.Common/AI/States/NavyStates.cs
if (!owner.IsValid)
return;
GoToRandomOwnBuilding(owner);

This comment has been minimized.

@abcdefg30

abcdefg30 Aug 14, 2017

Member

Not sure if that is suitable for naval units, but ok.

@abcdefg30

abcdefg30 Aug 14, 2017

Member

Not sure if that is suitable for naval units, but ok.

This comment has been minimized.

@forcecore

forcecore Aug 14, 2017

Contributor

Yes, I think it is the best way to make the naval units flee when the SPEN/SYRD is gone.

@forcecore

forcecore Aug 14, 2017

Contributor

Yes, I think it is the best way to make the naval units flee when the SPEN/SYRD is gone.

Show outdated Hide outdated OpenRA.Mods.Common/AI/States/NavyStates.cs
Show outdated Hide outdated OpenRA.Mods.Common/AI/States/NavyStates.cs
Show outdated Hide outdated OpenRA.Mods.Common/AI/States/NavyStates.cs
Show outdated Hide outdated OpenRA.Mods.Common/AI/States/NavyStates.cs
@forcecore

This comment has been minimized.

Show comment
Hide comment
@forcecore

forcecore Aug 14, 2017

Contributor

I guess I'd wait for #13849 to be merged then come back.

Contributor

forcecore commented Aug 14, 2017

I guess I'd wait for #13849 to be merged then come back.

@forcecore

This comment has been minimized.

Show comment
Hide comment
@forcecore

forcecore Aug 18, 2017

Contributor

#13849 merged but now I'd wait for #13603, which looks quite close to being accepted, hopefully?

Contributor

forcecore commented Aug 18, 2017

#13849 merged but now I'd wait for #13603, which looks quite close to being accepted, hopefully?

@forcecore

This comment has been minimized.

Show comment
Hide comment
@forcecore

forcecore Aug 26, 2017

Contributor

Navy state separation is back, with GroundStates improvements merged.

Contributor

forcecore commented Aug 26, 2017

Navy state separation is back, with GroundStates improvements merged.

protected Actor FindClosestEnemy(Squad owner)
{
var first = owner.Units.First();

This comment has been minimized.

@Mailaender

Mailaender Aug 27, 2017

Member

I am not sure if caching this is safe. What if the unit is gone while it loops through those LINQ queries below?

@Mailaender

Mailaender Aug 27, 2017

Member

I am not sure if caching this is safe. What if the unit is gone while it loops through those LINQ queries below?

This comment has been minimized.

@forcecore

forcecore Aug 27, 2017

Contributor

Can it be gone? I believe one function is not interrupted while it is
running. NavyUnitsAttackMoveState/GroundUnitsAttackMoveState rely on a leader unit to pack in tight formations so if this is a problem, that part has such problem too.

@forcecore

forcecore Aug 27, 2017

Contributor

Can it be gone? I believe one function is not interrupted while it is
running. NavyUnitsAttackMoveState/GroundUnitsAttackMoveState rely on a leader unit to pack in tight formations so if this is a problem, that part has such problem too.

@Mailaender

This comment has been minimized.

Show comment
Hide comment
@Mailaender

Mailaender Aug 27, 2017

Member

We don't have good naval combat maps, but I tried this out on some of @wuschel's maps from http://resource.openra.net/maps/author/wuschel/ with bots allowed on the slots. This seems to work as advertised and is definitely and improvement.

Member

Mailaender commented Aug 27, 2017

We don't have good naval combat maps, but I tried this out on some of @wuschel's maps from http://resource.openra.net/maps/author/wuschel/ with bots allowed on the slots. This seems to work as advertised and is definitely and improvement.

@forcecore

This comment has been minimized.

Show comment
Hide comment
@forcecore

forcecore Aug 27, 2017

Contributor

Winter Storm is a good map to test even though it isn't a navy map. AI would build a naval yard or SPEN and its ground units get stuck without this PR's changes.

Contributor

forcecore commented Aug 27, 2017

Winter Storm is a good map to test even though it isn't a navy map. AI would build a naval yard or SPEN and its ground units get stuck without this PR's changes.

Separated ship squad from ground unit squad
Just like aircrafts are independent from ground squads.
@forcecore

This comment has been minimized.

Show comment
Hide comment
@forcecore

forcecore Sep 6, 2017

Contributor

Rebased, added domain index check

Contributor

forcecore commented Sep 6, 2017

Rebased, added domain index check

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Sep 13, 2017

Contributor

Looks good to me, and since the domain check (which works fine) was the only thing other than the Ships -> Naval rename that changed since @Mailaender's +1, I consider it still valid.

Contributor

reaperrr commented Sep 13, 2017

Looks good to me, and since the domain check (which works fine) was the only thing other than the Ships -> Naval rename that changed since @Mailaender's +1, I consider it still valid.

@reaperrr reaperrr merged commit 8027bed into OpenRA:bleed Sep 13, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr
Contributor

reaperrr commented Sep 13, 2017

@forcecore forcecore deleted the forcecore:NavySeparation branch Jan 5, 2018

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