2e5bfe5be6
## About The Pull Request ### How things work As things currently stand, when a mob breaths several things happen (simplified to focus on the stupid) We assert the existance of all possible breathable gases, and pull partial pressures for them Then we walk through all possible interactions lungs could have with these gases, one by one, and see if they're happening or not As we go we are forced to cleanup potential alerts caused by the previous breath, even if those effects never actually happen At the end we clear out all the unused gas ids, and handle the temperature of the breath. ### What sucks There's I'd say 3 different types of gas reactions. - You can "need" a gas to survive. o2, n2 and plasma all fall into this category - A gas can do something to you while it's in your system. This applies to most gas types - Variation on the previous, some gases do cleanup when they're not in your system, or when there isn't much of them in the first place The main headache here is that second one, constantly cleaning up potential side effects sucks, and fixing it would require a lot of dummy variables There's other suckage too. Needing to constantly check for a gas type even if it isn't there is stupid, and leads to wasted time It's also really annoying to do subtypes in this system. There is what amounts to a hook proc you can override, but you can't override the reaction to a gas type. It also just like, sucks to add new gases. one mega proc smells real stupid. ### Improvements In the interest of speed: - I'd like to build a system that doesn't require manually checking for gas - Reacting to gas "disappearing" should be promoted by the system, instead of being hacky. - I would like to avoid needing to assert the existence of all possible gases, as this is slow on both the assert and the garbage collect. In the interest of dev ergonomics: - It should be easy to define a new gas reaction - It should be easy for subtypes to implement their own gas reactions. The current method of vars on the lung is all tangled up and not really undoable as of now, but I'd like to not require it - It should be possible to fully override how a gas is handled ### What I've Done Lungs have 3 lists of proc paths stored on them Each list handles a different way the lung might want to interact with a gas. There's a list for always processing on a gas (we use this for stuff that's breathed), a list for handling a gas in our breath, and a list for reacting to a gas previously being in our breath, but not any more. Lungs fill out these lists using a helper proc during Initialize() Then, when it comes time to breath, we loop over the gas in the breath and react to it. We also keep track of the previous list of partial pressures, which we calculate for free here, and use that to figure out when to call the loss reactions. This proc pattern allows for overrides, easy reactions to removals, lower indentation code and early returns, and better organization of signal handlers It's also significantly faster. Ballpark 4x faster ### Misc Removes support for breathing co2, and dying from n2 poisoning. They were both unused, and I think it's cringe to clutter these procs even further Added "do we even have oxyloss" checks to most cases of passive breathing. This is a significant save, since redundant adjustoxy's are decently expensive at the volume of calls we have here. Fixes a bug with breathing out if no gas is passed in, assigning a var to another var doesn't perform a copy Rewrote breathe_gas_volume() slightly to insert gas into an immutable mix stored on the lung, rather then one passed in This avoids passing of a gas_mixture around just to fill a hole. I may change my mind on this, since it would be nice to have support for temperature changing from a hot/cold breath. Not gonna be done off bodytemp tho lord no. Uses merge() instead of a hard coded version to move the gas ids over. This is slightly slower with lower gas counts but supports more things in future and is also just easier to read. ## Why It's Good For The Game Faster, easier to work with and read (imo) Profiles: [breath_results_old.txt](https://github.com/tgstation/tgstation/files/11068247/breath_results_old.txt) [breath_results_pre_master.txt](https://github.com/tgstation/tgstation/files/11068248/breath_results_new.txt) [breath_results_new.txt](https://github.com/tgstation/tgstation/files/11068349/breath_results_new.txt) (These profiles were initially missing #73026. Merging this brings the savings from 16% to 12%. Life is pain) --------- Co-authored-by: san7890 <the@san7890.com>
Unit Tests
What is unit testing?
Unit tests are automated code to verify that parts of the game work exactly as they should. For example, a test to make sure that the amputation surgery actually amputates the limb. These are ran every time a PR is made, and thus are very helpful for preventing bugs from cropping up in your code that would've otherwise gone unnoticed. For example, would you have thought to check that beach boys would still work the same after editing pizza? If you value your time, probably not.
On their most basic level, when UNIT_TESTS is defined, all subtypes of /datum/unit_test will have their Run proc executed. From here, if Fail is called at any point, then the tests will report as failed.
How do I write one?
- Find a relevant file.
All unit test related code is in code/modules/unit_tests. If you are adding a new test for a surgery, for example, then you'd open surgeries.dm. If a relevant file does not exist, simply create one in this folder, then #include it in _unit_tests.dm.
- Create the unit test.
To make a new unit test, you simply need to define a /datum/unit_test.
For example, let's suppose that we are creating a test to make sure a proc square correctly raises inputs to the power of two. We'd start with first:
/datum/unit_test/square/Run()
This defines our new unit test, /datum/unit_test/square. Inside this function, we're then going to run through whatever we want to check. Tests provide a few assertion functions to make this easy. For now, we're going to use TEST_ASSERT_EQUAL.
/datum/unit_test/square/Run()
TEST_ASSERT_EQUAL(square(3), 9, "square(3) did not return 9")
TEST_ASSERT_EQUAL(square(4), 16, "square(4) did not return 16")
As you can hopefully tell, we're simply checking if the output of square matches the output we are expecting. If the test fails, it'll report the error message given as well as whatever the actual output was.
- Run the unit test
Open code/_compile_options.dm and uncomment the following line.
//#define UNIT_TESTS //If this is uncommented, we do a single run though of the game setup and tear down process with unit tests in between
Then, run tgstation.dmb in Dream Daemon. Don't bother trying to connect, you won't need to. You'll be able to see the outputs of all the tests. You'll get to see which tests failed and for what reason. If they all pass, you're set!
How to think about tests
Unit tests exist to prevent bugs that would happen in a real game. Thus, they should attempt to emulate the game world wherever possible. For example, the quick swap sanity test emulates a real scenario of the bug it fixed occurring by creating a character and giving it real items. The unrecommended alternative would be to create special test-only items. This isn't a hard rule, the reagent method exposure tests create a test-only reagent for example, but do keep it in mind.
Unit tests should also be just that--testing units of code. For example, instead of having one massive test for reagents, there are instead several smaller tests for testing exposure, metabolization, etc.
The unit testing API
You can find more information about all of these from their respective doc comments, but for a brief overview:
/datum/unit_test - The base for all tests to be ran. Subtypes must override Run(). New() and Destroy() can be used for setup and teardown. To fail, use TEST_FAIL(reason).
/datum/unit_test/proc/allocate(type, ...) - Allocates an instance of the provided type with the given arguments. Is automatically destroyed when the test is over. Commonly seen in the form of var/mob/living/carbon/human/human = allocate(/mob/living/carbon/human/consistent).
TEST_FAIL(reason) - Marks a failure at this location, but does not stop the test.
TEST_ASSERT(assertion, reason) - Stops the unit test and fails if the assertion is not met. For example: TEST_ASSERT(powered(), "Machine is not powered").
TEST_ASSERT_NOTNULL(a, message) - Same as TEST_ASSERT, but checks if !isnull(a). For example: TEST_ASSERT_NOTNULL(myatom, "My atom was never set!").
TEST_ASSERT_NULL(a, message) - Same as TEST_ASSERT, but checks if isnull(a). If not, gives a helpful message showing what a was. For example: TEST_ASSERT_NULL(delme, "Delme was never cleaned up!").
TEST_ASSERT_EQUAL(a, b, message) - Same as TEST_ASSERT, but checks if a == b. If not, gives a helpful message showing what both a and b were. For example: TEST_ASSERT_EQUAL(2 + 2, 4, "The universe is falling apart before our eyes!").
TEST_ASSERT_NOTEQUAL(a, b, message) - Same as TEST_ASSERT_EQUAL, but reversed.
TEST_FOCUS(test_path) - Only run the test provided within the parameters. Useful for reducing noise. For example, if we only want to run our example square test, we can add TEST_FOCUS(/datum/unit_test/square). Should never be pushed in a pull request--you will be laughed at.
Final Notes
- Writing tests before you attempt to fix the bug can actually speed up development a lot! It means you don't have to go in game and folllow the same exact steps manually every time. This process is known as "TDD" (test driven development). Write the test first, make sure it fails, then start work on the fix/feature, and you'll know you're done when your tests pass. If you do try this, do make sure to confirm in a non-testing environment just to double check.
- Make sure that your tests don't accidentally call RNG functions like
prob. Since RNG is seeded during tests, you may not realize you have until someone else makes a PR and the tests fail! - Do your best not to change the behavior of non-testing code during tests. While it may sometimes be necessary in the case of situations such as the above, it is still a slippery slope that can lead to the code you're testing being too different from the production environment to be useful.