Make escorts only pathfind through systems that the player has visited.

[quyykk]

Bugfix: This PR fixes #6096

Fix Details

I made AI aware of PlayerInfo so that it knows which systems the player has visited or not. DistanceMap got a new constructor so that you can add a PlayerInfo to its pathfinding for a ship that is not the flagship.

Now escorts will not travel through systems that the player hasn't visisted.

Testing Done

I used the save file to reproduce the issue and fix it.

Save File

The save file from the linked issue above.

[quyykk]

Is this solution/PR not wanted? I mean I get that passing a PlayerInfo to AI is not pretty but I'm not really seeing an alternative. Maybe adding a way for GameData to access HasSeen/HasVisited but that's not exactly better either. Or is this fix not wanted? Basically I need some form of feedback :)

[Zitchas]

In my experience, the Devs are pretty quick to slap a "unlikely" label on stuff they don't think will get into the game. And usually say something to that effect. And Amazinite has put the mechanics label on it, which tends to indicate stuff that they want to work on. So it's most likely that they just haven't been able to get to it yet; or haven't felt that they have the time to really dig into this as of yet.

[pepoluan]

The game hasn't already done this??

Ahaha, I thought the game already did this, which is why whenever a new area opened up I rode solo through the space using a JD+HD combo (to save fuel) to map systems in the area...

[petervdmeer]

Nice; the actual pathfinding code was already there; the only change that was needed was to pass the PlayerInfo for player-owned escorts.

[tehhowch]

What should we do about a hyperdrive-only escort that has no known route to the flagship system because the player used their JD to get there and didn't buy maps?

Should the player get a message every time they take off that they need to go back / explore sideways?
Should we provide a new escort pip color in the map to highlight "stranded" escorts, and a tooltip as well?
Should we let the escort ship explore on its own, perhaps adding the systems it visits to the player's "seen" systems?
Should we let the escort ship "cheat" and take the shortest path between known paths?
Should we restrict the map view to known-accessible-via-hyperspace-lane-only systems, placing an information "fog-of-war" on star clusters not accessible by hyperlink from the player's current system (no trade info, etc.), completely cutting the player off from their stranded escorts?

[tehhowch]

Probably we should refactor DistanceMap to take an optional set of known systems, rather than a player object.

(We can do that in a sooner-rather-then-later follow-up though.)

[quyykk]

We need the player object as well to check if a wormhole was visited by the player.

[tehhowch]

That's required by the current implementation, yes. If we're passing in the knowledge of usable systems, passing in the knowledge of usable planets/wormholes is probably expected.

[quyykk]

What should we do about a hyperdrive-only escort that has no known route to the flagship system because the player used their JD to get there and didn't buy maps?

We already have that problem: What happens currently if the flagship system has no links? The same behavior should happen in this case. I think the escort just stays in the system they were in. I think that's fine.

Should the player get a message every time they take off that they need to go back / explore sideways?

No. But we can maybe add something to the escort info display on the lower right to indicate that the escort is stranded.

Should we provide a new escort pip color in the map to highlight "stranded" escorts, and a tooltip as well?

I think that's a good idea.

Should we let the escort ship explore on its own, perhaps adding the systems it visits to the player's "seen" systems?

No, that would be weird.

Should we let the escort ship "cheat" and take the shortest path between known paths?

Nope.

Should we restrict the map view to known-accessible-via-hyperspace-lane-only systems, placing an information "fog-of-war" on star clusters not accessible by hyperlink from the player's current system (no trade info, etc.), completely cutting the player off from their stranded escorts?

Maybe, maybe not, but in any case I think that's for another PR if we were to do that.

[petervdmeer]

"Waiting on OP" due to open review-comments and unresolved merge-conflict.

[tehhowch]

Last thing for this PR

[tehhowch]

How permanent shall this field be?

If "very," then we should remove the PlayerInfo parameter from the non-static class methods.

If "not very," we should add a TODO: remove this field. comment.

[quyykk]

@tehhowch I'm having a bit of trouble trying to avoid the player field. I would need to pass around 3 sets (seen systems, visited systems, visited planets) around to avoid using player, which isn't that great imo.

So, what would you recommend?

[Amazinite]

Going through PRs older than three months to find PRs that could be closed.

Closing this PR because Quyykk himself has said that he's not getting to it soon and that he doesn't think it's too important of a change. I'll also note that there has been some discussion of reviving #5979, which would conflict with this change and cause player escorts to become stuck in shrouded systems because of those system being removed from the visited list. If we are to make both changes, then each change needs to keep the other in mind.

[Zitchas]

Just a gentle nudge that this - while not a priority - would be much appreciated fix for escort behavior.

[ziproot]

It looks like this is waiting on reviewers. There are no merge conflicts and no unaddressed review comments as far as I can tell.