Fixes seed extractors not taking seeds from plant bags. (#68842)

* Fixes seed extractors not taking seeds from plant bags.
- When refactored, it accidently changed it from taking the seed "from its loc" to "from the extractor itself", which always failed

* Unit test

* Genericises it a bit

code/modules/hydroponics/seed_extractor.dm

@@ -105,43 +105,57 @@
 	default_unfasten_wrench(user, tool)
 	return TOOL_ACT_TOOLTYPE_SUCCESS
 
-/obj/machinery/seed_extractor/attackby(obj/item/O, mob/living/user, params)
-
-	if(default_deconstruction_screwdriver(user, "sextractor_open", "sextractor", O))
-		return
-
-	if(default_pry_open(O))
-		return
-
-	if(default_deconstruction_crowbar(O))
-		return
-
-	if(istype(O, /obj/item/storage/bag/plants))
-		var/obj/item/storage/P = O
-		var/loaded = 0
-		for(var/obj/item/seeds/G in P.contents)
-			if(contents.len >= max_seeds)
-				break
-			++loaded
-			add_seed(G)
-		if (loaded)
-			to_chat(user, span_notice("You put as many seeds from \the [O.name] into [src] as you can."))
-		else
-			to_chat(user, span_notice("There are no seeds in \the [O.name]."))
-		return
-
-	else if(seedify(O,-1, src, user))
-		to_chat(user, span_notice("You extract some seeds."))
-		return
-	else if (istype(O, /obj/item/seeds))
-		if(add_seed(O))
-			to_chat(user, span_notice("You add [O] to [src.name]."))
-		return
-	else if(!user.combat_mode)
-		to_chat(user, span_warning("You can't extract any seeds from \the [O.name]!"))
-	else
+/obj/machinery/seed_extractor/attackby(obj/item/attacking_item, mob/living/user, params)
+	if(!isliving(user) || user.combat_mode)
 		return ..()
 
+	if(default_deconstruction_screwdriver(user, "sextractor_open", "sextractor", attacking_item))
+		return TRUE
+
+	if(default_pry_open(attacking_item))
+		return TRUE
+
+	if(default_deconstruction_crowbar(attacking_item))
+		return TRUE
+
+	if(istype(attacking_item, /obj/item/storage/bag/plants))
+		var/loaded = 0
+		for(var/obj/item/seeds/to_store in attacking_item.contents)
+			if(contents.len >= max_seeds)
+				to_chat(user, span_warning("[src] is full."))
+				break
+			if(!add_seed(to_store, attacking_item))
+				continue
+			loaded += 1
+
+		if(loaded)
+			to_chat(user, span_notice("You put as many seeds from [attacking_item] into [src] as you can."))
+		else
+			to_chat(user, span_warning("There are no seeds in [attacking_item]."))
+
+		return TRUE
+
+	if(seedify(attacking_item, -1, src, user))
+		to_chat(user, span_notice("You extract some seeds."))
+		return TRUE
+
+	else if(istype(attacking_item, /obj/item/seeds))
+		if(contents.len >= max_seeds)
+			to_chat(user, span_warning("[src] is full."))
+
+		else if(add_seed(attacking_item, user))
+			to_chat(user, span_notice("You add [attacking_item] to [src]."))
+
+		else
+			to_chat(user, span_warning("You can't seem to add [attacking_item] to [src]."))
+		return TRUE
+
+	else if(!attacking_item.tool_behaviour) // Using the wrong tool shouldn't assume you want to turn it into seeds.
+		to_chat(user, span_warning("You can't extract any seeds from [attacking_item]!"))
+		return TRUE
+
+	return ..()
+
 /**
  * Generate seed string
  *
@@ -161,27 +175,26 @@
  * Adds the seeds to the contents and to an associated list that pregenerates the data
  * needed to go to the ui handler
  *
+ * to_add - what seed are we adding?
+ * taking_from - where are we taking the seed from? A mob, a bag, etc?
+ * user - who is inserting the seed?
  **/
-/obj/machinery/seed_extractor/proc/add_seed(obj/item/seeds/O)
-	if(contents.len >= 999)
-		to_chat(usr, span_notice("\The [src] is full."))
+/obj/machinery/seed_extractor/proc/add_seed(obj/item/seeds/to_add, atom/taking_from)
+	if(ismob(taking_from))
+		var/mob/mob_loc = taking_from
+		if(!mob_loc.transferItemToLoc(to_add, src))
+			return FALSE
+
+	else if(!taking_from.atom_storage?.attempt_remove(to_add, src, silent = TRUE))
 		return FALSE
 
-	if(atom_storage)
-		if(!atom_storage.attempt_remove(O, src, silent = TRUE))
-			return FALSE
-	else if(ismob(O.loc))
-		var/mob/M = O.loc
-		if(!M.transferItemToLoc(O, src))
-			return FALSE
-
-	var/seed_string = generate_seed_string(O)
+	var/seed_string = generate_seed_string(to_add)
 	if(piles[seed_string])
-		piles[seed_string] += WEAKREF(O)
+		piles[seed_string] += WEAKREF(to_add)
 	else
-		piles[seed_string] = list(WEAKREF(O))
+		piles[seed_string] = list(WEAKREF(to_add))
 
-	. = TRUE
+	return TRUE
 
 /obj/machinery/seed_extractor/ui_state(mob/user)
 	return GLOB.notcontained_state

code/modules/unit_tests/_unit_tests.dm

@@ -98,6 +98,7 @@
 #include "heretic_knowledge.dm"
 #include "heretic_rituals.dm"
 #include "holidays.dm"
+#include "hydroponics_extractor_storage.dm"
 #include "hydroponics_harvest.dm"
 #include "hydroponics_self_mutations.dm"
 #include "hydroponics_validate_genes.dm"

code/modules/unit_tests/hydroponics_extractor_storage.dm

@@ -0,0 +1,70 @@
+/// Unit test to ensure seeds can properly be added to the plant seed extractor through multiple methods.
+/// This only tests transferring seeds to the storage, it does NOT test creating seeds.
+/datum/unit_test/hydroponics_extractor_storage
+
+/datum/unit_test/hydroponics_extractor_storage/Run()
+	var/obj/machinery/seed_extractor/extractor = allocate(/obj/machinery/seed_extractor)
+	var/mob/living/carbon/human/dummy = allocate(/mob/living/carbon/human)
+
+	var/obj/item/storage/bag/plants/storage = allocate(/obj/item/storage/bag/plants)
+
+	// Set up all the seeds we're gonna test storing
+	var/list/seeds_to_put_in_the_bag = list(/obj/item/seeds/orange, /obj/item/seeds/lemon)
+	var/num_seeds_to_make_of_each = 5
+	// Put some seeds in the bag, based on the above variables
+	for(var/i in 1 to num_seeds_to_make_of_each)
+		for(var/seed_type in seeds_to_put_in_the_bag)
+			var/obj/item/seeds/new_seed = new seed_type(dummy.loc)
+			storage.atom_storage.attempt_insert(to_insert = new_seed, user = dummy)
+
+	// Store the number of seeds we start with in the bag for later.
+	var/num_seeds_starting_with = length(storage.contents)
+	TEST_ASSERT_EQUAL(num_seeds_starting_with, num_seeds_to_make_of_each * length(seeds_to_put_in_the_bag), "The plant bag failed to populate itself with [num_seeds_to_make_of_each] of each seed.")
+
+	// Put 1 seed into the dummy's hand
+	// If they fail to pick up the seed, we have an issue
+	var/obj/item/seeds/apple/apple_seed = new(dummy.loc)
+	if(!dummy.put_in_active_hand(apple_seed))
+		return TEST_FAIL("The dummy failed to pick up the apple seed.")
+
+	// Okay, all our seeds are setup, let's try to insert them
+	apple_seed.melee_attack_chain(dummy, extractor)
+	// The apple seed should not be in our hands anymore
+	TEST_ASSERT_NOTEQUAL(dummy.get_active_held_item(), apple_seed, "The dummy failed to insert a singular seed into the plant seed extractor.")
+
+	// The apple seed should be in the seed extractor now
+	var/obj/item/seeds/apple/apple_now_stored = locate() in extractor
+	TEST_ASSERT_NOTNULL(apple_now_stored, "The apple seed was removed from the dummy's hands, but is not in the plant seed extractor's contents.")
+
+	// The apple seed's key should be in the extractor's "piles" list
+	var/apple_seed_key = extractor.generate_seed_string(apple_now_stored)
+	TEST_ASSERT(apple_seed_key in extractor.piles, "The apple seed was added to the plant seed extractor's contents correctly, but did not register in the piles list, and is unaccessible.")
+
+	// And it should be tracked in the piles list as a weakref
+	TEST_ASSERT_EQUAL(length(extractor.piles[apple_seed_key]), 1, "While 1 apple seed was added to the plant seed extractor, its weakref was not added to the piles list correctly.")
+
+	// Let's test the plant bag now.
+	// If they fail to pick up the bag, we have an issue.
+	if(!dummy.put_in_active_hand(storage))
+		return TEST_FAIL("The dummy failed to pick up the plant bag.")
+
+	storage.melee_attack_chain(dummy, extractor)
+	// We should have 0 seeds remaining in the bag itself.
+	var/num_seeds_remaining = length(storage.contents)
+	// If the number of seeds in the bag unchanged, no seeds moved, we have an issue as they all failed to move
+	TEST_ASSERT(num_seeds_remaining < num_seeds_starting_with, "The plant bag transferred no seeds to the plant seed extractor. (Started with [num_seeds_starting_with], ended with [num_seeds_remaining])")
+	// If the number of seeds in the bag went down, but is not 0, we have an issue as some failed to move
+	TEST_ASSERT(num_seeds_remaining <= 0, "The plant bag still had [num_seeds_remaining] seeds remaining of the [num_seeds_starting_with] it started with after transferring its seeds to the plant seed extractor.")
+
+	for(var/obj/item/seeds/seed_type as anything in seeds_to_put_in_the_bag)
+
+		// All seeds should be in the extractor now
+		var/obj/item/seeds/seed_now_stored = locate(seed_type) in extractor
+		TEST_ASSERT_NOTNULL(seed_now_stored, "The plant bag transferred its [initial(seed_type.name)] somewhere, but they were not found in the plant seed extractor.")
+
+		// All keys shold be independently in the piles list
+		var/stored_seed_key = extractor.generate_seed_string(seed_now_stored)
+		TEST_ASSERT(stored_seed_key in extractor.piles, "The [initial(seed_type.name)] was added to the plant seed extractor's contents correctly, but did not register in the piles list, and is unaccessible.")
+
+		// And all seeds should be tracked as weakrefs
+		TEST_ASSERT_EQUAL(length(extractor.piles[stored_seed_key]), num_seeds_to_make_of_each, "While [num_seeds_to_make_of_each] [initial(seed_type.name)]s were added to the plant seed extractor, not all weakrefs were added to the piles list correctly.")