From fd7e71d0b449f77b3b4aff57ac69c36a20394b6f Mon Sep 17 00:00:00 2001 From: warriorstar-orion Date: Sun, 7 Sep 2025 01:57:02 -0400 Subject: [PATCH] fix cooking step failures (#30349) * fix cooking step failures * a timing issue one hopes --- code/modules/cooking/recipe_tracker.dm | 63 +++++++++++++++++------ code/modules/cooking/steps/recipe_step.dm | 9 +++- code/tests/_game_test_puppeteer.dm | 5 ++ code/tests/game_tests.dm | 1 + code/tests/test_cooking.dm | 53 +++++++++++++++++++ 5 files changed, 114 insertions(+), 17 deletions(-) create mode 100644 code/tests/test_cooking.dm diff --git a/code/modules/cooking/recipe_tracker.dm b/code/modules/cooking/recipe_tracker.dm index e6dd4237070..e202c5567ab 100644 --- a/code/modules/cooking/recipe_tracker.dm +++ b/code/modules/cooking/recipe_tracker.dm @@ -52,6 +52,24 @@ /// Core function that checks if a object meets all the requirements for certain /// recipe actions. +/// +/// This is one of the thornier and grosser parts of the cooking system and most +/// people working with it or implementing recipes should never have to look at +/// this. The core idea is: +/// +/// * we keep track of what recipes are still valid outcomes by testing the used +/// item against the list of recipes which are valid so far. +/// * each valid recipe is at a certain step, and check the used object against +/// [/datum/cooking/recipe_step/proc/check_conditions_met]. if we meet the +/// conditions, we track the recipe and the step. +/// * for each unique step type that we're tracking, call +/// [/datum/cooking/recipe_step/proc/follow_step] on the first instance of +/// that step type, then [/datum/cooking/recipe_step/proc/is_complete] on +/// all recipe step instances of that type, to see if we advance their +/// respective recipes. +/// +/// Once a recipe reaches its final step, the tracker completes the recipe and +/// typically stops existing at that point. /datum/cooking/recipe_tracker/proc/process_item(mob/user, obj/used) // TODO: I *hate* passing in a user here and want to move all the necessary // UI interactions (selecting which recipe to complete, selecting which step @@ -61,8 +79,6 @@ var/list/completed_recipes = list() var/list/silent_recipes = list() var/list/attempted_step_per_recipe = list() - var/datum/cooking/recipe_step/use_step_type - for(var/datum/cooking/recipe/recipe in recipes_last_completed_step) var/current_idx = recipes_last_completed_step[recipe] @@ -73,11 +89,9 @@ next_step = recipe.steps[++current_idx] var/conditions = next_step.check_conditions_met(used, src) if(conditions == PCWJ_CHECK_VALID) - LAZYADD(valid_steps[next_step], next_step) + LAZYADD(valid_steps[next_step.type], next_step) LAZYADD(valid_recipes[next_step.type], recipe) attempted_step_per_recipe[recipe] = current_idx - if(!use_step_type) - use_step_type = next_step.type match = TRUE break else if(conditions == PCWJ_CHECK_SILENT) @@ -94,16 +108,33 @@ return PCWJ_PARTIAL_SUCCESS return PCWJ_NO_STEPS - var/datum/cooking/recipe_step/sample_step = valid_steps[1] - var/step_data = sample_step.follow_step(used, src) - step_reaction_message = step_data["message"] + var/list/recipes_with_completed_steps = list() + var/list/step_data var/complete_steps = 0 - for(var/i in 1 to length(valid_recipes[use_step_type])) - var/datum/cooking/recipe/recipe = valid_recipes[use_step_type][i] - var/datum/cooking/recipe_step/recipe_step = valid_steps[i] - if(recipe_step.is_complete(used, src, step_data)) - recipes_last_completed_step[recipe] = attempted_step_per_recipe[recipe] - complete_steps++ + for(var/step_type in valid_steps) + // For each valid step type we only call follow_step() once since it's + // pointless to e.g. add an item to the container more than once. + // + // However, we are still calling follow_step more than once. which means + // we have to deal with the possibility that two valid steps may do two + // different things with the used item and may expect different results. + // Sojurn tried to handle this by adding a user prompt at this point, + // asking which step the player wanted to perform. I want to avoid + // throwing up interfaces during cooking, especially when unexpected, so + // for now, we do nothing, and just watch out for situations where two + // different recipe steps with incompatible end states are valid with + // the same object. + var/datum/cooking/recipe_step/sample_step = valid_steps[step_type][1] + step_data = sample_step.follow_step(used, src) + step_reaction_message = step_data["message"] + + for(var/i in 1 to length(valid_recipes[step_type])) + var/datum/cooking/recipe/recipe = valid_recipes[step_type][i] + var/datum/cooking/recipe_step/recipe_step = valid_steps[step_type][i] + if(recipe_step.is_complete(used, src, step_data)) + recipes_last_completed_step[recipe] = attempted_step_per_recipe[recipe] + recipes_with_completed_steps |= recipe + complete_steps++ var/obj/item/reagent_containers/cooking/container = locateUID(container_uid) if(complete_steps) @@ -121,8 +152,8 @@ else return PCWJ_PARTIAL_SUCCESS - for(var/datum/cooking/recipe in recipes_last_completed_step) - if(!(recipe in valid_recipes[sample_step.type])) + for(var/recipe in recipes_last_completed_step) + if(!(recipe in recipes_with_completed_steps)) recipes_last_completed_step -= recipe var/datum/cooking/recipe/recipe_to_complete diff --git a/code/modules/cooking/steps/recipe_step.dm b/code/modules/cooking/steps/recipe_step.dm index 3cde37ae34d..313c7e34a1b 100644 --- a/code/modules/cooking/steps/recipe_step.dm +++ b/code/modules/cooking/steps/recipe_step.dm @@ -8,6 +8,12 @@ RESTRICT_TYPE(/datum/cooking/recipe_step) if("optional" in options) optional = options["optional"] +/// See if the *used_item* meets the conditions for this recipe step. This will +/// typically be something like ensuring that a recipe step for adding a +/// specific kind of item has been passed an item of that type. +/// +/// Returns one of [PCWJ_CHECK_INVALID], [PCWJ_CHECK_VALID], [PCWJ_CHECK_FULL], +/// [PCWJ_CHECK_SILENT]. /datum/cooking/recipe_step/proc/check_conditions_met(obj/used_item, datum/cooking/recipe_tracker/tracker) SHOULD_CALL_PARENT(FALSE) SHOULD_BE_PURE(TRUE) @@ -26,7 +32,8 @@ RESTRICT_TYPE(/datum/cooking/recipe_step) /datum/cooking/recipe_step/proc/follow_step(obj/used_item, datum/cooking/recipe_tracker/tracker, mob/user) return list() -/// Special function to check if the step has been satisfied. Sometimed just following the step is enough, but not always. +/// Special function to check if the step has been satisfied. Sometimes just +/// following the step is enough, but not always. /datum/cooking/recipe_step/proc/is_complete(obj/added_item, datum/cooking/recipe_tracker/tracker, list/step_data) return TRUE diff --git a/code/tests/_game_test_puppeteer.dm b/code/tests/_game_test_puppeteer.dm index 2754757559f..05db0cbd286 100644 --- a/code/tests/_game_test_puppeteer.dm +++ b/code/tests/_game_test_puppeteer.dm @@ -73,6 +73,11 @@ puppet.next_click = world.time puppet.next_move = world.time +/datum/test_puppeteer/proc/alt_click_on(target, params) + var/plist = params2list(params) + plist["alt"] = TRUE + click_on(target, list2params(plist)) + /datum/test_puppeteer/proc/spawn_mob_nearby(mob_type) for(var/turf/T in RANGE_TURFS(1, puppet)) if(!T.is_blocked_turf()) diff --git a/code/tests/game_tests.dm b/code/tests/game_tests.dm index 91aa459a6d4..68b7879a366 100644 --- a/code/tests/game_tests.dm +++ b/code/tests/game_tests.dm @@ -27,6 +27,7 @@ #include "test_apc_construction.dm" #include "test_components.dm" #include "test_config_sanity.dm" +#include "test_cooking.dm" #include "test_crafting_lists.dm" #include "test_dynamic_budget.dm" #include "test_elements.dm" diff --git a/code/tests/test_cooking.dm b/code/tests/test_cooking.dm new file mode 100644 index 00000000000..eb37314f807 --- /dev/null +++ b/code/tests/test_cooking.dm @@ -0,0 +1,53 @@ +/datum/cooking/recipe/test_soylent + container_type = /obj/item/reagent_containers/cooking/pot + product_type = /obj/item/food/soylentgreen + steps = list( + PCWJ_ADD_ITEM(/obj/item/food/meat/human), + PCWJ_ADD_ITEM(/obj/item/food/meat/human), + PCWJ_ADD_REAGENT("water", 10), + PCWJ_USE_STOVE(J_MED, 1 SECONDS), + ) + appear_in_default_catalog = FALSE + +/datum/game_test/room_test/cooking/Run() + var/datum/test_puppeteer/player = new(src) + player.puppet.name = "Player" + + // Burger + var/obj/structure/table/table = player.spawn_obj_nearby(__IMPLIED_TYPE__, SOUTH) + var/obj/item/reagent_containers/cooking/board/board = player.spawn_obj_nearby(__IMPLIED_TYPE__, SOUTH) + player.spawn_obj_in_hand(/obj/item/food/bun) + player.click_on(board) + TEST_ASSERT_LAST_CHATLOG(player, "You add the bun") + player.spawn_obj_in_hand(/obj/item/food/meat/patty) + player.click_on(board) + TEST_ASSERT_LAST_CHATLOG(player, "You add the patty") + player.spawn_obj_in_hand(/obj/item/food/grown/lettuce) + player.click_on(board) + TEST_ASSERT_LAST_CHATLOG(player, "You finish cooking with the cutting board") + player.alt_click_on(board) + TEST_ASSERT(locate(/obj/item/food/burger) in get_turf(board), "could not find completed burger") + + // Soylent + var/obj/machinery/cooking/stovetop/stove = player.spawn_obj_nearby(__IMPLIED_TYPE__, EAST) + var/obj/item/reagent_containers/cooking/pot/pot = player.spawn_obj_in_hand(__IMPLIED_TYPE__) + player.click_on(table) + var/obj/item/food/meat/human/meat = player.spawn_obj_in_hand(__IMPLIED_TYPE__) + player.click_on(pot) + TEST_ASSERT_LAST_CHATLOG(player, "You add [meat]") + player.spawn_obj_in_hand(/obj/item/food/meat/human) + player.click_on(pot) + var/obj/item/reagent_containers/glass/beaker/beaker = player.spawn_obj_in_hand(__IMPLIED_TYPE__) + beaker.reagents.add_reagent("water", 10) + player.click_on(pot) + player.put_away(beaker) + player.click_on(pot) + player.click_on(stove, "icon-x=18&icon-y=18") + var/surface_idx = stove.clickpos_to_surface(list("icon-x" = 18, "icon-y" = 18)) + var/datum/cooking_surface/surface = stove.surfaces[surface_idx] + surface.temperature = J_MED + surface.turn_on(player.puppet) + sleep(3 SECONDS) + surface.turn_off(player.puppet) + player.alt_click_on(stove, "icon-x=18&icon-y=18") + TEST_ASSERT(locate(/obj/item/food/soylentgreen) in stove.loc, "could not find complete soylent")