66f726dfe3
## About The Pull Request The changes made can be best summarized into points **1. Cleans up `code/_DEFINES/construction.dm`** Looking at the top comment of this file0fb8b8b218/code/__DEFINES/construction.dm (L1)One would expect stuff related to materials, rcd, and other construction related stuff. Well this file is anything but Why is there stuff related to food & crafting over here then?0fb8b8b218/code/__DEFINES/construction.dm (L91-L94)It gets worse why are global lists declared here?0fb8b8b218/code/__DEFINES/construction.dm (L115)There is a dedicated folder to store global lists i.e. `code/_globalvars/lists` for lists like these. These clearly don't belong here On top of that a lot of construction related defines has been just dumped here making it too large for it's purposes. which is why this file has been scraped and it's 1. crafting related stuff have been moved to its `code/_DEFINES/crafting.dm` 2. global lists for crafting moved to `code/_globalvars/lists/crafting.dm` 3. Finally remaining construction related defines split apart into 4 file types under the new `code/_DEFINES/construction` folder - `code/_DEFINES/construction/actions.dm` -> for wrench act or other construction related actions - `code/_DEFINES/construction/material.dm` -> contains your sheet defines and cable & stack related values. Also merged `code/_DEFINES/material.dm` with this file so it belongs in one place - `code/_DEFINES/construction/rcd.dm` -> dedicated file for everything rcd related - `code/_DEFINES/construction/structures.dm` -> contains the construction states for various stuff like walls, girders, floodlight etc By splitting this file into multiple meaningful define file names will help in reducing merge conflicts and will aid in faster navigation so this is the 1st part of this PR **2. Debloats the `RCD.dm` file(Part 1)** This uses the same concepts as above. i.e. moving defines into their appropriate files for e.g.0fb8b8b218/code/game/objects/items/rcd/RCD.dm (L170)1. Global vars belong in the `code/_globalvars` folder so these vars and their related functions to initialize them are moved into the `code/_globalvars/rcd.dm` file 2. See this proc0fb8b8b218/code/game/objects/items/rcd/RCD.dm (L200)This proc does not belong to the `obj/item/construction/rcd` type it's a global "helper function" this is an effect proc as it creates rcd holograms so it has been moved to the `code/game/objects/effects/rcd.dm` file which is a global effect that can be used by anyone And with that we have moved these vars & procs into their correct places & reduced the size of this file . We can go even further **3. Debloats the `RCD.dm` file(Part 2)** This deals with the large list which contains all the designs supported by the RCD0fb8b8b218/code/game/objects/items/rcd/RCD.dm (L42)This list contains a lot of local defines. We can scrape some of them and reduce the overall bulkiness & memory requirements of this list and so the following defines ``` #define WINDOW_TYPE "window_type" #define COMPUTER_DIR "computer_dir" #define WALLFRAME_TYPE "wallframe_type" #define FURNISH_TYPE "furnish_type" #define AIRLOCK_TYPE "airlock_type" #define TITLE "title" #define ICON "icon" #define CATEGORY_ICON_STATE "category_icon_state" #define CATEGORY_ICON_SUFFIX "category_icon_suffix" #define TITLE_ICON "ICON=TITLE" ``` Have all been removed making this list a lot more cleaner. Why? because a lot of these are just semantic sugar, we can infer the value of a lot of these defines if we just know the type path of the structure the rcd is trying to build for e.g. take these 2 defines0fb8b8b218/code/game/objects/items/rcd/RCD.dm (L13-L15)These defines tell the rcd UI the name and the icon it should display. Rather than specifying these manually in the design we can infer them like this ``` var/obj/design = /obj/structure/window //let's say the rcd is trying to build an window var/name = initial(design.name) //we have inferred the name of the design without requiring TITLE define var/icon = initial(design.icon_state) //we have inferred the icon of the design without requiring ICON define ``` And so by using similar logic to the remaining defines we can eliminate a lot of these local defines and reduce the overall size of this list. The same logic applies to the different modes of construction, the following defines0fb8b8b218/code/__DEFINES/construction.dm (L186-L192)Have all been removed and replaced with a single value `RCD_STRUCTURE` All these modes follow the same principle when building them 1. First check the turf if the structure exists. If it does early return 2. If not create a new structure there and that's it So rather than creating a new construction mode every time you want to add a new design we can use this mode to apply this general approach every time The design list has also now been made into a global list rather than a private static list. The big advantage to this is that the rcd asset cache can now access this list and load the correct icons from the list directly. This means you no longer have to manually specify what icons you want to load which is the case currently.0fb8b8b218/code/modules/asset_cache/assets/rcd.dm (L8-L9)This has lead to the UI icons breaking twice now in the past - #74194 - #77217 Hopefully this should never repeat itself again **4. Other RCD like device changes** - Fixed the broken silo link icon when the radial menu of the RLD was opened - replaced a lot of vars inside RLD with defines to save memory - Small changes to `ui_act` across RCD, Plumbing RCD and RTD - Removed unused vars in RCD and snowflaked code - Moved a large majority of `ui_data()` to `ui_static_data()` making the experience much faster Just some general clean up going on here **5. The Large majority of other code changes** These are actually small code changes spread across multiple files. These effect the `rcd_act()` & the `rcd_vals()` procs across all items. Basically it - Removes a large majority of `to_chat()` & `visible_message()` calls. This was done because we already have enough visual feedback of what's going on. When we construct a wall we don't need a `to_chat()` to tell us you have a built a wall, we can clearly see that - replaces the static string `"mode"` with a predefined constant `RCD_DESIGN_MODE` to bring some standard to use across all cases Should reduce chat spam and improve readability of code. **6. Airlock & Window names** The rcd asset cache relies on the design name to be unique. So i filled in the missing names for some airlocks & windows which are subjective and open to change but must have some value **7 Removes Microwave PDA upgrade** The RCD should not be allowed to build this microwave considering how quickly it can spawn multiple structures and more importantly as it's a special multipurpose machine so you should spend some effort in printing it's parts and acquiring tools to complete it. This upgrade makes obsolete the need to carry an - A RPED to install the parts - A screwdriver to complete the frame - The circuit board for the microwave The most important point to note here is that whenever an RPED/circuit board is printed at an lathe it charges you "Lathe Tax". The RCD with this upgrade would essentially bypass the need to "Pay Taxes" at these lathes as you are just creating a circuit board from thin air. This causes economy imbalance(10 cr per print) which scales up the more of these machines you make so to avoid this let's end the problem here Not to mention people would not find the need to print the circuit board for a regular microwave now if they have an RCD because they can just make this microwave type making the need for a regular microwave completely pointless. Just build a machine frame with the RCD and complete the microwave from there ## Changelog 🆑 code: moved global vars, lists and helper procs for construction related stuff to their appropriate files code: reduced overall code size & memory of rcd design list and removed unused defines refactor: removed a ton of chat alerts for rcd related actions to help reduce chat spam refactor: some airlock & window default names have changed fix: broken icon in radial menu of rld silo link remove: removes microwave pda upgrade from RCD. It's a special machine so spend some time in building it rather than shitting them out for free with the RCD. Use the RCD upgrade to spawn a machine frame instead & go from there /🆑 --------- Co-authored-by: Ghom <42542238+Ghommie@users.noreply.github.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.