Ship Weapon Fix (#23038)

* Fixed ship munitions occasionally failing to enter their overmap
target when their original physical projectile no longer existed.
* Fixed several ship munitions reconstructing as the wrong projectile
after overmap travel, ensuring explosive rounds correctly detonate on
impact.

Also had the AI write a unit test that simulates ship combat using every
munition type, so we don't have more regressions here.

AI usage disclosure: The code for this was created using GPT 5.6 Sol.

code/modules/overmap/ship_weaponry/_ship_ammunition.dm

@@ -22,7 +22,8 @@
 	var/obj/effect/overmap/origin
 	var/atom/overmap_target
 	var/obj/entry_point
-	var/obj/projectile/original_projectile
+	/// The exact physical projectile type to reconstruct when this ammunition leaves the overmap.
+	var/fired_projectile_type
 	var/heading = SOUTH
 	var/range = OVERMAP_PROJECTILE_RANGE_MEDIUM
 	var/mob_carry_size = 12 //How large a mob has to be to carry the shell
@@ -61,7 +62,8 @@
 	origin = null
 	overmap_target = null
 	entry_point = null
-	original_projectile = null
+	// The typepath is runtime metadata for one fired round and must not outlive it.
+	fired_projectile_type = null
 	return ..()
 
 /obj/item/ship_ammunition/attackby(obj/item/attacking_item, mob/user)
@@ -322,9 +324,11 @@
 				H.playsound_local(null, 'sound/effects/explosionfar.ogg', 25)
 				shake_camera(H, 2, 2)
 		..()
+	// Preserve only the concrete type needed at the destination. The ammunition
+	// has already moved into the overmap carrier, so this physical shell is spent.
+	ammo.fired_projectile_type = type
 	if(ammo.touch_map_edge(z))
-		ammo.original_projectile = src
-		forceMove(ammo)
+		qdel(src)
 
 /obj/projectile/ship_ammo/on_hit(atom/target, blocked, def_zone, var/is_landmark_hit = FALSE) //is_landmark_hit is TRUE when we hit a landmark on a visitable non-ship overmap object.
 	if(target && !hit_target)

code/modules/overmap/ship_weaponry/projectiles/_overmap_projectiles.dm

@@ -101,12 +101,16 @@
 
 /obj/effect/overmap/projectile/proc/prepare_for_entry()
 	moving = FALSE
-	entering = FALSE
+	// Entry is a one-shot ownership handoff; stale Move() callbacks must not repeat it.
+	entering = TRUE
 	walk(src, 0)
 
 ///Checks if we can hit the thing we just bumped into. If we can, do the initial setup for the projectile entering the submab and continue.
 /obj/effect/overmap/projectile/proc/check_entry()
 	. = FALSE
+	// A translated carrier can remain referenced until qdel completes.
+	if(entering)
+		return TRUE
 	if(!ammunition)
 		return
 	var/turf/T = get_turf(src)
@@ -116,12 +120,16 @@
 		if(istype(A, /obj/effect/overmap/visitable))
 			var/obj/effect/overmap/visitable/V = A
 			if((V.check_ownership(submap_target)) || (V == target)) //If the visitable is owned by the target landmark, or is the target itself, we can hit it.
+				if(!ammunition.fired_projectile_type)
+					// This carrier was not produced by a valid ship projectile transition.
+					qdel(src)
+					return TRUE
 				var/turf/target_turf = get_turf(submap_target)
-				var/obj/projectile/ship_ammo/widowmaker = new ammunition.original_projectile.type
+				var/obj/projectile/ship_ammo/widowmaker = new ammunition.fired_projectile_type
 				prepare_for_entry()
 				widowmaker.ammo = ammunition
-				qdel(ammunition.original_projectile) //No longer needed.
-				ammunition.original_projectile = widowmaker
+				// Transfer the payload before deleting the now-empty overmap carrier.
+				ammunition.forceMove(widowmaker)
 				widowmaker.primed = TRUE
 
 				if(istype(V, /obj/effect/overmap/visitable/sector/exoplanet) && (ammunition.overmap_behaviour & SHIP_AMMO_CAN_HIT_PLANETS))

code/modules/overmap/ship_weaponry/weaponry/blaster.dm

@@ -20,6 +20,7 @@
 	caliber = SHIP_CALIBER_BLASTER
 	overmap_icon_state = "med_laser"
 	impact_type = SHIP_AMMO_IMPACT_BLASTER
+	projectile_type_override = /obj/projectile/ship_ammo/blaster
 
 /obj/projectile/ship_ammo/blaster
 	name = "blaster charge"

code/modules/overmap/ship_weaponry/weaponry/bruiser.dm

@@ -29,6 +29,7 @@
 	burst = 11
 	cookoff_heavy = 0
 	overmap_icon_state = "cannon_salvo"
+	projectile_type_override = /obj/projectile/ship_ammo/bruiser
 
 /obj/item/ship_ammunition/bruiser/get_speed()
 	return 35

code/modules/overmap/ship_weaponry/weaponry/coilgun.dm

@@ -20,6 +20,7 @@
 	caliber = SHIP_CALIBER_COILGUN
 	overmap_icon_state = "cannon"
 	impact_type = SHIP_AMMO_IMPACT_BUNKERBUSTER
+	projectile_type_override = /obj/projectile/ship_ammo/coilgun
 
 /obj/projectile/ship_ammo/coilgun
 	name = "high-power tungsten rod"

code/modules/overmap/ship_weaponry/weaponry/francisca.dm

@@ -28,6 +28,7 @@
 	caliber = SHIP_CALIBER_40MM
 	burst = 8
 	cookoff_heavy = 0
+	projectile_type_override = /obj/projectile/ship_ammo/francisca
 
 /obj/item/ship_ammunition/francisca/ap
 	name = "40mm AP ammunition box"

code/modules/overmap/ship_weaponry/weaponry/lammergeier.dm

@@ -19,6 +19,7 @@
 	caliber = SHIP_CALIBER_200MM
 	overmap_icon_state = "cannon"
 	impact_type = SHIP_AMMO_IMPACT_HE
+	projectile_type_override = /obj/projectile/ship_ammo/lammergeier
 
 /obj/projectile/ship_ammo/lammergeier
 	name = "typhoon shell"

code/modules/overmap/ship_weaponry/weaponry/leviathan.dm

@@ -128,6 +128,7 @@
 	caliber = SHIP_CALIBER_ZAT
 	impact_type = SHIP_AMMO_IMPACT_ZAT
 	overmap_icon_state = "heavy_pulse"
+	projectile_type_override = /obj/projectile/ship_ammo/leviathan
 
 /obj/item/ship_ammunition/leviathan/Initialize()
 	. = ..()

code/modules/overmap/ship_weaponry/weaponry/light_coilgun.dm

@@ -21,6 +21,7 @@
 	caliber = SHIP_CALIBER_COILGUN
 	overmap_icon_state = "light_pulse"
 	impact_type = SHIP_AMMO_IMPACT_AP
+	projectile_type_override = /obj/projectile/ship_ammo/coilgun/light
 
 /obj/projectile/ship_ammo/coilgun/light
 	name = "low-power tungsten rod"

code/modules/overmap/ship_weaponry/weaponry/longbow_ammo.dm

@@ -6,6 +6,7 @@
 	item_state = "generic_casing_obj"
 	caliber = SHIP_CALIBER_406MM
 	ammunition_behaviour = SHIP_AMMO_BEHAVIOUR_DUMBFIRE
+	projectile_type_override = /obj/projectile/ship_ammo/longbow
 	var/obj/item/primer/primer
 	var/obj/item/warhead/longbow/warhead
 

code/modules/overmap/ship_weaponry/weaponry/nephilim.dm

@@ -34,6 +34,7 @@
 	caliber = SHIP_CALIBER_BLASTER
 	burst = 15
 	cookoff_heavy = 0
+	projectile_type_override = /obj/projectile/ship_ammo/nephilim/he
 
 /obj/item/ship_ammunition/nephilim/ap
 	name = "Blaster AP hydrogen cell"
@@ -42,6 +43,7 @@
 	icon = 'icons/obj/guns/ship/ship_ammo_nephilim.dmi'
 	icon_state = "repeater_ap"
 	impact_type = SHIP_AMMO_IMPACT_AP
+	projectile_type_override = /obj/projectile/ship_ammo/nephilim/ap
 
 /obj/projectile/ship_ammo/nephilim
 	name = "hydrogen cell he"
@@ -58,6 +60,7 @@
 	damage = 20
 	armor_penetration = 100
 	penetrating = 4
+	explosion_strength = list(0, 0, 0)
 
 /obj/projectile/ship_ammo/nephilim/he/on_hit(atom/target, blocked, def_zone, is_landmark_hit)
 	. = ..()

code/unit_tests/overmap_tests.dm

@@ -46,3 +46,169 @@
 	if(!failures)
 		TEST_PASS("All ships have a class and designation.")
 	return TRUE
+
+// Lightweight combat doubles: they exercise the overmap collision and payload
+// handoff without firing a real projectile across one of the live ship maps.
+/obj/effect/overmap/visitable/unit_test_combat_target/Initialize()
+	SHOULD_CALL_PARENT(FALSE)
+	// Avoid registering this lightweight collision target as a real sector.
+	return INITIALIZE_HINT_NORMAL
+
+/obj/effect/overmap/visitable/unit_test_combat_target/check_ownership(obj/object)
+	return FALSE
+
+/obj/effect/overmap/projectile/unit_test_combat
+	/// Number of payload handoffs performed by this combat carrier.
+	var/entry_calls = 0
+	/// Whether the reconstructed projectile became the ammunition's physical owner.
+	var/payload_transferred = FALSE
+	/// Physical projectile created by the overmap entry code under test.
+	var/obj/projectile/ship_ammo/reconstructed_projectile
+
+/obj/effect/overmap/projectile/unit_test_combat/Initialize()
+	SHOULD_CALL_PARENT(FALSE)
+	// Avoid starting the production movement timer before the test configures the carrier.
+	return INITIALIZE_HINT_NORMAL
+
+/obj/effect/overmap/projectile/unit_test_combat/check_entry_visitable(obj/projectile/ship_ammo/widowmaker, turf/target_turf)
+	// Capture the handoff result without firing across a live ship map.
+	entry_calls++
+	payload_transferred = (widowmaker.ammo?.loc == widowmaker)
+	reconstructed_projectile = widowmaker
+	qdel(src)
+	return TRUE
+
+/obj/effect/unit_test_ship_ammo_impact/Initialize()
+	return ..()
+
+/datum/unit_test/overmap_projectile_entry_is_one_shot
+	name = "OVERMAP: Combat projectile entry is one-shot"
+	groups = list("map", "overmap")
+
+/datum/unit_test/overmap_projectile_entry_is_one_shot/start_test()
+	var/test_z = SSatlas.current_map?.overmap_z ? SSatlas.current_map.overmap_z : 1
+	var/turf/combat_turf = locate(2, 2, test_z)
+	TEST_ASSERT_NOTNULL(combat_turf, "Could not locate an overmap turf for the combat simulation.")
+
+	var/obj/effect/overmap/visitable/unit_test_combat_target/combat_target = new(combat_turf)
+	var/obj/effect/overmap/projectile/unit_test_combat/overmap_projectile = new(combat_turf)
+	var/obj/item/ship_ammunition/ammunition = new(overmap_projectile)
+	ammunition.fired_projectile_type = /obj/projectile/ship_ammo
+	ammunition.overmap_behaviour = SHIP_AMMO_CAN_HIT_VISITABLES
+	overmap_projectile.ammunition = ammunition
+	overmap_projectile.target = combat_target
+
+	TEST_ASSERT(overmap_projectile.check_entry(), "The projectile did not enter its overmap combat target.")
+	TEST_ASSERT(overmap_projectile.entering, "The overmap projectile was not marked as entering.")
+	TEST_ASSERT(overmap_projectile.check_entry(), "A repeated entry check did not report the handled collision.")
+	TEST_ASSERT_EQUAL(overmap_projectile.entry_calls, 1, "The payload was translated more than once.")
+	TEST_ASSERT(overmap_projectile.payload_transferred, "The ammunition was not transferred to the reconstructed projectile.")
+
+	qdel(combat_target)
+	qdel(overmap_projectile.reconstructed_projectile)
+	qdel(overmap_projectile)
+	TEST_PASS("An overmap combat projectile translates its payload only once.")
+	return TRUE
+
+/datum/unit_test/overmap_projectile_missing_type
+	name = "OVERMAP: Combat carrier rejects a missing projectile type"
+	groups = list("map", "overmap")
+
+/datum/unit_test/overmap_projectile_missing_type/start_test()
+	var/test_z = SSatlas.current_map?.overmap_z ? SSatlas.current_map.overmap_z : 1
+	var/turf/combat_turf = locate(2, 2, test_z)
+	TEST_ASSERT_NOTNULL(combat_turf, "Could not locate an overmap turf for the combat simulation.")
+
+	var/obj/effect/overmap/visitable/unit_test_combat_target/combat_target = new(combat_turf)
+	var/obj/effect/overmap/projectile/unit_test_combat/overmap_projectile = new(combat_turf)
+	var/obj/item/ship_ammunition/ammunition = new(overmap_projectile)
+	ammunition.overmap_behaviour = SHIP_AMMO_CAN_HIT_VISITABLES
+	overmap_projectile.ammunition = ammunition
+	overmap_projectile.target = combat_target
+
+	TEST_ASSERT(overmap_projectile.check_entry(), "The malformed carrier collision was not handled.")
+	TEST_ASSERT(QDELETED(overmap_projectile), "A carrier with no projectile type was not deleted.")
+	TEST_ASSERT_EQUAL(overmap_projectile.entry_calls, 0, "Entry ran without a projectile type.")
+
+	qdel(combat_target)
+	qdel(overmap_projectile)
+	TEST_PASS("An invalid overmap carrier is discarded without attempting reconstruction.")
+	return TRUE
+
+/datum/unit_test/overmap_all_ship_munitions
+	name = "OVERMAP: All ship munitions reconstruct and explosive rounds detonate"
+	groups = list("map", "overmap")
+
+/datum/unit_test/overmap_all_ship_munitions/start_test()
+	var/test_z = SSatlas.current_map?.overmap_z ? SSatlas.current_map.overmap_z : 1
+	var/turf/combat_turf = locate(2, 2, test_z)
+	TEST_ASSERT_NOTNULL(combat_turf, "Could not locate an overmap turf for the munitions simulation.")
+
+	var/list/explosive_impacts = list(
+		SHIP_AMMO_IMPACT_HE,
+		SHIP_AMMO_IMPACT_BLASTER,
+		SHIP_AMMO_IMPACT_BUNKERBUSTER,
+		SHIP_AMMO_IMPACT_ZAT
+	)
+	var/tested_munitions = 0
+	var/test_status = UNIT_TEST_PASSED
+
+	// Enumerate typepaths so newly added loadable ammunition is covered automatically.
+	for(var/ammunition_type in subtypesof(/obj/item/ship_ammunition))
+		if(is_abstract(ammunition_type))
+			continue
+
+		var/obj/item/ship_ammunition/ammunition = new ammunition_type
+		if(!ammunition.can_be_loaded())
+			qdel(ammunition)
+			continue
+
+		tested_munitions++
+		if(!ammunition.projectile_type_override)
+			test_status = TEST_FAIL("[ammunition_type] has no explicit physical projectile type to reconstruct after overmap travel.")
+			qdel(ammunition)
+			continue
+
+		// Probes intentionally remain on the overmap and never impact a visitable.
+		if(!ammunition.overmap_behaviour)
+			qdel(ammunition)
+			continue
+
+		var/obj/effect/overmap/visitable/unit_test_combat_target/combat_target = new(combat_turf)
+		var/obj/effect/overmap/projectile/unit_test_combat/overmap_projectile = new(combat_turf)
+		ammunition.forceMove(overmap_projectile)
+		ammunition.fired_projectile_type = ammunition.projectile_type_override
+		overmap_projectile.ammunition = ammunition
+		overmap_projectile.target = combat_target
+
+		if(!overmap_projectile.check_entry())
+			test_status = TEST_FAIL("[ammunition_type] did not enter its overmap combat target.")
+		else if(overmap_projectile.entry_calls != 1 || !overmap_projectile.payload_transferred || !overmap_projectile.reconstructed_projectile)
+			test_status = TEST_FAIL("[ammunition_type] did not reconstruct and transfer its payload exactly once.")
+
+		var/obj/projectile/ship_ammo/reconstructed = overmap_projectile.reconstructed_projectile
+		var/should_explode = reconstructed && ((ammunition.impact_type in explosive_impacts) || max(reconstructed.explosion_strength) > 0)
+		if(should_explode)
+			// Calling the real on_hit() must enqueue an explosion. Remove that queued
+			// work afterward so this test cannot damage the unit-test map.
+			var/queued_explosions = length(SSexplosives.work_queue)
+			var/obj/effect/unit_test_ship_ammo_impact/impact_target = new(combat_turf)
+			reconstructed.forceMove(combat_turf)
+			reconstructed.on_hit(impact_target)
+			if(length(SSexplosives.work_queue) <= queued_explosions)
+				test_status = TEST_FAIL("[ammunition_type] ([ammunition.impact_type]) did not produce an explosion on impact.")
+			else
+				var/list/test_explosions = SSexplosives.work_queue.Copy(queued_explosions + 1)
+				SSexplosives.work_queue -= test_explosions
+				QDEL_LIST(test_explosions)
+			qdel(impact_target)
+
+		qdel(reconstructed)
+		qdel(combat_target)
+		qdel(overmap_projectile)
+
+	if(!tested_munitions)
+		return TEST_FAIL("No loadable ship munitions were discovered.")
+	if(test_status == UNIT_TEST_PASSED)
+		TEST_PASS("All [tested_munitions] loadable ship munitions reconstructed; every explosive impact queued an explosion.")
+	return TRUE

html/changelogs/geeves-ship_weapon_fix.yml

@@ -0,0 +1,6 @@
+author: Geeves
+delete-after: True
+
+changes:
+  - bugfix: "Fixed ship munitions occasionally failing to enter their overmap target when their original physical projectile no longer existed."
+  - bugfix: "Fixed several ship munitions reconstructing as the wrong projectile after overmap travel, ensuring explosive rounds correctly detonate on impact."