Skip to content

Commit

Permalink
[MIRROR] Machinery Destroy() side effect clean up (#2011)
Browse files Browse the repository at this point in the history
* Machinery Destroy() side effect clean up (#82659)

## About The Pull Request

I have combed over implementations of `Destroy()` for `obj/machinery`,
and noticed quite a few was spawning items or playing sounds.

**Slot machines**:
Moved payout to on_deconstruction()

**Windoors**:
Break sound moved to on_deconstruction().
I have also slightly cleaned up Destroy(), the windoor calls
air_update_turf directly, as that proc already retrieves the turf it is
on.
 
**Atmospheric pipe**:
Releases air and deconstructs meter objects on_deconstruction().

**Portable atmospheric devices**:
Drop hyper noblium crystal on on_destruction().

**Pump, Scrubbers**:
Releases air on_deconstruction().

**PACMAN power generator**:
Spawns dropped fuel on_deconstruction().

**Runic vendor**:
Moved vanishing effects to on_deconstruction().

I did not change Destroy side effects in the following instances:

- side effects are critical for the round (e.g. doomsday device, nuke,
blackbox recorder dropping the tape, gulag item reclaimer [less critical
but still])
- might spawn messages and noises, but moving them to on_deconstruct
would put linked items into an unusable state if deleted directly (e.g.
express order console, cyborg lockdown console, tram paired sensors)
- would potentially delete mobs we don't want deleted (e.g. disposals,
slime camera console)

Out of 220 Destroy defines, I found only 8 side effects that could not
be moved to other procs, so `machinery\Destroy()` has almost always been
used properly! I really hope `structure` will be as well made.

Other changes:

- Stasis beds had a completely empty destroy, removed
- Mass drivers had two destroy procs, merged

## Why It's Good For The Game

The Destroy() proc should only contain reference clean ups, barring edge
cases that would harm playability.

## Changelog

Nothing player facing.

* Machinery Destroy() side effect clean up

---------

Co-authored-by: Profakos <[email protected]>
  • Loading branch information
2 people authored and StealsThePRs committed Apr 16, 2024
1 parent 4777227 commit 912dbbc
Show file tree
Hide file tree
Showing 11 changed files with 38 additions and 39 deletions.
8 changes: 3 additions & 5 deletions code/game/machinery/doors/windowdoor.dm
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,8 @@

/obj/machinery/door/window/Destroy()
set_density(FALSE)
if(atom_integrity == 0)
playsound(src, SFX_SHATTER, 70, TRUE)
electronics = null
var/turf/floor = get_turf(src)
floor.air_update_turf(TRUE, FALSE)
air_update_turf(TRUE, FALSE)
return ..()

/obj/machinery/door/window/update_icon_state()
Expand Down Expand Up @@ -300,11 +297,12 @@
if(BURN)
playsound(src, 'sound/items/welder.ogg', 100, TRUE)


/obj/machinery/door/window/on_deconstruction(disassembled)
if(disassembled)
return

playsound(src, SFX_SHATTER, 70, TRUE)

for(var/i in 1 to shards)
drop_debris(new /obj/item/shard(src))
if(rods)
Expand Down
5 changes: 1 addition & 4 deletions code/game/machinery/mass_driver.dm
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@
. = ..()
wires = new /datum/wires/mass_driver(src)

/obj/machinery/mass_driver/Destroy()
QDEL_NULL(wires)
. = ..()

/obj/machinery/mass_driver/chapelgun
name = "holy driver"
id = MASSDRIVER_CHAPEL
Expand All @@ -35,6 +31,7 @@
for(var/obj/machinery/computer/pod/control as anything in SSmachines.get_machines_by_type_and_subtypes(/obj/machinery/computer/pod))
if(control.id == id)
control.connected = null
QDEL_NULL(wires)
return ..()

/obj/machinery/mass_driver/connect_to_shuttle(mapload, obj/docking_port/mobile/port, obj/docking_port/stationary/dock)
Expand Down
3 changes: 1 addition & 2 deletions code/game/machinery/slotmachine.dm
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,9 @@
coinvalues["[cointype]"] = C.get_item_credit_value()
qdel(C) //Sigh

/obj/machinery/computer/slot_machine/Destroy()
/obj/machinery/computer/slot_machine/on_deconstruction(disassembled)
if(balance)
give_payout(balance)
return ..()

/obj/machinery/computer/slot_machine/process(seconds_per_tick)
. = ..() //Sanity checks.
Expand Down
3 changes: 0 additions & 3 deletions code/game/machinery/stasis.dm
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@
. = ..()
AddElement(/datum/element/elevation, pixel_shift = 6)

/obj/machinery/stasis/Destroy()
. = ..()

/obj/machinery/stasis/examine(mob/user)
. = ..()
. += span_notice("Alt-click to [stasis_enabled ? "turn off" : "turn on"] the machine.")
Expand Down
13 changes: 11 additions & 2 deletions code/modules/atmospherics/machinery/other/meter.dm
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@

/obj/machinery/meter/Destroy()
SSair.stop_processing_machine(src)
target = null
if(!isnull(target))
UnregisterSignal(target, COMSIG_QDELETING)
target = null
return ..()

/obj/machinery/meter/Initialize(mapload, new_piping_layer)
Expand All @@ -45,8 +47,14 @@
candidate = pipe
if(candidate)
target = candidate
RegisterSignal(target, COMSIG_QDELETING, PROC_REF(drop_meter))
setAttachLayer(candidate.piping_layer)

///Called when the parent pipe is removed
/obj/machinery/meter/proc/drop_meter()
SIGNAL_HANDLER
deconstruct(FALSE)

/obj/machinery/meter/proc/setAttachLayer(new_layer)
target_layer = new_layer
PIPING_LAYER_DOUBLE_SHIFT(src, target_layer)
Expand Down Expand Up @@ -135,7 +143,8 @@
return TRUE

/obj/machinery/meter/on_deconstruction(disassembled)
new /obj/item/pipe_meter(loc)
var/obj/item/pipe_meter/meter_object = new /obj/item/pipe_meter(get_turf(src))
transfer_fingerprints_to(meter_object)

/obj/machinery/meter/interact(mob/user)
if(machine_stat & (NOPOWER|BROKEN))
Expand Down
15 changes: 5 additions & 10 deletions code/modules/atmospherics/machinery/pipes/pipes.dm
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,13 @@
if(hide)
AddElement(/datum/element/undertile, TRAIT_T_RAY_VISIBLE) //if changing this, change the subtypes RemoveElements too, because thats how bespoke works

/obj/machinery/atmospherics/pipe/Destroy()
QDEL_NULL(parent)

/obj/machinery/atmospherics/pipe/on_deconstruction(disassembled)
releaseAirToTurf()

var/turf/local_turf = loc
for(var/obj/machinery/meter/meter in local_turf)
if(meter.target != src)
continue
var/obj/item/pipe_meter/meter_object = new (local_turf)
meter.transfer_fingerprints_to(meter_object)
qdel(meter)
return ..()

/obj/machinery/atmospherics/pipe/Destroy()
QDEL_NULL(parent)
return ..()

//-----------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,17 @@
AddElement(/datum/element/climbable, climb_time = 3 SECONDS, climb_stun = 3 SECONDS)
AddElement(/datum/element/elevation, pixel_shift = 8)

/obj/machinery/portable_atmospherics/on_deconstruction(disassembled)
if(nob_crystal_inserted)
new /obj/item/hypernoblium_crystal(src)

return ..()

/obj/machinery/portable_atmospherics/Destroy()
disconnect()
air_contents = null
SSair.stop_processing_machine(src)

if(nob_crystal_inserted)
new /obj/item/hypernoblium_crystal(src)

return ..()

/obj/machinery/portable_atmospherics/examine(mob/user)
Expand Down
2 changes: 1 addition & 1 deletion code/modules/atmospherics/machinery/portable/pump.dm
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

volume = 1000

/obj/machinery/portable_atmospherics/pump/Destroy()
/obj/machinery/portable_atmospherics/pump/on_deconstruction(disassembled)
var/turf/local_turf = get_turf(src)
local_turf.assume_air(air_contents)
return ..()
Expand Down
6 changes: 3 additions & 3 deletions code/modules/atmospherics/machinery/portable/scrubber.dm
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
/datum/gas/halon,
)

/obj/machinery/portable_atmospherics/scrubber/Destroy()
var/turf/T = get_turf(src)
T.assume_air(air_contents)
/obj/machinery/portable_atmospherics/scrubber/on_deconstruction(disassembled)
var/turf/local_turf = get_turf(src)
local_turf.assume_air(air_contents)
return ..()

/obj/machinery/portable_atmospherics/scrubber/update_icon_state()
Expand Down
2 changes: 1 addition & 1 deletion code/modules/power/port_gen.dm
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
var/obj/S = sheet_path
sheet_name = initial(S.name)

/obj/machinery/power/port_gen/pacman/Destroy()
/obj/machinery/power/port_gen/pacman/on_deconstruction(disassembled)
DropFuel()
return ..()

Expand Down
11 changes: 6 additions & 5 deletions code/modules/vending/runic_vendor.dm
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
vend_reply = "Please, stand still near the vending machine for your special package!"
resistance_flags = FIRE_PROOF
light_mask = "RunicVendor-light-mask"
obj_flags = parent_type::obj_flags | NO_DEBRIS_AFTER_DECONSTRUCTION
/// How long the vendor stays up before it decays.
var/time_to_decay = 30 SECONDS
/// Area around the vendor that will pushback nearby mobs.
Expand Down Expand Up @@ -60,15 +61,16 @@

return .

/obj/machinery/vending/runic_vendor/handle_deconstruct(disassembled)
SHOULD_NOT_OVERRIDE(TRUE)

/obj/machinery/vending/runic_vendor/Destroy()
visible_message(span_warning("[src] flickers and disappears!"))
playsound(src,'sound/weapons/resonator_blast.ogg',25,TRUE)
return ..()

/obj/machinery/vending/runic_vendor/proc/runic_explosion()
explosion(src, light_impact_range = 2)
qdel(src)
deconstruct(FALSE)

/obj/machinery/vending/runic_vendor/proc/runic_pulse()
var/pulse_locs = spiral_range_turfs(pulse_distance, get_turf(src))
Expand All @@ -82,10 +84,9 @@
mob_to_be_pulsed_back.throw_at(target, 4, 4)

/obj/machinery/vending/runic_vendor/screwdriver_act(mob/living/user, obj/item/I)
explosion(src, light_impact_range = 2)
qdel(src)
runic_explosion()

/obj/machinery/vending/runic_vendor/proc/decay()
qdel(src)
deconstruct(FALSE)

#undef PULSE_DISTANCE_RANGE

0 comments on commit 912dbbc

Please sign in to comment.