Remove mission search radius from most missions

[RenechCDDA]

Summary

None

Purpose of change

"Failed to assign mission target <name_of_OMT>"

We let our searches purposefully fail due to putting in a too-low number in json. Why?

There is absolutely no reason to let these searches fail on purpose. But for some reason, instead of just searching as far as reasonably possible, we let them fail. Not only that, but we leave it up to the JSON to determine just how often we want to purposefully fail, with wildly varying results. Why?!

Most of them are just cargo culted, and didn't understand what they were doing. Well let's lock it down.

Describe the solution

Guard against people accidentally setting a lower search range by enforcing random. If it's not random, it'll always search closest first and then you never want to stop the search early.

Upgrade the default search range from OMAPX (180 OMTs) to OMAPX * 14 (2520 OMTs). Upgrade the error to mention how far we searched, too. So that errors in search radius are apparent in debug messages, and don't have to be sussed out.

I audited every single search radius and kept search ranges for anything that previously had random search and seemed vaguely appropriate.

Conversely, I removed random (and did not return manual search range) from a few cases where it was clearly inappropriate. Anything searching for a GLOBAL_UNIQUE, for example, has no business searching an extra-limited range.

The end result is that all of our missions looking for singular important locations (i.e. all the ones which fail constantly) should be much more reliable.

Describe alternatives you've considered

Enforced 5000 tile search range

Testing

Loads. Properly errors if you try to lower the search range for no reason.

Additional context

[github-actions]

Auto-requesting reviews from non-collaborators: @LyleSY @Light-Wave @Night-Pryanik

[github-actions]

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

[JSON & C++ formatters](https://github.com/CleverRaven/Cataclysm-DDA/blob/master/doc/DEVELOPER_TOOLING.md)

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Magiclysm/npc/forge_diviner_missions.json

Lines 187 to 192 in ad827b4

	"assign_mission_target": {
	"om_terrain": "triffid_grove",
	"om_special": "Triffid Grove",
	"reveal_radius": 3,
	"cant_see": true
	}

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Magiclysm/npc/forge_diviner_missions.json

Lines 229 to 234 in ad827b4

	"assign_mission_target": {
	"om_terrain": "goblin_1A",
	"om_special": "goblin_encampment",
	"reveal_radius": 3,
	"cant_see": true
	}

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Magiclysm/npc/forge_diviner_missions.json

Lines 260 to 265 in ad827b4

	"assign_mission_target": {
	"om_terrain": "black_dragon_lair_z-0_NW",
	"om_special": "black_dragon_lair",
	"reveal_radius": 3,
	"cant_see": true
	}

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Magiclysm/npc/forge_diviner_missions.json

Lines 278 to 283 in ad827b4

	"assign_mission_target": {
	"om_terrain": "mine_balrog_central",
	"om_special": "Balrog mine",
	"reveal_radius": 3,
	"cant_see": true
	}

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Magiclysm/npc/forge_diviner_missions.json

Lines 309 to 314 in ad827b4

	"assign_mission_target": {
	"om_terrain": "central_lab_entrance",
	"om_special": "Central Lab",
	"reveal_radius": 3,
	"cant_see": true
	}

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Magiclysm/npc/forge_diviner_missions.json

Lines 353 to 358 in ad827b4

	"assign_mission_target": {
	"om_terrain": "exodii_base_x0y3z0",
	"om_special": "exodii_base",
	"reveal_radius": 3,
	"cant_see": true
	}

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Magiclysm/npc/forge_diviner_missions.json

Lines 371 to 376 in ad827b4

	"assign_mission_target": {
	"om_terrain": "temple_stairs",
	"om_special": "Strangle Temple",
	"reveal_radius": 3,
	"cant_see": true
	}

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Magiclysm/npc/forge_diviner_missions.json

Lines 389 to 394 in ad827b4

	"assign_mission_target": {
	"om_terrain": "attunement_altar_NW",
	"om_special": "attunement altar",
	"reveal_radius": 3,
	"cant_see": true
	}

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Magiclysm/npc/forge_diviner_missions.json

Lines 446 to 451 in ad827b4

	"assign_mission_target": {
	"om_terrain": "druid_ritual_home_z0",
	"om_special": "druid_tower",
	"reveal_radius": 3,
	"cant_see": true
	}

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Magiclysm/npc/forge_diviner_missions.json

Lines 539 to 544 in ad827b4

	"assign_mission_target": {
	"om_terrain": "magic_field_a1",
	"om_special": "magic_field",
	"reveal_radius": 3,
	"cant_see": true
	}

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Sky_Island/EOCs.json

Lines 984 to 991 in ad827b4

	"effect": [
	{
	"mapgen_update": "mx_skyisland_pond",
	"om_terrain": "sky_island_core",
	"offset_x": -1,
	"offset_y": -1
	}
	]

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Sky_Island/EOCs.json

Lines 996 to 1003 in ad827b4

	"effect": [
	{
	"mapgen_update": "mx_skyisland_pond",
	"om_terrain": "sky_island_core",
	"offset_x": 1,
	"offset_y": -1
	}
	]

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Sky_Island/EOCs.json

Lines 1008 to 1015 in ad827b4

	"effect": [
	{
	"mapgen_update": "mx_skyisland_pond",
	"om_terrain": "sky_island_core",
	"offset_x": -1,
	"offset_y": 1
	}
	]

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Sky_Island/EOCs.json

Lines 1020 to 1027 in ad827b4

	"effect": [
	{
	"mapgen_update": "mx_skyisland_pond",
	"om_terrain": "sky_island_core",
	"offset_x": 1,
	"offset_y": 1
	}
	]

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Sky_Island/island_upgrades.json

Lines 145 to 151 in ad827b4

	"target_params": {
	"om_terrain": "sky_island_core",
	"om_terrain_match_type": "CONTAINS",
	"offset_z": -1,
	"offset_x": -1,
	"offset_y": -1
	}

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Sky_Island/island_upgrades.json

Lines 164 to 170 in ad827b4

	"target_params": {
	"om_terrain": "sky_island_core",
	"om_terrain_match_type": "CONTAINS",
	"offset_z": -1,
	"offset_x": 1,
	"offset_y": -1
	}

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Sky_Island/island_upgrades.json

Lines 192 to 198 in ad827b4

	"target_params": {
	"om_terrain": "sky_island_core",
	"om_terrain_match_type": "CONTAINS",
	"offset_z": -1,
	"offset_x": -1,
	"offset_y": 1
	}

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Sky_Island/island_upgrades.json

Lines 211 to 217 in ad827b4

	"target_params": {
	"om_terrain": "sky_island_core",
	"om_terrain_match_type": "CONTAINS",
	"offset_z": -1,
	"offset_x": 1,
	"offset_y": 1
	}

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Xedra_Evolved/npc/boann_quests.json

Lines 106 to 111 in ad827b4

	"assign_mission_target": {
	"om_terrain": "mi-go_camp1",
	"om_special": "Mi-Go Encampment",
	"reveal_radius": 3,
	"cant_see": true
	}

---

[JSON & C++ formatters] _{reported by reviewdog 🐶}

Cataclysm-DDA/data/mods/Xedra_Evolved/npc/boann_quests.json

Lines 199 to 204 in ad827b4

	"assign_mission_target": {
	"om_terrain": "triffid_grove",
	"om_special": "Triffid Grove",
	"reveal_radius": 3,
	"cant_see": true
	}

[John-Candlebury]

I think expanding the search radius is fine when it comes to missions trying to find unique placement specials.

However there's incredibly many reasons for why you'd want something to forcibly spawn within <X overmap tiles of the player, (mission and non mission related) and the mission spawning code is AFAIK the only way in which this can be achieved, so completely getting rid of the functionality is not a great idea.

For example I'm not sure how this will trully affect the spawning of Aftershock shuttles, if the player has explored enough fields around them, could the shuttle be placed so far away that they wont be able to reach it before it leaves? I wouldnt want that to ever happen.

[RenechCDDA]

I think expanding the search radius is fine when it comes to missions trying to find unique placement specials.

However there's incredibly many reasons for why you'd want something to forcibly spawn within <X overmap tiles of the player, (mission and non mission related) and the mission spawning code is AFAIK the only way in which this can be achieved, so completely getting rid of the functionality is not a great idea.

For example I'm not sure how this will trully affect the spawning of Aftershock shuttles, if the player has explored enough fields around them, could the shuttle be placed so far away that they wont be able to reach it before it leaves? I wouldnt want that to ever happen.

get_target_position() (lambda) always tries to find or create on your overmap first. It only proceeds to other overmaps if it fails in your local overmap. The shuttle eoc looks like it can just be placed down in any field regardless of existing submaps.

TBH if we find issues with this it's a lot easier to re-add the functionality for the few cases that (might?) need it, than to allow it for every mission. Missions should definitely not be using this unless they explicitly need it, and (no offense intended) I am not convinced any missions have a need for it.

[John-Candlebury]

But you still gave no valid reason to remove it? Like I see its being extensively misused, but you are completely getting rid of our ability to say: "Game spawn this far enough but not too far away"

Sure you cannot think any reasons for why youd want to use the option in vanilla but that not true off in mods. Just familiarizing myself with the changes here, the sky island mod setting the exildistance between two values seems extremely deliberate and a valid use case to me, especially since it changes depending on choices the platyer makes?

Never mind that theres other valid reasons that are not implemented example: you use hellgate spell and the helgate spawns between 3 and 5 overmaps away from you (because you dont want to do the effects of spawning the thing live in the reality bubble but you dont want it too far away), same logic applies to for example Aftershock using a SAM site to down a drone/plane/spaceship.

[RenechCDDA]

Okay after some discussion with Candlebury: They're completely right. That WAS doing something useful. Oops.

So I went through every single one of them, again, and re-added the search ranges to anything that previously had random search and seemed vaguely appropriate.

Conversely, I removed random (and did not return manual search range) from a few cases where it was clearly inappropriate. Anything searching for a GLOBAL_UNIQUE, for example, has no business searching a limited range. Dinomod had one case of searching only one overmap for a 4x4 (!!!) surface lab. That was incredibly prone to failure, so I just nixed that to make sure it could be found.

Other than that, very cut and dry. But they're in a separate commit so mod authors/reviewers can look them over to be sure.

[Procyonae]

I don't hate this as a temporary measure but this needs a more nuanced permanent solution, I've thought quite a bit about ideal solutions but still haven't decided on anything, you can find some of my previous thoughts here #76684 (comment)