What are the exact scenarios that triggered the crash?
I tested with master on the latest commit #2303, and on test-staging with the latest commit #2292.
Gave attack command to an addclass bot. No crash either way. With a target, they did attack it, without a target, they did nothing (so they did not say "I have no target").

It does actually make sense to move the flight block below IsInWorld. And in fact the definitions can go to below IsWithinLOSInMap as they are not checked before then. Regardless, it doesn't make sense that the current order would cause a crash in the way you described it. Something else is going on.

I tried with altbots only. Any time I issued a command to any bot to attack with a target selected, it was an immediate crash. I didn't actually try with no target selected.

I was on test-staging, plus various things I've changed, but none of it should have any relevance to the issue (e.g., boss strategies that apply only when the strategy is enabled, and I was not doing this in an instance). I was farther along on the Core than the Playerbots Core PR, but none of the changes I had beyond the PR seemed like they would have any relevance to this issue. Rebuilding didn't solve anything.

I tried reverting various recent commits, and the crash persisted, unless I moved up the target check (I didn't move up the world check at the time). If I removed the check, the crash would come back. The next day, I tried reverting 2288, and the crash disappeared even without moving the check, but then when I restored 2288 (still without moving the check), the crash was gone regardless.

For what it's worth, the call stack actually pointed to line 46 every time. But I couldn't figure out any concrete reason for why the crash was occurring.

And you absolutely have no custom commits on top? I just tried it with an alt on test-staging. Ordered it to attack nothing, to attack a hostile, to attack a friendly, to attack a dummy, to attack myself. No unexpected behaviour, and certainly no crash.

You might need to do a GDB core dump that hopefully can shed more light.
Also, how did you revert commits for the diagnosis? Did you do actual one-by-one reverts?
I do git checkout HEAD~X, where X is how many commits before the current last commit I want to revert the state of the branch. When done testing I go back with git checkout test-staging.

I do have custom commits. There's just nothing that seems like it could be remotely related to this, but I didn't test on a completely clean build. I use GitHub Desktop so I just reverted the commits through the list in the interface. I need to figure out a better way to handle switching branches so I don't have to spend 40+ minutes recompiling every time...

At the end of the day, it could have been some bad merge or pull or commit that I did. It's probably the most likely explanation. I just figured that even if the subject of this PR is not the root of the issue that I was experiencing, the target check should be moved up anyway. If that's not correct, then I'll close this.

No as I said before, the check arrangements you propose make sense anyway, and the bool definitions can be moved even further down to after IsWithinLOSInMap. But even if what's causing the crash on your end is because of some odd interaction with some custom commit you have, learning about the exact nature of that interaction can be useful.