Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MDB IGNORE] Reagent container code cutdown #36398

Open
wants to merge 144 commits into
base: Bleeding-Edge
Choose a base branch
from

Conversation

SECBATON-GRIFFON
Copy link
Contributor

@SECBATON-GRIFFON SECBATON-GRIFFON commented Apr 24, 2024

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

🆑

  • 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.

@hacker-on-steroids
Copy link
Contributor

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

@SECBATON-GRIFFON
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Collaborator

jwhitak commented May 5, 2024

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.

@SECBATON-GRIFFON SECBATON-GRIFFON marked this pull request as draft May 20, 2024 16:42
@SECBATON-GRIFFON SECBATON-GRIFFON marked this pull request as ready for review May 20, 2024 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
File Cleanup Cleans up oldcoder messes. System Modifies an underlying system within the game, may not affect players in any way.
Projects
None yet
4 participants