Goonstation Forums
[MERGED PR] Manufacturer grammar and code touchup - Printable Version

+- Goonstation Forums (https://forum.ss13.co)
+-- Forum: Discussion (https://forum.ss13.co/forumdisplay.php?fid=6)
+--- Forum: Ideas & Suggestions (https://forum.ss13.co/forumdisplay.php?fid=8)
+---- Forum: Good ideas! (https://forum.ss13.co/forumdisplay.php?fid=19)
+---- Thread: [MERGED PR] Manufacturer grammar and code touchup (/showthread.php?tid=18976)



[MERGED PR] Manufacturer grammar and code touchup - github_bot - 06-01-2022

PULL REQUEST DETAILS



[CLEANLINESS]

About the PR
hey! first time working with this codebase, so please holler at me if I've done something wrong.

this PR touches up `manufacturer.dm`, which seems to be very old, by doing a few things:
* updated the names and descriptions to use unified standards - lowercased names, added some variety to descriptions, and so on;
* moved the defs for manufacturer blueprints to the bottom of the file, instead of having them between the base manufacturer def and the defs of the subtypes;
* audited a lot of the syntax - replaced some magic numbers with `TRUE`/`FALSE` defines, removed some extraneous `var/` calls in proc arguments, sorted and documented the variables (renaming them where beneficial), and so on;
* removed several variables and `Topic()` checks that appear to be unused.

I'd like to do more work on this, but this PR is getting big enough as it is and I wanted to get the PR up before I dived into semantics so that I could get feedback on if this is the right stuff to do, if it should be atomized and to what degree, etc.

I've gone through the map defs in VSC and made sure that manufacturers with unique descriptions have maintained them, as well as resaving the files in strongDMM. if there are any manufacturer instances in the secret submodule that have overridden descriptions, though, those might need to be tweaked and the file resaved; this is due to the new `supplemental_desc` variable that appends to the existing desc in subtypes to avoid copy-pasting of the regular `desc` variable, but I'm happy to axe this and do things the old-fashioned way if it's cleaner.

Why's this needed?
originally this was just for the grammar changes, which I think are beneficial for adding consistency to player-facing stuff like names and descriptions. when I saw the rest of the file, I decided to try and fix it up while I was in the neighborhood; the code is very old, and I think giving it a modernization pass makes it more readable as well as probably more stable.

Testing
I've done some preliminary tests on my local copy to make sure that manufacturers are still able to accept and print things like they should and thus far I haven't had any significant problems, ~~but I've run into some issues with electrocutions not occurring that I'm still hashing out.~~ (fixed!) ~~I'd like to play a miner round on my local branch before I sign off on it as being fully stable, though!~~ - done, mined 'til I got the mechanized boots + industrial armor and then a little while longer and I didn't run into any issues, but I also didn't try any hacking or anything...

PULL REQUEST DETAILS