From e712aa2b414225abe0c74979bb5ea9c0e2c777d9 Mon Sep 17 00:00:00 2001 From: Bloop <13398309+vinylspiders@users.noreply.github.com> Date: Wed, 20 Sep 2023 08:50:48 -0400 Subject: [PATCH] Fixes rescue hooks, adds a unit test to help prevent it from breaking again (#78418) ## About The Pull Request Fixes https://github.com/Skyrat-SS13/Skyrat-tg/issues/23763 Somewhere along https://github.com/tgstation/tgstation/pull/77739 and the following fishing PR's this feature got overlooked and broken. The args for `dispense_reward()` and `find_chasm_contents()` needed to be updated as they were just wrong. This sets them straight and adds an additional fishing unit test for the rescue hook to hopefully prevent this edge case from being overlooked. ## Why It's Good For The Game Rescue hooks work again, hooray. ## Changelog :cl: fix: rescue hooks will once again drop the mob next to the fisherman instead of just displaying a balloon alert and doing nothing /:cl: --------- Co-authored-by: Ghom <42542238+Ghommie@users.noreply.github.com> --- code/datums/components/chasm.dm | 4 +- code/modules/fishing/fish/chasm_detritus.dm | 19 +++-- code/modules/fishing/sources/_fish_source.dm | 9 ++- code/modules/unit_tests/fish_unit_tests.dm | 74 ++++++++++++++++++++ 4 files changed, 91 insertions(+), 15 deletions(-) diff --git a/code/datums/components/chasm.dm b/code/datums/components/chasm.dm index 12dff2b6aa9..adca3d509d6 100644 --- a/code/datums/components/chasm.dm +++ b/code/datums/components/chasm.dm @@ -235,13 +235,13 @@ GLOBAL_LIST_EMPTY(chasm_fallen_mobs) /obj/effect/abstract/chasm_storage/Entered(atom/movable/arrived) . = ..() - if (isliving(arrived)) + if(isliving(arrived)) RegisterSignal(arrived, COMSIG_LIVING_REVIVE, PROC_REF(on_revive)) GLOB.chasm_fallen_mobs += arrived /obj/effect/abstract/chasm_storage/Exited(atom/movable/gone) . = ..() - if (isliving(gone)) + if(isliving(gone)) UnregisterSignal(gone, COMSIG_LIVING_REVIVE) GLOB.chasm_fallen_mobs -= gone diff --git a/code/modules/fishing/fish/chasm_detritus.dm b/code/modules/fishing/fish/chasm_detritus.dm index b3964eb4015..8d665378159 100644 --- a/code/modules/fishing/fish/chasm_detritus.dm +++ b/code/modules/fishing/fish/chasm_detritus.dm @@ -40,22 +40,21 @@ GLOBAL_LIST_INIT_TYPED(chasm_detritus_types, /datum/chasm_detritus, init_chasm_d ), ) -/datum/chasm_detritus/proc/dispense_reward(turf/fishing_spot, turf/fisher_turf) - if (prob(default_contents_chance)) +/datum/chasm_detritus/proc/dispense_detritus(mob/fisherman, turf/fishing_spot) + if(prob(default_contents_chance)) var/default_spawn = pick(default_contents[default_contents_key]) - return new default_spawn(fisher_turf) - return find_chasm_contents(fishing_spot, fisher_turf) + return new default_spawn(get_turf(fisherman)) + return find_chasm_contents(fishing_spot, get_turf(fisherman)) /// Returns the chosen detritus from the given list of things to choose from /datum/chasm_detritus/proc/determine_detritus(list/chasm_stuff) return pick(chasm_stuff) -/// Returns an objected which is currently inside of a nearby chasm. -/datum/chasm_detritus/proc/find_chasm_contents(datum/source, turf/fishing_spot, turf/fisher_turf) - SIGNAL_HANDLER +/// Returns an object which is currently inside of a nearby chasm. +/datum/chasm_detritus/proc/find_chasm_contents(turf/fishing_spot, turf/fisher_turf) var/list/chasm_contents = get_chasm_contents(fishing_spot) - if (!length(chasm_contents)) + if(!length(chasm_contents)) var/default_spawn = pick(default_contents[default_contents_key]) return new default_spawn(fisher_turf) @@ -63,7 +62,7 @@ GLOBAL_LIST_INIT_TYPED(chasm_detritus_types, /datum/chasm_detritus, init_chasm_d /datum/chasm_detritus/proc/get_chasm_contents(turf/fishing_spot) . = list() - for (var/obj/effect/abstract/chasm_storage/storage in range(5, fishing_spot)) + for(var/obj/effect/abstract/chasm_storage/storage in range(5, fishing_spot)) for (var/thing as anything in storage.contents) . += thing @@ -76,7 +75,7 @@ GLOBAL_LIST_INIT_TYPED(chasm_detritus_types, /datum/chasm_detritus, init_chasm_d /datum/chasm_detritus/restricted/get_chasm_contents(turf/fishing_spot) . = list() - for (var/obj/effect/abstract/chasm_storage/storage in range(5, fishing_spot)) + for(var/obj/effect/abstract/chasm_storage/storage in range(5, fishing_spot)) for (var/thing as anything in storage.contents) if(!istype(thing, chasm_storage_restricted_type)) continue diff --git a/code/modules/fishing/sources/_fish_source.dm b/code/modules/fishing/sources/_fish_source.dm index e0f16914570..5a34db9ce5f 100644 --- a/code/modules/fishing/sources/_fish_source.dm +++ b/code/modules/fishing/sources/_fish_source.dm @@ -127,21 +127,24 @@ GLOBAL_LIST_INIT(preset_fish_sources, init_subtypes_w_path_keys(/datum/fish_sour fish_table -= reward_path var/atom/movable/reward = spawn_reward(reward_path, fisherman, fishing_spot) - if(!reward) //baloon alert instead + if(!reward) //balloon alert instead fisherman.balloon_alert(fisherman,pick(duds)) return if(isitem(reward)) //Try to put it in hand INVOKE_ASYNC(fisherman, TYPE_PROC_REF(/mob, put_in_hands), reward) + else // for fishing things like corpses, move them to the turf of the fisherman + INVOKE_ASYNC(reward, TYPE_PROC_REF(/atom/movable, forceMove), get_turf(fisherman)) fisherman.balloon_alert(fisherman, "caught [reward]!") + SEND_SIGNAL(fisherman, COMSIG_MOB_FISHING_REWARD_DISPENSED, reward) return reward /// Spawns a reward from a atom path right where the fisherman is. Part of the dispense_reward() logic. -/datum/fish_source/proc/spawn_reward(reward_path, mob/fisherman, turf/fishing_spot) +/datum/fish_source/proc/spawn_reward(reward_path, mob/fisherman, turf/fishing_spot) if(reward_path == FISHING_DUD) return if(ispath(reward_path, /datum/chasm_detritus)) - return GLOB.chasm_detritus_types[reward_path].dispense_reward(fishing_spot, get_turf(fisherman)) + return GLOB.chasm_detritus_types[reward_path].dispense_detritus(fisherman, fishing_spot) if(!ispath(reward_path, /atom/movable)) CRASH("Unsupported /datum path [reward_path] passed to fish_source/proc/spawn_reward()") var/atom/movable/reward = new reward_path(get_turf(fisherman)) diff --git a/code/modules/unit_tests/fish_unit_tests.dm b/code/modules/unit_tests/fish_unit_tests.dm index 6c4a278a396..1ef15f8d0f5 100644 --- a/code/modules/unit_tests/fish_unit_tests.dm +++ b/code/modules/unit_tests/fish_unit_tests.dm @@ -123,5 +123,79 @@ . = ..() probability = 0 //works around the global list initialization skipping abstract/impossible evolutions. +// we want no default spawns in this unit test +/datum/chasm_detritus/restricted/bodies/no_defaults + default_contents_chance = 0 + +/// Checks that we are able to fish people out of chasms with priority and that they end up in the right location +/datum/unit_test/fish_rescue_hook + priority = TEST_LONGER + var/original_turf_type + var/original_turf_baseturfs + var/list/mobs_spawned + +/datum/unit_test/fish_rescue_hook/Run() + // create our human dummies to be dropped into the chasm + var/mob/living/carbon/human/consistent/get_in_the_hole = allocate(/mob/living/carbon/human/consistent) + var/mob/living/basic/mining/lobstrosity/you_too = allocate(/mob/living/basic/mining/lobstrosity) + var/mob/living/carbon/human/consistent/mindless = allocate(/mob/living/carbon/human/consistent) + var/mob/living/carbon/human/consistent/no_brain = allocate(/mob/living/carbon/human/consistent) + var/mob/living/carbon/human/consistent/empty = allocate(/mob/living/carbon/human/consistent) + var/mob/living/carbon/human/consistent/dummy = allocate(/mob/living/carbon/human/consistent) + + mobs_spawned = list( + get_in_the_hole, + you_too, + mindless, + no_brain, + empty, + dummy, + ) + + // create our chasm and remember the previous turf so we can change it back once we're done + original_turf_type = run_loc_floor_bottom_left.type + original_turf_baseturfs = islist(run_loc_floor_bottom_left.baseturfs) ? run_loc_floor_bottom_left.baseturfs.Copy() : run_loc_floor_bottom_left.baseturfs + run_loc_floor_bottom_left.ChangeTurf(/turf/open/chasm) + var/turf/open/chasm/the_hole = run_loc_floor_bottom_left + + // into the hole they go + for(var/mob/mob_spawned in mobs_spawned) + the_hole.drop(mob_spawned) + sleep(0.2 SECONDS) // we have to WAIT because the drop() proc sleeps. + + // our 'fisherman' where we expect the item to be moved to after fishing it up + var/mob/living/carbon/human/consistent/a_fisherman = allocate(/mob/living/carbon/human/consistent, run_loc_floor_top_right) + + // pretend like this mob has a mind. they should be fished up first + no_brain.mind_initialize() + + SEND_SIGNAL(the_hole, COMSIG_PRE_FISHING) // we need to do this for the fishing spot component to be attached + var/datum/component/fishing_spot/the_hole_fishing_spot = the_hole.GetComponent(/datum/component/fishing_spot) + var/datum/fish_source/fishing_source = the_hole_fishing_spot.fish_source + var/obj/item/fishing_hook/rescue/the_hook = allocate(/obj/item/fishing_hook/rescue, run_loc_floor_top_right) + the_hook.chasm_detritus_type = /datum/chasm_detritus/restricted/bodies/no_defaults + + // try to fish up our minded victim + var/atom/movable/reward = fishing_source.dispense_reward(the_hook.chasm_detritus_type, a_fisherman, the_hole) + + // mobs with minds (aka players) should have precedence over any other mobs that are in the chasm + TEST_ASSERT_EQUAL(reward, no_brain, "Fished up [reward] ([REF(reward)]) with a rescue hook; expected to fish up [no_brain]([REF(no_brain)])") + // it should end up on the same turf as the fisherman + TEST_ASSERT_EQUAL(get_turf(reward), get_turf(a_fisherman), "[reward] was fished up with the rescue hook and ended up at [get_turf(reward)]; expected to be at [get_turf(a_fisherman)]") + + // let's further test that by giving a second mob a mind. they should be fished up immediately.. + empty.mind_initialize() + + reward = fishing_source.dispense_reward(the_hook.chasm_detritus_type, a_fisherman, the_hole) + + TEST_ASSERT_EQUAL(reward, empty, "Fished up [reward]([REF(reward)]) with a rescue hook; expected to fish up [empty]([REF(empty)])") + TEST_ASSERT_EQUAL(get_turf(reward), get_turf(a_fisherman), "[reward] was fished up with the rescue hook and ended up at [get_turf(reward)]; expected to be at [get_turf(a_fisherman)]") + +// clean up so we don't mess up subsequent tests +/datum/unit_test/fish_rescue_hook/Destroy() + QDEL_LIST(mobs_spawned) + run_loc_floor_bottom_left.ChangeTurf(original_turf_type, original_turf_baseturfs) + return ..() + #undef TRAIT_FISH_TESTING