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

Let AI consider fighter vs planet mechanics #1434

Merged
merged 8 commits into from Apr 6, 2017

Conversation

Projects
None yet
6 participants
@Morlic-fo
Contributor

Morlic-fo commented Mar 23, 2017

This PR introduces an additional rating_vs_planets which does not consider fighter stats for combat rating calculations.

Generally, all situations where we force combat in a system with enemy planets should check both the total fleet strength vs total enemy threat and fleet strength vs planets vs enemy planet threat.

There may be more places in the code which require those checks. The ones in this PR are all I found. In case more are found, it should be trivially to add the relevant checks...

@MatGB

This comment has been minimized.

Show comment
Hide comment
@MatGB

MatGB Mar 27, 2017

Member

K, @Dilvish-fo said to check and test this PR so I rebased it into my testing branch for a game I'd started earlier today, hadn't got to turn 100 yet. It's now turn 153 and I'm in trouble, two nearby AIs are clearly superior to me in tech and total military force: I have more colonies so I'll survive but it's the first time I've really felt challenged for months. Some of that is obviously due to the colonisation bug but a lot of it has to be this combined with other recent changes.

I haven't noticed anything specifically bad, colonies are definitely being built properly and I've yet to see a massive pile of troop ships waiting for a warship to turn up. I have seen some strange routes taken by AI ships but that could easily be reprioritising due to unseen-by-me threats. Ergo, my initial assessment is that this is a substantial improvement.

Also, that next cycle we need to make the planetary defence techs more expensive and split them up a bit because top tier defences set on Defend when you're still using Robohulls means dead fleets, and the nearest AI to me appears to have rushed them.

Member

MatGB commented Mar 27, 2017

K, @Dilvish-fo said to check and test this PR so I rebased it into my testing branch for a game I'd started earlier today, hadn't got to turn 100 yet. It's now turn 153 and I'm in trouble, two nearby AIs are clearly superior to me in tech and total military force: I have more colonies so I'll survive but it's the first time I've really felt challenged for months. Some of that is obviously due to the colonisation bug but a lot of it has to be this combined with other recent changes.

I haven't noticed anything specifically bad, colonies are definitely being built properly and I've yet to see a massive pile of troop ships waiting for a warship to turn up. I have seen some strange routes taken by AI ships but that could easily be reprioritising due to unseen-by-me threats. Ergo, my initial assessment is that this is a substantial improvement.

Also, that next cycle we need to make the planetary defence techs more expensive and split them up a bit because top tier defences set on Defend when you're still using Robohulls means dead fleets, and the nearest AI to me appears to have rushed them.

@Morlic-fo

This comment has been minimized.

Show comment
Hide comment
@Morlic-fo

Morlic-fo Mar 27, 2017

Contributor

Sounds good. Most of your observations are a cumulative effect over the last changes and probably not directly caused by this PR (always good it does not obviously break something, though).

That being said, the impact of this PR is hard to measure. It will mostly affect edge cases and those will (or: should) simply not happen anymore. Basically, as long as you do not see fighter-only fleets trying to attack planets, it seems to serve its purpose. But then again, hard to say if the AI would have otherwise without this PR...

Contributor

Morlic-fo commented Mar 27, 2017

Sounds good. Most of your observations are a cumulative effect over the last changes and probably not directly caused by this PR (always good it does not obviously break something, though).

That being said, the impact of this PR is hard to measure. It will mostly affect edge cases and those will (or: should) simply not happen anymore. Basically, as long as you do not see fighter-only fleets trying to attack planets, it seems to serve its purpose. But then again, hard to say if the AI would have otherwise without this PR...

@memem359

This comment has been minimized.

Show comment
Hide comment
@memem359

memem359 Mar 27, 2017

Contributor

Short version: Haven't noticed any problems, and at least one of the AI was behaving smarter.

About two weeks ago for a particular seed, the Chato_AI conquered a Mu Ursh system, but failed to colonize Good planets 1 jump from its homeworld.
After last weeks changes, the Chato_AI colonized those Good planets, but ignored the Mu Ursh.
With the build from today, including these suggested code changes, the Chato_AI colonized AND conquered. So whatever alchemy of decisions led to this point, it is definitely an improvement over a month ago.

I did notice one thing (spying with the Super-Tester and looking through the AI_log).
It is probably unrelated to these changes, but I'll mention it just in case.
For the Mu Ursh system, it had 26 defending troops. The Chato_AI created a goal of having 26 troops. (Seemed to be procrastinating on building them, but maybe it had other concerns.) It then eventually sent those 26 troops to invade, got the Mu Ursh to 0, then started building ships for 10 more troops. After those were built, it then invaded with all of them (overkill). I'd think that the AI should have requested more than 26 troops to start with, and/or not use so much troop overkill.

Contributor

memem359 commented Mar 27, 2017

Short version: Haven't noticed any problems, and at least one of the AI was behaving smarter.

About two weeks ago for a particular seed, the Chato_AI conquered a Mu Ursh system, but failed to colonize Good planets 1 jump from its homeworld.
After last weeks changes, the Chato_AI colonized those Good planets, but ignored the Mu Ursh.
With the build from today, including these suggested code changes, the Chato_AI colonized AND conquered. So whatever alchemy of decisions led to this point, it is definitely an improvement over a month ago.

I did notice one thing (spying with the Super-Tester and looking through the AI_log).
It is probably unrelated to these changes, but I'll mention it just in case.
For the Mu Ursh system, it had 26 defending troops. The Chato_AI created a goal of having 26 troops. (Seemed to be procrastinating on building them, but maybe it had other concerns.) It then eventually sent those 26 troops to invade, got the Mu Ursh to 0, then started building ships for 10 more troops. After those were built, it then invaded with all of them (overkill). I'd think that the AI should have requested more than 26 troops to start with, and/or not use so much troop overkill.

@Morlic-fo

This comment has been minimized.

Show comment
Hide comment
@Morlic-fo

Morlic-fo Mar 27, 2017

Contributor

then started building ships for 10 more troops.

If it was a single ship completely stacked with troops, then this is currently the expected (but obviously non-optimum) behaviour.

Contributor

Morlic-fo commented Mar 27, 2017

then started building ships for 10 more troops.

If it was a single ship completely stacked with troops, then this is currently the expected (but obviously non-optimum) behaviour.

@Morlic-fo

This comment has been minimized.

Show comment
Hide comment
@Morlic-fo

Morlic-fo Mar 27, 2017

Contributor

For the Mu Ursh system, it had 26 defending troops. The Chato_AI created a goal of having 26 troops.

Fix attempt: #1444

Contributor

Morlic-fo commented Mar 27, 2017

For the Mu Ursh system, it had 26 defending troops. The Chato_AI created a goal of having 26 troops.

Fix attempt: #1444

@Vezzra

This comment has been minimized.

Show comment
Hide comment
@Vezzra

Vezzra Apr 5, 2017

Member

Needs rebase.

Aside from this, anything preventing this from getting merged? @freeorion/ai-team?

Member

Vezzra commented Apr 5, 2017

Needs rebase.

Aside from this, anything preventing this from getting merged? @freeorion/ai-team?

@Morlic-fo

This comment has been minimized.

Show comment
Hide comment
@Morlic-fo

Morlic-fo Apr 5, 2017

Contributor

Rebased.

Contributor

Morlic-fo commented Apr 5, 2017

Rebased.

@Dilvish-fo

This comment has been minimized.

Show comment
Hide comment
@Dilvish-fo

Dilvish-fo Apr 6, 2017

Member

Good job, this looks to me like a great start, and playtests fine for me.

If you have a chance, some more comment lines particularly in MilitaryAI would be very helpful -- the way these allocation classes are set up it seems I have to do an awful lot of searching and jumping around to figure out just where and how ratings vs planets are being used.

I am also noting some ideas here about some points that I think would be most helpful to focus on for continued improvement in this area post-0.4.7, though perhaps we should take that ongoing discussion to the programming forum?

  • using the vs planets rating for defense planning as well as offense: this PR puts sets up the vs planets rating to be used for offensive planning, which was the biggest point of need, but the AI military planning would be more effective if we also took these issues into account for defense planning as well.

  • vs planets rating excluding fighters altogether: this seems like a reasonable first pass sort of estimate, which should be a conservative one for the offensive planning purposes where it is used in this PR. The planets can actually attack the fighters, though, even if the fighters cannot attack back, and so even in a pure ships vs planet battle the fighters should actually help the ship side a bit and it would be nice if we could more accurately capture that. (nm, as has been pointed out I misunderstood that part of the combat code)

  • Mixed Battle Assessment: with this PR the code takes into account both ships vs. ships and ships vs. planets; the handling for assessing ships vs a ships+planets force, if I am understanding it correctly, seems like a reasonable stopgap (setting a target rating as the max of (i) regular rating for total enemy forces or (ii) non-fighter rating vs just enemy planets rating), but still a rather rough estimate. I think these will probably be the most significant type of battles (the ones with the most empire resources at stake). Trying to figure out how to handle ratings for that kind of battle will be more tricky, but I expect it would be quite helpful.

Member

Dilvish-fo commented Apr 6, 2017

Good job, this looks to me like a great start, and playtests fine for me.

If you have a chance, some more comment lines particularly in MilitaryAI would be very helpful -- the way these allocation classes are set up it seems I have to do an awful lot of searching and jumping around to figure out just where and how ratings vs planets are being used.

I am also noting some ideas here about some points that I think would be most helpful to focus on for continued improvement in this area post-0.4.7, though perhaps we should take that ongoing discussion to the programming forum?

  • using the vs planets rating for defense planning as well as offense: this PR puts sets up the vs planets rating to be used for offensive planning, which was the biggest point of need, but the AI military planning would be more effective if we also took these issues into account for defense planning as well.

  • vs planets rating excluding fighters altogether: this seems like a reasonable first pass sort of estimate, which should be a conservative one for the offensive planning purposes where it is used in this PR. The planets can actually attack the fighters, though, even if the fighters cannot attack back, and so even in a pure ships vs planet battle the fighters should actually help the ship side a bit and it would be nice if we could more accurately capture that. (nm, as has been pointed out I misunderstood that part of the combat code)

  • Mixed Battle Assessment: with this PR the code takes into account both ships vs. ships and ships vs. planets; the handling for assessing ships vs a ships+planets force, if I am understanding it correctly, seems like a reasonable stopgap (setting a target rating as the max of (i) regular rating for total enemy forces or (ii) non-fighter rating vs just enemy planets rating), but still a rather rough estimate. I think these will probably be the most significant type of battles (the ones with the most empire resources at stake). Trying to figure out how to handle ratings for that kind of battle will be more tricky, but I expect it would be quite helpful.

@Vezzra

This comment has been minimized.

Show comment
Hide comment
@Vezzra

Vezzra Apr 6, 2017

Member

perhaps we should take that ongoing discussion to the programming forum?

That would be better. After all, the comment sections of PRs are meant for reviewing/discussing the proposed implementation. Although we go beyond that frequently enough, we should make some effort to keep that to a minimum. 😉

The planets can actually attack the fighters, though, even if the fighters cannot attack back

OUCH! Are you sure about that? Because the whole point of this restriction actually is that planets don't target fighters in combat, and waste their high damage shots on a myriad of low structure targets. Otherwise fighters would become the ultimate anti-planet weapon.

Of course that only works if fighters can't target planet also.

Meaning, if planets actually can and do target fighters in combat, that would be a very serious, release blocking bug.

@geoffthemedio? As the one who implemented the code in question, can you shed light on this?

Member

Vezzra commented Apr 6, 2017

perhaps we should take that ongoing discussion to the programming forum?

That would be better. After all, the comment sections of PRs are meant for reviewing/discussing the proposed implementation. Although we go beyond that frequently enough, we should make some effort to keep that to a minimum. 😉

The planets can actually attack the fighters, though, even if the fighters cannot attack back

OUCH! Are you sure about that? Because the whole point of this restriction actually is that planets don't target fighters in combat, and waste their high damage shots on a myriad of low structure targets. Otherwise fighters would become the ultimate anti-planet weapon.

Of course that only works if fighters can't target planet also.

Meaning, if planets actually can and do target fighters in combat, that would be a very serious, release blocking bug.

@geoffthemedio? As the one who implemented the code in question, can you shed light on this?

@MatGB

This comment has been minimized.

Show comment
Hide comment
@MatGB

MatGB Apr 6, 2017

Member

I haven't ever noticed a planet targetting a fighter and I've just checked through my most recent several turns and can't seen an instance, the game is at the stage where planets can actually still be firing in the second or third round of combat.

Of course, not seeing something happening doesn't mean it isn't/can't happen, but it shouldn't and if it is it needs fixing, at least until we rework planetary defences (because I do think planets should have fighter bases at some point and a ROF for planetary defences would be cool)

Member

MatGB commented Apr 6, 2017

I haven't ever noticed a planet targetting a fighter and I've just checked through my most recent several turns and can't seen an instance, the game is at the stage where planets can actually still be firing in the second or third round of combat.

Of course, not seeing something happening doesn't mean it isn't/can't happen, but it shouldn't and if it is it needs fixing, at least until we rework planetary defences (because I do think planets should have fighter bases at some point and a ROF for planetary defences would be cool)

@geoffthemedio

This comment has been minimized.

Show comment
Hide comment
@geoffthemedio
Member

geoffthemedio commented Apr 6, 2017

https://github.com/freeorion/freeorion/blob/master/combat/CombatSystem.cpp#L1174

Planets seemingly shouldn't be attacking fighters...

@Dilvish-fo

This comment has been minimized.

Show comment
Hide comment
@Dilvish-fo

Dilvish-fo Apr 6, 2017

Member

Ah, sorry, I had seen the function AttackPlanetFighter
https://github.com/freeorion/freeorion/blob/master/combat/CombatSystem.cpp#L469

and forgot to check that it was actually used (whereas I had checked that there is no AttackFighterPlanet that I could see) and so I got confused by that. So never mind that point, I have marked that part of my comment above with strikethru now to minimize confusion for future readers.

Member

Dilvish-fo commented Apr 6, 2017

Ah, sorry, I had seen the function AttackPlanetFighter
https://github.com/freeorion/freeorion/blob/master/combat/CombatSystem.cpp#L469

and forgot to check that it was actually used (whereas I had checked that there is no AttackFighterPlanet that I could see) and so I got confused by that. So never mind that point, I have marked that part of my comment above with strikethru now to minimize confusion for future readers.

@Dilvish-fo

This comment has been minimized.

Show comment
Hide comment
@Dilvish-fo

Dilvish-fo Apr 6, 2017

Member

ps., although now I checked, and AttackPlanetFighter is actually used, although apparently in a pathway that can't actually be triggered due to the line that Geoff pointed to, so perhaps the AttackPlanetFighter function deserves a comment noting that it is currently never triggered due to the line that Geoff noted.

Member

Dilvish-fo commented Apr 6, 2017

ps., although now I checked, and AttackPlanetFighter is actually used, although apparently in a pathway that can't actually be triggered due to the line that Geoff pointed to, so perhaps the AttackPlanetFighter function deserves a comment noting that it is currently never triggered due to the line that Geoff noted.

@Vezzra

This comment has been minimized.

Show comment
Hide comment
@Vezzra

Vezzra Apr 6, 2017

Member

Ah, ok. Good 😃

So, LGTM, right?

Member

Vezzra commented Apr 6, 2017

Ah, ok. Good 😃

So, LGTM, right?

@MatGB

This comment has been minimized.

Show comment
Hide comment
@MatGB

MatGB Apr 6, 2017

Member

Yup, it's working fine my end.

Member

MatGB commented Apr 6, 2017

Yup, it's working fine my end.

@Dilvish-fo

This comment has been minimized.

Show comment
Hide comment
@Dilvish-fo

Dilvish-fo Apr 6, 2017

Member

So, LGTM, right?

Sure, I had approved the code already, I just held off from merging in case Morlic wanted to add some more comments, but that could always be done later.

Member

Dilvish-fo commented Apr 6, 2017

So, LGTM, right?

Sure, I had approved the code already, I just held off from merging in case Morlic wanted to add some more comments, but that could always be done later.

@geoffthemedio

This comment has been minimized.

Show comment
Hide comment
@geoffthemedio

geoffthemedio Apr 6, 2017

Member

I wrote "seemingly shouldn't", not "don't" because it shouldn't but not necessarily can't happen.

Member

geoffthemedio commented Apr 6, 2017

I wrote "seemingly shouldn't", not "don't" because it shouldn't but not necessarily can't happen.

@Morlic-fo

This comment has been minimized.

Show comment
Hide comment
@Morlic-fo

Morlic-fo Apr 6, 2017

Contributor

I just held off from merging in case Morlic wanted to add some more comments, but that could always be done later.

I will update the comments by providing another PR / pushing to master directly.

Thanks for testing / review.

Contributor

Morlic-fo commented Apr 6, 2017

I just held off from merging in case Morlic wanted to add some more comments, but that could always be done later.

I will update the comments by providing another PR / pushing to master directly.

Thanks for testing / review.

@Morlic-fo Morlic-fo merged commit 7cb3de2 into freeorion:master Apr 6, 2017

2 checks passed

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

@Morlic-fo Morlic-fo deleted the Morlic-fo:CarrierVsPlanets branch Apr 6, 2017

@Morlic-fo

This comment has been minimized.

Show comment
Hide comment
@Morlic-fo

Morlic-fo Apr 6, 2017

Contributor

the handling for assessing ships vs a ships+planets force, if I am understanding it correctly, seems like a reasonable stopgap (setting a target rating as the max of (i) regular rating for total enemy forces or (ii) non-fighter rating vs just enemy planets rating), but still a rather rough estimate.

The intention was to use both:
(i) The attacking fleet must have a regular rating greater than the total defenses
AND
(ii) The attacking fleet must have a rating vs planets (i.e. rating without fighters) greater than the planetary defenses.

Both are required for fleet allocation:
https://github.com/freeorion/freeorion/pull/1434/files#diff-26ae651908f4d03ac27d4d28e5cac947R786

The maximum is considered here as the rating vs planets (rvp) is always smaller or equal to the actual rating. So if we need X rvp, then our allocation must have at least Y regular rating even if our regular rating is already greater than the total rating of the enemy force.
I do this so the estimation of the remaining rating after allocation as in these blocks is still more or less valid even if we do not require any more regular rating (e.g. having a large fleet of fighters vs planets)

But yes, generally all the combat ratings are somewhat crude estimates and should be improved at some point.

Contributor

Morlic-fo commented Apr 6, 2017

the handling for assessing ships vs a ships+planets force, if I am understanding it correctly, seems like a reasonable stopgap (setting a target rating as the max of (i) regular rating for total enemy forces or (ii) non-fighter rating vs just enemy planets rating), but still a rather rough estimate.

The intention was to use both:
(i) The attacking fleet must have a regular rating greater than the total defenses
AND
(ii) The attacking fleet must have a rating vs planets (i.e. rating without fighters) greater than the planetary defenses.

Both are required for fleet allocation:
https://github.com/freeorion/freeorion/pull/1434/files#diff-26ae651908f4d03ac27d4d28e5cac947R786

The maximum is considered here as the rating vs planets (rvp) is always smaller or equal to the actual rating. So if we need X rvp, then our allocation must have at least Y regular rating even if our regular rating is already greater than the total rating of the enemy force.
I do this so the estimation of the remaining rating after allocation as in these blocks is still more or less valid even if we do not require any more regular rating (e.g. having a large fleet of fighters vs planets)

But yes, generally all the combat ratings are somewhat crude estimates and should be improved at some point.

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