search

ParadiseSS13 / Paradise

Paradise Station's GitHub main repository.
https://www.paradisestation.org/forum
GNU Affero General Public License v3.0
405 stars 1.2k forks source link

Terror eggs, traitor spider eggs, cluwne organs, and MMI Holders all runtime on removal #20846

Closed Contrabang closed 1 year ago

Contrabang commented 1 year ago

Issue Description:

terror eggs, traitor spider eggs, cluwne organs, and MMI Holders all apparently runtime on removal. Found this out via codediving after seeing a weird surgery runtime in the logs. These are all organs with the flag destroy_on_removal.

Runtime

[2023-04-09T05:27:43] Runtime in robotics.dm,508: Cannot execute null.forceMove().
[2023-04-09T05:27:43]   proc name: end step (/datum/surgery_step/robotics/manipulate_robotic_organs/extract/end_step)
[2023-04-09T05:27:43]   usr: *REDACTED USER*
[2023-04-09T05:27:43]   usr.loc: The floor (114,146,2) (/turf/simulated/floor/plasteel)
[2023-04-09T05:27:43]   src: extract cybernetic organ (/datum/surgery_step/robotics/manipulate_robotic_organs/extract)
[2023-04-09T05:27:43]   call stack:
[2023-04-09T05:27:43]   extract cybernetic organ (/datum/surgery_step/robotics/manipulate_robotic_organs/extract): end_step
[2023-04-09T05:27:43]   extract cybernetic organ (/datum/surgery_step/robotics/manipulate_robotic_organs/extract): initiate
[2023-04-09T05:27:43]   extract cybernetic organ (/datum/surgery_step/robotics/manipulate_robotic_organs/extract): try_op
[2023-04-09T05:27:43]   manipulate robotic organs (pro... (/datum/surgery_step/proxy/robotics/manipulate_organs): try_next_step
[2023-04-09T05:27:43]   manipulate robotic organs (pro... (/datum/surgery_step/proxy/robotics/manipulate_organs): try_op
[2023-04-09T05:27:43]   Internal Component Manipulatio... (/datum/surgery/robotics/cybernetic_repair/internal): next_step
[2023-04-09T05:27:43]   *Hidden1* (/mob/living/carbon/human): attackby
[2023-04-09T05:27:43]   the multitool (/obj/item/multitool): melee_attack_chain
[2023-04-09T05:27:43]   *Hidden2* (/mob/living/carbon/human): ClickOn
[2023-04-09T05:27:43]   *Hidden1* (/mob/living/carbon/human): Click

Why is this bad/What are the consequences:

Removing brains thankfully still works, but these runtimes shouldnt happen.

Steps to reproduce the problem:

Remove posibrain from an IPC

When did the problem start happening:

7 months ago, when #18325 and #18624

Extra information:

Relevant code: destroy_on_removal flag. https://github.com/ParadiseSS13/Paradise/blob/6ac3c0a90f4c8ae021b56bca49ae1216e81be74b/code/modules/surgery/organs/mmi_holder.dm#L2-L8

destroy_on_removal causes it to not return null here in the remove() proc https://github.com/ParadiseSS13/Paradise/blob/6ac3c0a90f4c8ae021b56bca49ae1216e81be74b/code/modules/surgery/organs/organ.dm#L260-L264

Returning null causes it to call null.forceMove() https://github.com/ParadiseSS13/Paradise/blob/6ac3c0a90f4c8ae021b56bca49ae1216e81be74b/code/modules/surgery/robotics.dm#L506-L511

An easy fix would be something like (maybe):

var/obj/item/thing = I.remove(target)
if(!thing)
        return ..()
if(!istype(thing))
    thing.forceMove(get_turf(target))
else
    user.put_in_hands(thing)
return ..()
Contrabang commented 1 year ago

Fixed by #21880