Pathfinder takes into account some undetected enemy units

[ObviousTech]

0.41.26 Double blind enabled.

When moving, attempt to plot a move to/through a hex that an unseen, undetected enemy unit is. Normally, Megamek does not take this enemy unit into account when pathfinding (which is correct). However, the attached save has an instance of an unseen, undetected affecting pathfinder.

If the move tries to end in the illegal hex, the number in the final hex turns gray. If the move tries to go through the enemy unit, a path is plotted around that unit. This gives information to the player that it should not have (and prevents moves which should be allowed to be attempted).

In the attached save, connect as "Pirates". There are enemy mechs in 1739, 1934, and 1935. Attempt to move VL-2T Paula Yi in hex 1938 to hex 1935. Megamek will not let you do so. The movement number is gray (should be yellow for running).
Attempt to move to 1933 or 1934. Megamek plots a course around the tunnels to the west instead of the obvious direct route. This is especially strange because there is another enemy mech in 1739 which doesn't block the pathfinder.

Normally when you attempt to move into/through an enemy unit, Megamek lets you enter that move and then your unit's movement gets stopped short. I'm not sure what is special in this case. Megamek correctly lets you attempt to move through the mech in 1739 and stops you in 1839, for example.

My guess is something is strange about the status of the mech in 1935 that is causing the pathfinder to count its hex as blocked.

movevulcan.sav.gz

[SJuliez]

The Griffin-2N in 1739 has an ECM. Because the Vulcan is in ECM range the server intentionally transmits the Griffin to the client (so the client can report things correctly, see Server.filterEntities). There, Compute.isEnemyIn reports a stacking problem, which makes the move illegal.

I have two questions:

• The vulcan can only seem to jump 2 or 3 hexes and I can't see why at all. Am I missing something? Does a jumping unit get stopped short by an enemy unit that turns visible? Or is this some setting?
• When I deactivate the stacking checker, the Vulcan can sprint through the units in 1935 and 1934 to end up in 1933. Would that be a correct move?

[SJuliez]

Okay, guess I'll answer b) myself... Moving through enemy units isn't ok.

[arlith]

The issue here is going to be that the client knows about an enemy unit that is undetected, because it needs to know about the ECM, but the player should not know about this entity. The problem is that Game (which stores the list of entities) doesn't know nor care about visibility/detection. When the path finder runs its check, it just uses the whole list of entities, and ultimately this is wrong.

The solution to this problem is going to be tricky, I think. It's basically going to require having some way for the Client to forget about units that the player doesn't know about, but the Client still does. I can foresee this being fairly complicated, and tricky.

One solution might be to temporarily manipulate the Game before the path is planned, removing any undetected/unseen entities, but this seems kludgey and prone to error.

Another approach might be to add some checks for this situation, but that's also error prone. For instance you could imagine updating the stacking check in Compute to ignore unseen/undetected units, however this should only be done from the Client perspective, and not Server. This also seems like it'd be easy to screw up and likely will require adding the check in lots of different places.

[SJuliez]

Yes, I was just looking at Compute.isEnemyIn and imagining its twin, isSeenEnemyInClientSide. An alternative would be generating a second (cleaned) entity list that is only used for movement calculation. I don't like that though because the code would be just as long as a second Compute method but much more obscure.

[Dylan-M]

Isn't there a way we can do this on the entity itself with a flag that it's visible? We track this info already, it's how the board view displays your units as semi-transparent with the H or U icons for hidden and undetected respectively.

[SJuliez]

The information is indeed available. Currently the server and client can share a single method to check stacking because the client usually doesn't even know about hidden enemy units. But in that special case of an ECM enemy mech with a friendly one in ECM range the client does know that ECM enemy mech and so the stack checker finds it (although it isnt displayed).

[Dylan-M]

Right, but the stack checker could check for visibility on the unit, and then ignore it if it's invisible. Which seems to be what the rules say.

[arlith]

No, it can't ignore it if it's invisible. You can't stack units, regardless of visibility (hidden or not). That's kind of the issue. The Client should allow it, because technically the player doesn't know about the unit, but the Server cannot allow it, otherwise the movement would be processed/allowed.

[Dylan-M]

Exactly what I mean, ignore it on the client side only. Then generate the proper stacking violation in movement when it's attempted, the same as any other hidden unit.

[arlith]

I'm pretty sure the easiest way to fix this would be to cull the Game entityList on the client-side before doing the path computation, and then restore the culled units afterwards. This would ensure that all computations during path finding are correct, without having to adjust a lot of code.

[SJuliez]

Is this all one-threaded? So, is this safe to do?

[arlith]

Exactly what I mean, ignore it on the client side only. Then generate the proper stacking violation in movement when it's attempted, the same as any other hidden unit.

I suspect that the stacking check isn't the only cause where this needs to be considered. Making sure we've got all of the different areas where this could matter could be difficult, plus it could require touching a fair amount of code.

[arlith]

Is this all one-threaded? So, is this safe to do?

Yes, for now. In theory, it shouldn't be, but right now everything happens on the AWT Event Dispatch thread.

edit: this hints at a bigger issue, that Game itself isn't thread safe. It would be really convenient if it were, but right now it is not and changing that I think is a lot of effort.

[Dylan-M]

If we're going to do it by culling the client's Game entityList, and then restore it, what method are you thinking of for restoring it? Deep copy it to a second list "originalEntityList", and then copy back when done? Or are you going to request it from the server?

[Dylan-M]

To wit: I think caching it via a deep copy is probably the better way, but I don't really know.

[SJuliez]

Wouldnt it be easier in this case to just keep a culled copy (generate it when receiving the list)?

[SJuliez]

And hmmmmmmm (deep thinking), doesnt that have the exact same chance for error than adding a different check?

[NickAragua]

Realistically speaking, it's not too much of a stretch to give the pathfinder an "ignore hidden units" parameter, which it can then pass to the MovePath as its generated to get it to ignore hidden unit collisions.

[Dylan-M]

Yes, yes it would. I didn't even think of that.

EDIT: Take that to respond to @SJuliez's comment since Nick chimed in just as I was posting.

[arlith]

Yea, I don't know that the current API supports doing this well, but from memory there's a Game.setEntityVector or something like that, which is used by the Client when it receives a definitive list of entities from the server. I think this could be used without triggering the Entity add/remove events that you would want to avoid.

[arlith]

Realistically speaking, it's not too much of a stretch to give the pathfinder an "ignore hidden units" parameter, which it can then pass to the MovePath as its generated to get it to ignore hidden unit collisions.

The problem is, it's not just the path finder that needs to be changed, otherwise, I'd agree. At some point, the path finder will use things like Compute.isStackingValid (or whatever it's called). I think this amounts to the same approach @Dylan-M was advocating.

edit: not to say that I disagree with that approach. I just think it'll involve more code changes, which doesn't make it wrong.

[SJuliez]

But @arlith just to be sure: Doesn't this have the same exact chance for error than using a different method that filters hidden units?

[SJuliez]

We'll have to carefully check the circumstances where IsEnemyIn is used in either case.

[arlith]

But @arlith just to be sure: Doesn't this have the same exact chance for error than using a different method that filters hidden units?

Entity already has state that knows whether it should be shown or not (there may be code for this in EntitySprite as well, don't know if that ever got pulled into Entity). We know that the code used to decide of the sprite should be displayed or not is good, otherwise we'd see units we shouldn't and there are no reports of that anymore.

[arlith]

Ultimately, this is a consequence of the changes I made for the client to receive these units. Initially, I only thought it mattered for display purposes. I never considered this case with the path finder.

[SJuliez]

OK, so rely on visibility information from the entity. Sounds good. Would be consistent.

[arlith]

I would definitely check EntitySprite to see how it handles determining when to show the unit or not, because it's a bit more complicated, I think (but I don't recall the exact details).

[SJuliez]

So, @Dylan-M already had the right idea 15 minutes ago :)

[arlith]

As long as you have a flag that indicates whether it's an authoritative check (like from the server) that should ignore visibility, or a non-authoritative check (like from the client doing the path finder; I don't know that the client should always be non-authoritative though) which should consider visibility.