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

Fixed the AI not avoiding boats when searching for viable paradrop targets #7922

Closed
wants to merge 1 commit into from

Conversation

Mailaender
Copy link
Member

Tries to lessen #6636 with the adjusting screws we already got.

avoid paradropping into the Water
@@ -82,7 +82,7 @@ Player:
CheckRadius: 10c0
Consideration@2:
Against: Enemy
Types: Vehicle, Tank, Infantry, Defense
Types: Vehicle, Tank, Infantry, Defense, Water
Copy link
Member

Choose a reason for hiding this comment

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

These will only tell the AI to avoid boats, so I don't think it will fix #6636 in general?

Copy link
Member Author

Choose a reason for hiding this comment

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

I saw a lot of boats (some are high value cruisers) at #6636 (comment) so I suggest this will let the AI avoid them.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not convinced, so i'll need to see some evidence that this helps before giving a 👍.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is useful to not have the AI fight boats with paratroopers...

@Mailaender
Copy link
Member Author

Accidentally pushed to the wrong remote? (this is upstream).

This happens when you use the web interface.

@reaperrr
Copy link
Contributor

Edit: Nevermind, misunderstood something.

@Mailaender
Copy link
Member Author

No change has been suggested yet.

@pchote
Copy link
Member

pchote commented May 1, 2015

The changes requested tag was for

i'll need to see some evidence that this helps

We don't have a more appropriate PR tag to apply there.

@Mailaender
Copy link
Member Author

Go test the pull request then.

@pchote
Copy link
Member

pchote commented May 2, 2015

Just a philosophical/procedural comment: our merge policy has never been "merge by default" (see first point of the old Playtest or Release Checklist). It is up to the PR author to justify why their patch should be included, and so comments like that ("no, you prove that it shouldn't be merged") just leads to stale PRs that nobody wants to review.

@penev92
Copy link
Member

penev92 commented May 2, 2015

Even if this does not prevent the AI from ever sending paratroopers into water again, the discussion at #6636 and the YAML itself speak that this should prevent the AI from trying to use paratroopers to attack boats, which is a very good thing by itself.

@Mailaender
Copy link
Member Author

That is exactly my point.

I have no idea why you are trying to make me proof complex AI behavior. Also not in the mood for philosophical discussion over such simple fixes. If you don't have time to test the pull request, then ignore it, but don't leave FUD comments to block it permanently, @pchote.

@pchote
Copy link
Member

pchote commented May 3, 2015

this should prevent the AI from trying to use paratroopers to attack boats, which is a very good thing by itself.

The AI already doesn't use paratroopers to attack boats, though. The change here just tells it to actively avoid boats, which is redundant once somebody fixes the real problem.

@Mailaender
Copy link
Member Author

Boats are in the water. Avoid boats, avoid the water. I don't see the downside.

@pchote
Copy link
Member

pchote commented May 4, 2015

The downside is that this is a bandaid that doesn't address the real problem (avoiding boats doesn't mean avoiding water), and that the real fix (which would have taken significantly less time than has gone into this discussion) will make this proposed change redundant.

@Mailaender
Copy link
Member Author

Believe me, I am sick of this discussion, too. Feels like leading nowhere. What is "the real fix"?

@pchote
Copy link
Member

pchote commented May 5, 2015

Adding a terrain type check, either to the AI (still a bandaid, but at least it fully solves the bug) or support powers in general (which also covers #6145).

@penev92
Copy link
Member

penev92 commented May 21, 2015

While I do like the idea of having a proper solution through terrain type checks, which would help not only here, this would make things a little better, and terrain type checks are well out of scope here.
I'm going to 👍 this with the above arguments and for the sake of moving things along.

@pchote
Copy link
Member

pchote commented May 21, 2015

If this is going to be merged despite my objections, then please at least update the commit so that it doesn't claim to fix the paradrop into water problem.

@Mailaender
Copy link
Member Author

It doesn't really claim that. What is your suggestion for the commit message then? sigh I can't edit it in the GitHub web interface.

@pchote
Copy link
Member

pchote commented May 21, 2015

The commit message is currently "Update ai.yaml <newlines> avoid paradropping into the Water".

I suggest changing this to the more accurate "Don't paradrop units near boats".

@Mailaender Mailaender changed the title Fixed the AI sending paratroopers into the water Fixed the AI not avoiding boats when searching for viable paradrop targets May 21, 2015
@Mailaender Mailaender closed this May 23, 2015
@Mailaender Mailaender deleted the water-troopers branch May 23, 2015 14:05
@Mailaender
Copy link
Member Author

Filed #8222. Hope everyone is happy now.

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

Successfully merging this pull request may close these issues.

None yet

4 participants