-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Account for visibility when selecting AI superweapon targets. #14492
Conversation
@@ -125,7 +125,7 @@ public int GetAttractiveness(Actor a, Stance stance, Player firedBy) | |||
if (a == null) | |||
return 0; | |||
|
|||
if (!a.IsTargetableBy(firedBy.PlayerActor)) | |||
if (!a.IsTargetableBy(firedBy.PlayerActor) || !a.CanBeViewedByPlayer(firedBy)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it belongs in the third commit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
033eb68
to
36be975
Compare
36be975
to
863efda
Compare
Rebased. |
var left = Math.Min(a.X, b.X) / 1024; | ||
var top = Math.Min(a.Y, b.Y) / 1024; | ||
var right = (Math.Max(a.X, b.X) + 1023) / 1024; | ||
var bottom = (Math.Max(a.Y, b.Y) + 1023) / 1024; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why add an almost full cell here?
I assume this expects that the division and rounding will take care of the excess, while the extra value is to fill the cell since the caller uses CenterOfCell
? But we really should be able to handle that better.
863efda
to
b51f289
Compare
I was trying to avoid touching an existing |
b51f289
to
f2600ac
Compare
Updated. |
var br = world.Map.CellContaining(pos + delta); | ||
var checkFrozen = frozenLayer.FrozenActorsInRegion(new CellRegion(world.Map.Grid.Type, tl, br)); | ||
foreach (var scrutinized in checkFrozen) | ||
if (scrutinized.IsValid) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this missing a Visible
check, like the below method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, Visible
is checked inside GetAttractiveness
. The IsValid
check here is to make sure that Owner
is not null. I have added a comment explaining this.
The rest looks good 👍 |
f2600ac
to
cfb16a8
Compare
Rebased and added comment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
This fixes the single biggest complaint from casual players – that the TD airstrike always knows exactly the best target to hit. This also helps make the AI feel more like a real player, as it will fire the weapons where it think the enemy base is, which may no longer be accurate.
I have tested and tweaked TD's AI, but RA and D2K will need to be tested during review.
Adding to the milestone because this gives us a very nice user-facing feature to talk about in the news post.
Closes #7659.