From 448932f01cf367219ff45b90d365bc54bc5b0212 Mon Sep 17 00:00:00 2001 From: Geeves <22774890+Geevies@users.noreply.github.com> Date: Mon, 10 Aug 2026 21:47:19 +0200 Subject: [PATCH] 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. --- .../overmap/ship_weaponry/_ship_ammunition.dm | 12 +- .../projectiles/_overmap_projectiles.dm | 16 +- .../overmap/ship_weaponry/weaponry/blaster.dm | 1 + .../overmap/ship_weaponry/weaponry/bruiser.dm | 1 + .../overmap/ship_weaponry/weaponry/coilgun.dm | 1 + .../ship_weaponry/weaponry/francisca.dm | 1 + .../ship_weaponry/weaponry/lammergeier.dm | 1 + .../ship_weaponry/weaponry/leviathan.dm | 1 + .../ship_weaponry/weaponry/light_coilgun.dm | 1 + .../ship_weaponry/weaponry/longbow_ammo.dm | 1 + .../ship_weaponry/weaponry/nephilim.dm | 3 + code/unit_tests/overmap_tests.dm | 166 ++++++++++++++++++ html/changelogs/geeves-ship_weapon_fix.yml | 6 + 13 files changed, 203 insertions(+), 8 deletions(-) create mode 100644 html/changelogs/geeves-ship_weapon_fix.yml diff --git a/code/modules/overmap/ship_weaponry/_ship_ammunition.dm b/code/modules/overmap/ship_weaponry/_ship_ammunition.dm index dc63d30f813..f11d09d670e 100644 --- a/code/modules/overmap/ship_weaponry/_ship_ammunition.dm +++ b/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) diff --git a/code/modules/overmap/ship_weaponry/projectiles/_overmap_projectiles.dm b/code/modules/overmap/ship_weaponry/projectiles/_overmap_projectiles.dm index 3eee8c1fb8d..2a16859ab50 100644 --- a/code/modules/overmap/ship_weaponry/projectiles/_overmap_projectiles.dm +++ b/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)) diff --git a/code/modules/overmap/ship_weaponry/weaponry/blaster.dm b/code/modules/overmap/ship_weaponry/weaponry/blaster.dm index 1b3895c9daa..3b37ed34eed 100644 --- a/code/modules/overmap/ship_weaponry/weaponry/blaster.dm +++ b/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" diff --git a/code/modules/overmap/ship_weaponry/weaponry/bruiser.dm b/code/modules/overmap/ship_weaponry/weaponry/bruiser.dm index 8768efada21..c3f3e7369ac 100644 --- a/code/modules/overmap/ship_weaponry/weaponry/bruiser.dm +++ b/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 diff --git a/code/modules/overmap/ship_weaponry/weaponry/coilgun.dm b/code/modules/overmap/ship_weaponry/weaponry/coilgun.dm index 1e5f9f88a9b..9b8de071cec 100644 --- a/code/modules/overmap/ship_weaponry/weaponry/coilgun.dm +++ b/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" diff --git a/code/modules/overmap/ship_weaponry/weaponry/francisca.dm b/code/modules/overmap/ship_weaponry/weaponry/francisca.dm index 26f8660a9d9..1b831439438 100644 --- a/code/modules/overmap/ship_weaponry/weaponry/francisca.dm +++ b/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" diff --git a/code/modules/overmap/ship_weaponry/weaponry/lammergeier.dm b/code/modules/overmap/ship_weaponry/weaponry/lammergeier.dm index 8ffe1792590..1541c21f025 100644 --- a/code/modules/overmap/ship_weaponry/weaponry/lammergeier.dm +++ b/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" diff --git a/code/modules/overmap/ship_weaponry/weaponry/leviathan.dm b/code/modules/overmap/ship_weaponry/weaponry/leviathan.dm index cd8dc95be6a..9d524fa707c 100644 --- a/code/modules/overmap/ship_weaponry/weaponry/leviathan.dm +++ b/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() . = ..() diff --git a/code/modules/overmap/ship_weaponry/weaponry/light_coilgun.dm b/code/modules/overmap/ship_weaponry/weaponry/light_coilgun.dm index e60ba2924c8..cd0c2e4bbee 100644 --- a/code/modules/overmap/ship_weaponry/weaponry/light_coilgun.dm +++ b/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" diff --git a/code/modules/overmap/ship_weaponry/weaponry/longbow_ammo.dm b/code/modules/overmap/ship_weaponry/weaponry/longbow_ammo.dm index d409e8423ee..37114d49272 100644 --- a/code/modules/overmap/ship_weaponry/weaponry/longbow_ammo.dm +++ b/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 diff --git a/code/modules/overmap/ship_weaponry/weaponry/nephilim.dm b/code/modules/overmap/ship_weaponry/weaponry/nephilim.dm index 2d910fc41c8..bd89b56bd2a 100644 --- a/code/modules/overmap/ship_weaponry/weaponry/nephilim.dm +++ b/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) . = ..() diff --git a/code/unit_tests/overmap_tests.dm b/code/unit_tests/overmap_tests.dm index 04a4b6e4f53..a298b08a079 100644 --- a/code/unit_tests/overmap_tests.dm +++ b/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 diff --git a/html/changelogs/geeves-ship_weapon_fix.yml b/html/changelogs/geeves-ship_weapon_fix.yml new file mode 100644 index 00000000000..d935fe66402 --- /dev/null +++ b/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."