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

[TG Mirror] General code maintenance for rcd devices and their DEFINE file [MDB IGNORE] #19

Closed
wants to merge 1 commit into from

Conversation

ffmirrorbot[bot]
Copy link

@ffmirrorbot ffmirrorbot bot commented Oct 12, 2023

Mirrored on Skyrat: ${this.url}
Original PR: (original url)

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 file
https://github.com/tgstation/tgstation/blob/0fb8b8b218400b3f1805ae81e9bb0364d7a4e9c6/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?
https://github.com/tgstation/tgstation/blob/0fb8b8b218400b3f1805ae81e9bb0364d7a4e9c6/code/__DEFINES/construction.dm#L91-L94

It gets worse why are global lists declared here?
https://github.com/tgstation/tgstation/blob/0fb8b8b218400b3f1805ae81e9bb0364d7a4e9c6/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.
https://github.com/tgstation/tgstation/blob/0fb8b8b218400b3f1805ae81e9bb0364d7a4e9c6/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 proc
    https://github.com/tgstation/tgstation/blob/0fb8b8b218400b3f1805ae81e9bb0364d7a4e9c6/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 RCD
https://github.com/tgstation/tgstation/blob/0fb8b8b218400b3f1805ae81e9bb0364d7a4e9c6/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 defines
https://github.com/tgstation/tgstation/blob/0fb8b8b218400b3f1805ae81e9bb0364d7a4e9c6/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 defines
https://github.com/tgstation/tgstation/blob/0fb8b8b218400b3f1805ae81e9bb0364d7a4e9c6/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.
https://github.com/tgstation/tgstation/blob/0fb8b8b218400b3f1805ae81e9bb0364d7a4e9c6/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

🆑 SyncIt21
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
/:cl:

…le [MDB IGNORE] (#24300)

* General code maintenance for rcd devices and their DEFINE file

* Update window.dm

* Update window.dm

* Update window.dm

---------

Co-authored-by: SyncIt21 <[email protected]>
Co-authored-by: Bloop <[email protected]>
@Iajret Iajret closed this Oct 17, 2023
@Iajret Iajret deleted the upstream-mirror-24300 branch October 17, 2023 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants