Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Random Processing Reviews: Part 1 (#2962)
* /obj/Destroy * Actually does the long overdue TODO * SA * 2020-11-10 commit #1 Case #1: /obj/item/tk_grab - Unproblematic, fastprocess → icon_update; Do we really need this tho? - Focus = null for safety Case #2: /obj/screen/alert/bloodsense/process() - (will be repathed to /atom/movable/screen) - SSprocessing → long-ass process() → GUI stuff; do we really need animate()? - left unchanged Case #3: /obj/screen/alert/clockwork/clocksense/process() - SSprocessing → short-ish process() → Basic desc - (creation and removal is sane) - Wrap-up: /obj/screen/alert/Destroy() * Removed duplicated `master = null` * Proper return ..() position on * /obj/screen/alert/clockwork/clocksense/Destroy() & * /obj/screen/alert/Destroy() Case #4: /datum/proc/process() - automatic killer for aimless START_PROCESS - obviously left unchanged Case #5: /datum/action/proc/Process() - A proc to be overriden? Why? - Removed; we already have /datum/proc/process() Case #6: /datum/action/cooldown/process() - return PROCESS_KILL Case #7: /datum/action/item_action/chameleon/change/process() - return PROCESS_KILL Case #8: /datum/forced_movement - Looks alright Case #9: /datum/point/vector/processed - Is this shit even used??? - Left as is since it might be used in the future (inb4 staying unused til 2025) * 2020-11-12 Case 10: /datum/radiation_wave/process() - Literally the centerpiece of radiation - Each process step progresses radiation wave by one tile * … and the place it used be is then filled by subsequent wave - Kat requested fix of tgstation/tgstation#42572 * I will start working on it soon(tm) - The code seems fine otherwise Case 11: /obj/effect/hallucination/simple/securitron - Responsible for making securitron hallucination lifelike - Too simple to review, IMO - victim = null for safety Case 12: /datum/component/chasm/process() - Responsible for “dropping” things every now and then, very greedily - I am not going to refactor this obviously - START_PROCESSING actually has sanity check, no problem with double START_PROCESSING - return PROCESS_KILL Case 13: /datum/component/lockon_aiming/process() - Responsible for keeping locks sane - Looks sane, left as is Case 14: /datum/component/manual_blinking/process() - Adminbus shit for now - Responsible for warnings and damage - Looks sane, left as is - *Even checks for life and eye-ness of parent* Case 15: /datum/component/manual_breathing/process() - Mostly ditto Case 16: /datum/component/mood/process() - Responsible for sanity, nutrition, and hygiene’s miasma production - NO MORE INFINITE SANITY AHHHHH * 2020-11-18 Commit 1 Case 17: /datum/component/nanites/process() - Responsible for nanites. - Is its own family; the /datum/nanite_program/proc/on_process() * Handles so much but only one on_process() * Things are generally good except some fuzzy cases where * Host is ded * Host is invalid as hell * etc etc * But I don’t care; LGTM Case 18: /datum/component/radioactive/process() - Responsible for making irradiating objects irradiate - THE GODDAMN prob(50) * wontfix - We don’t need return PROCESS_KILL here since it qdels anyway * … and Destroy() shouldn’t meaninglessly STOP_PROCESSING Case 19: /datum/component/rot/process() - Responsible for miasma production for whatever that rots - For the love of god, write actual real Destroy() Case 20: /datum/component/rot/corpse/process() - Special handling for corpse rots - Seems good Case 21: /datum/component/spawner/process() - Spawns mobs, yay - stop_spawning(force) is in lieu of Destroy, mostly * Are we sure that the spawner never dies otherwise? I don’t know. Case 22: /datum/component/wet_floor/process() - time_left_list doesn’t look sane; will investigate further * Nah, everything works as intended * I am not sure if we need bitflag and assoc list tortures Case 23: /datum/component/plumbing/process() - return PROCESS_KILL * IMO it shouldn’t be reachable in the first place * Hey does this work well with GC? (probably yes) - STOP_PROCESSING in disable() is alright * Update code/game/objects/objs.dm For clarity...! Parenthesis FTW Co-authored-by: ike709 <ike709@users.noreply.github.com> * 2020-11-21 Commit 1 Case 24: /datum/component/plumbing/acclimator/process() - Makes acclimator do nothing when empyting - LGTM Case 25: /obj/structure/alien/flesh/node/process() - Hey, it is that weird pink skin stuff - Controls flesh expansion through expand() - LGTM Case 26: /datum/element/earhealing/process() - Only used by earmuffs - I think we have enough tickchecks - I should stop before looking at every CHECK_TICK usage and * just walk away AHHHHHHHHHHHHHH Case 27: /datum/status_effect/process() - Responsible for qdeling status effects as needed - LGTM Case 28: /datum/quirk/process() - It too is its own family: on_process() - /datum/quirk/jolly/on_process() * Sends positive mood event really really rarely: * once per avg. 2000 seconds, yet the mood effect only lasts 2 minutes * Seems a bit too rare for me, but now isn’t a time for that - /datum/quirk/spiritual/on_process() * Actually really loops through every *living thing* nearby * ‘Holy people’ here is chaplain, inquisitor ERT chaplain & commander * … and people who used syndicate bible on themselves * From now on only real human can appear holy to spiritual people * Like, don’t be comforted by holy monkeys or roaches or something - /datum/quirk/neet/on_process() * Checks hygiene for NEET stuff * I think I should start considering hygiene signals - /datum/quirk/badback/on_process() * I want to replace this with signal but quirks aren’t components * So it gets to live - /datum/quirk/blooddeficiency/on_process() * Just removes blood from the holder, oof * LGTM - /datum/quirk/brainproblems/on_process() * Just damages brain over time * LGTM - /datum/quirk/family_heirloom/on_process() * Constantly checks whether the holder has the heirloom * LGTM - /datum/quirk/nyctophobia/on_process() * Oh boy can’t we just change the type of the quirk_holder? * Handles nyctophobia mood and makes nyctophboes walk in the dark (?!) * LGTM - /datum/quirk/insanity/on_process() * Makes holder go mad when they have no mindbreaker toxin * Feels like madness() call is unnecessary - /datum/quirk/social_anxiety/on_process() * Ah yes, the *thing* * Code is a l r i g h t , t r u s t m e - /datum/quirk/junkie/on_process() * Piggybacks on addiction mechanics * Mostly just keeps persistent addiction going * LGTM - /datum/quirk/junkie/smoker/on_process() * Picky smokers... * LGTM - /datum/quirk/neat/on_process() * NEET, but… much less asocial * LGTM * 2020-11-22 Commit 1 Case 29: /area/process() - OwO wats dis, Cyberboss? - tgstation/tgstation#23566 - Saner placement of START_PROCESSING and STOP_PROCESSING Case 30: /datum/game_mode/process() - Base proc for game_mode process()s - Not that necessary but I am leaving this for doc * I should consider giving game mode process()s a new name - We shouldn’t return; return value isn’t even used lmao Case 31: /obj/item/clothing/shoes/clown_shoes/banana_shoes/combat/process() - Responsible recharging the combat bananium shoes - I think I can refactor this away but I am too lazy * TODO - Don’t forget the Destroy() Case 32: /datum/game_mode/dynamic/process() - I am not touching this since * people are tinkering with dynamic gamemode * and I don't like dynamemes - LGTM(?) anyway Case 33: /obj/machinery/dominator/process() - WHY ..() - Breaking kills process anyway - return PROCESS_KILL is actually more important for machines Case 34: /datum/game_mode/meteor/process() - I'll tell you about the cursed thing about compile time optimizations * (var * 10 * 10) is *slower* than (var * (10 * 10)) - I have no idea why it looks like this, but I am leaving this * for posterity? Idk Case 35: /datum/game_mode/revolution/process() - Culls unnecessary returns; let’s not confuse coders - Removes != 0 - return values are a batshit. I need to take care of this… * in another PR, probably, soon-ish Case 36: /datum/game_mode/revolution/speedy/process() - We don’t return Case 37: /obj/machinery/process() - Automatic killer a la /datum/process - The truth is, we don’t need this * Postmortem: This code is a ridiculous heap of legacy * This automatic killer is actually older(8 y/o) * ...than the killer at datum(5 y/o) * This thing should’ve been phased out 5 years ago - Proper STOP_PROCESSING - Guess it would be good to unify normal processing and SSmachines more Postscript: I will Signal-lize badback quirk next commit * 2020-11-24 Commit 1 Important: - Bad back on_process won’t be signalized away since * there aren’t appropriate signals * and I don’t want to write another signal (COMSIG_MOB_EQUIP?) * …unless I somehow find a few other probable use cases - porting tgstation/tgstation#51392 should be a different PR - game_mode process() should be left alone Case 38: /obj/machinery/airlock_sensor/process() - responsible for constant input of air info… like a good sensor - DOES THIS GET TURNED OFF IN THE FIRST PLACE - update_icon() on demand Case 39: /obj/machinery/advanced_airlock_controller/process() - ..() in process() IS NOT A GOOD IDEA MOST OF THE TIME * In this case it kills the process, rendering this process moot - BeeStation/BeeStation-Hornet#2059 ← how did this happen to Bee - yogstation13/Yogstation#7906 ← the culprit - Oh god oh fuck I am in pain why are doing this to me monster860 - Every time I review through a handful of cases to prepare a commit * I am subjected to a constant pain - Trilemma ensues: * Keep doing this really slowly * Stop this shit right now * Be masochist - This fix is going to create a tiny(?) bit of lag Case 40: /obj/machinery/computer/bank_machine/process() - /obj/machinery/computer/process() is a whole new kind of weird shit * It is to be used to check whether it should run process() or not but * This proc doesn’t follow that lmao - More and more I investigate the machine code * more and more I lose hope in humanity - This process is responsible for siphoning process - Early return for having no power; who didn’t think of this? Okay, let’s wrap up this batshit: - Currently, BROKEN flag is misused * The checks are all over the place * BROKENness prevents interaction whilst it still eats energy * Probably other shits will be dug up once I take a serious look - Power consumption depends on process() * I am not joking they really do * So idle machines still get process()ed * And power consumption isn’t very well controlled in general - The technical debt here is damn too high just kill me - So things won’t be fixed in this PR * This PR then will focus on mis-codings on machinery process()s * FOR THE LOVE OF GIT STOP CALLING ..() EVERYWHERE AHHHHH * 2020-11-25 Commit 1 Case 41: /obj/machinery/bluespace_beacon/process() - Responsible for… beacon? - Plz keep icon updates sane - The only thing that changes invisibility is hide() * and hide() obviously is sane * yet it always calls it * faded version isn't vital in the first place so I am doing this * See /obj/machinery/navbeacon code for more doc Case 42: /obj/machinery/cell_charger/process() - Responsible for actual charging - Problems in power usage and BROKEN check are present but, * as I said, I am not touching that now - LGTM otherwise; update_icon here is necessary Case 43: /obj/machinery/clonepod/process() - Responsible for cloning; oh god oh fuck - Surprisingly LGTM Case 44: /obj/machinery/cryopod/process() - Responsible for freezing people away (read: qdel) - Proper Destroy() - Otherwise LGTM Case 45: /obj/machinery/jukebox/process() - Responsible playing and stopping music as needed - Order of addition and removal can be reversed * so I am going to reduce the amount of list iteration - return PROCESS_KILL - SSobj bad, use SSmachines Case 46: /obj/machinery/jukebox/disco/process() - Does the same as above except it also bops a lot - LGTM Case 47: /obj/machinery/defibrillator_mount/process() - Cell charger is 100% * I am not touching the balance here - update_icon is sane and needed Case 48: /obj/machinery/dish_drive/process() - Responsible for constant suction and occasional beaming - Suction code doesn’t check for other nearby drives but I don’t care - LGTM Case 49: /obj/machinery/droneDispenser/process() - power_change code is duplicated - Responsible for drone production; cycles through three modes - Except the useless parent call, the code LGTM Case 50: /obj/machinery/fat_sucker/process() - Responsible fat sucking and random trivia - Oh boy powered() situation is a bit deranged * /obj/machinery/proc/power_change() handles NOPOWER flag nicely * ... and powered() isn’t to be used everywhere * But so many machinery codes to * Arghhh I will deal with this in a different PR * For now it’ll only be dealt with on process() * So this powered() will be is_operational() * Simplifies meteor gamemode number * Replaced lagacy with a shiny new comment Co-authored-by: ike709 <ike709@users.noreply.github.com>
- Loading branch information
1 parent
ef40876
commit 057c9e2