Navigation: Improve error message for unreachable polygon case

[VewDev]

Change error message per the comments in #113913

Let me know if the message should be changed to anything else to be more descriptive.

Fun fact: The "reachable polygon not found" error message is in line 404 in both files 😆

[AThousandShips]

LGTM, thank you! Would be good to get an opinion from the navigation team that this is indeed a correct interpretation of the wording, but it should be

[MJacred]

To my understanding, this message would be sent every time an agent reached a point where they (most likely) cannot get any closer.
I reckon the message is sent in order to tell the dev: "Just so you know, your agent cannot get any closer than this. You really should test if your destination is reachable. And if it's not, don't keep asking your agent to go there. Then I won't spam you with more messages."
If navigation optimization is done, these messages would occur rarely.
The reason they cannot get closer might be because the max path search parameters restrict the agent too much. Or some navigation link is not compatible with the agent. Or the region got fractured into islands. Or start and end polygon are in different regions that are not connected at all.

[smix8]

The legacy naming does not do justice how severe this error case here is.

That code is only reachable after the loop had already a broken case where it runs past the point of exit it should take.

This new error msg is too mild and inaccurate and should not be merged like this.

If we want to rename this msg we really would need something closer to "Printer is on fire" to convey how severe this error is. A project that manages to trigger it has totally messed up navmesh geometry or placement, else it is impossible to reach it and likely their error log is already full from other stuff in that case.

The reason this msg is so archaic is because it is rarely reachable by end users. It is the kind of error that core devs face naturally when they tinker with the actual core code and mess the index or connections up on core changes.

A normal user would need to totally ignore all the connection merge errors in its project to be ever able to cause a case to reach that error so they have far more very important stuff to fix first before they should bother themself about this error here.

The msg for end users should basically be "dont' worry, fix your other error stuff and this will disappear automatically." and for core devs "congratulation, you likely f-ed the index or connection pointers up".

[VewDev]

In response to @smix8

A project that manages to trigger it has totally messed up navmesh geometry or placement

The msg for end users should basically be "dont' worry, fix your other error stuff and this will disappear automatically." and for core devs "congratulation, you likely f-ed the index or connection pointers up".

Would an error message like Invalid navigation index or connection pointers. Check preceding navmesh geometry or placement errors. or Unreachable navigation state encountered. Check preceding navmesh geometry or placement errors. be good?

If I were tinkering with core and got that error without the preceding errors, I would know I broke something. If I were just a "regular" user, I would check the preceding errors.

[VewDev]

Updated the message to reflect this.

[VewDev]

Should I squash these changes or something? Or will you just do a squash commit?

[AThousandShips]

Please squash the changes we do not use squash commits, thank you!

[VewDev]

Done. Ignore the closed/reopened, GitHub apparently didnt like me deleting my commits and making a new squashed one (it was a single line so I thought this would be the easiest way)

[Repiteo]

Thanks! Congratulations on your first merged contribution! 🎉