Fix a series of visibility/targeting bugs #9672
Conversation
penev92
force-pushed
the
fixAutoTarget
branch
from
a2d8e4f
to
523ea1b
Compare
| @@ -223,13 +223,29 @@ public IEnumerable<Armament> ChooseArmamentsForTarget(Target t, bool forceAttack | |||
| // (short-circuiting in the logical expression below) | |||
| var owner = null as Player; | |||
| if (t.Type == TargetType.FrozenActor) | |||
| owner = t.FrozenActor.Owner; | |||
| { | |||
| owner = t.FrozenActor.Actor.EffectiveOwner != null && t.FrozenActor.Actor.EffectiveOwner.Owner != null | |||
There was a problem hiding this comment.
This will crash if the actor is dead (t.FrozenActor.Actor == null). You need to check for that case and use t.FrozenActor.Owner instead.
|
|
||
| // Armaments are enumerated in attack.Armaments in construct order | ||
| // When autotargeting, first choose targets according to the used armament construct order | ||
| // And then according to distance from actor | ||
| // This enables preferential treatment of certain armaments | ||
| // (e.g. tesla trooper's tesla zap should have precedence over tesla charge) | ||
| var actrarms = inRange | ||
| var armamentForActor = inRange |
There was a problem hiding this comment.
actorByArmament would be more accurate.
There was a problem hiding this comment.
True, but it makes no sense :|
- Move a check for AutoTargetIgnore up the chain - Simplify some LINQs to a single .FirstOrDefault() - Rename local variable
penev92
force-pushed
the
fixAutoTarget
branch
from
523ea1b
to
189acb1
Compare
|
Updated. |
|
Confirmed this fixes being able to attack/auto-target cloaked structures. Spies can no longer be attacked/auto-targeted by normal units. Dogs can still attack and will auto-target disguised spies just fine. 👍 |
|
Also fixes #9351. |
| }) | ||
|
|
||
| .Where(kv => kv.Key != null && kv.Value.TraitOrDefault<AutoTargetIgnore>() == null) | ||
| .Where(kv => kv.Key != null && self.Owner.CanViewActor(kv.Value)) |
There was a problem hiding this comment.
Any reason why you added the filter here and not before or after the AutoTargetIgnore test in line 161?
There was a problem hiding this comment.
Performance. I wanted to do the visibility check as late as possible, because at that point chances are that most of the candidates have already been excluded.
There was a problem hiding this comment.
Right, sounds reasonable.
|
Would maybe fix #2959 as well? |
|
Also maybe #9470? |
penev92
force-pushed
the
fixAutoTarget
branch
from
189acb1
to
a69f26b
Compare
|
Fixed the frozen actors issue. |
|
Works now 👍 |
Fix a series of visibility/targeting bugs
The commit messages say it all.
Fixes #9667 - spies were totally broken.
Fixes #9157 - basically nobody cared about invisibility on structures.
P.S.: Also fixes #9351.
P.P.S.: Also fixes #2959.
And let's say it fixes #9470.