[MDB IGNORE] Reagent container code cutdown

[SECBATON-GRIFFON]

What this does

takes ALL of the add_reagent stuff in new() from reagent container objects and makes it automatic with the reagents_to_add variable, as seen in lollipops gives that var a proper unit test compresses some food into subtypes splits some food and drink into separate files like chemicals were# moves reagent_refill() down from hyposprays to the general container level, making them work with them all, and renames it refill() (currently not used by anything but admin hyposprays still) Closes #35989. Closes #16054.

Why it's good

much less code to write, easier to sift through files

Changelog

:cl:

• tweak: Poutine and carrot fries now have a proper colour when extracted with a fork.
• tweak: Salads now leave snack bowls behind more consistently between types.
• bugfix: Hot drinks machine chifir now has its icon back.
• bugfix: Individual slices of slicable foods found by themselves from no parent item now have nutrients inside.

[jwhitak]

Wew lad. It's a big refactor, which is something that I wouldn't try to push if I were in the bugfix mines. That said, nice stuff. I like the reagent rework especially. I'll review fully when it's ready, unless chicken says no.

[SECBATON-GRIFFON]

Wew lad. It's a big refactor, which is something that I wouldn't try to push if I were in the bugfix mines. That said, nice stuff. I like the reagent rework especially. I'll review fully when it's ready, unless chicken says no.

dunno where refactors land

[jwhitak]

Lands in "damian has comments" category for sure.

https://www.byond.com/forum/post/2086980?page=2#comment19776775

Apparently this refactor will slow down server startup times/item spawning times, since these lists are all being manually added with a hidden /Init() proc when the object is spawned, before New() is called.

Edit: considering it's just food and not all food, this is as efficient as it can be with the system you've written. I'm not sure if there's signfiicant slowdown to be had.

[hacker-on-steroids]

personally i hate split files because it makes you trawl through more files instead of being able to ctrlF everything at one place

[Eneocho]

personally i hate split files because it makes you trawl through more files instead of being able to ctrlF everything at one place

Same, but I see the point in having it separated. At least you can open all the relevant files and ctrl+F bounded to all the open files, which works more or less the same.

[Eneocho]

Mandatory Prepared for Unforseen Consequences post.

[SECBATON-GRIFFON]

Mandatory Prepared for Unforseen Consequences post.

that's what unit tests are for

[hacker-on-steroids]

we have unit tests? i mean i know we have them but they're not enabled are they?

[SECBATON-GRIFFON]

we have unit tests? i mean i know we have them but they're not enabled are they?

i wonder what that compile and run tests thing in the checks is for

[hacker-on-steroids]

yeah, those i remember seeing that the thing did a "does walking on a banana peel slip you?" test or something along those lines, going "wow, we have some pretty thorough test coverage" and then quite a few PRs released nervegas without failing any tests so i assume they're either broken or disabled (or maybe the bananapeel is the only one)

[SECBATON-GRIFFON]

and then quite a few PRs released nervegas without failing any tests so i assume they're either broken or disabled (or maybe the bananapeel is the only one)

the tests don't really cover everything

[jwhitak]

This PR actually costs additional memory and startup time. Any lists defined in an object's defintion cause an invisible /New() to be made which initalizes the list and stores it to memory, vs normal static variables. These lists as implemented are also permanently inside of the object, where they can be VV'd even after creation. The file sizes are slightly smaller and more compact, but it's harder on the server.

The file cleanup is nice at least.

[hacker-on-steroids]

Lands in "damian has comments" category for sure. https://www.byond.com/forum/post/2086980?page=2#comment19776775

reading that thread, lummox says the armor() list on /obj/item is bad? making it null and letting only /obj/item/clothing make their armor lists made server startup time go from 21.5 to 21.35 seconds(n=2) is there a reason we aren't doing this?

[jwhitak]

This is a major revision beyond simple bug fixes and is technically a parole violation. There was weeks of discussion on if this is to be allowed and there was no consensus so I will err to the side of not merging this. I'm not a fan of keeping a list of the initial reagents permanently in each object.