Merge pull request #550 from Sea-of-Lost-Souls/upstreammaint

Screams from Rebase [MDB IGNORE] [IDB IGNORE]

.github/CODEOWNERS

@@ -1,8 +1,8 @@
 # This list auto requests reviews from the specified org members
 # when a PR that modifies the file in question is opened
-# This list is alphabetized by User -> Filename KEEP IT THAT WAY
-# In the event that multiple org members are to be informed of changes
-# to the same file or dir, add them to the end under Multiple Owners
+# This list is alphabetized by User -> Filename and separated into sections for Maintainers/Contributors KEEP IT THAT WAY
+# In the event that people are to be informed of changes
+# to the same file or dir, add them to the end of under Multiple Owners
 
 
 #Shivurr

.github/CONTRIBUTING.md

@@ -98,13 +98,13 @@ Our team is entirely voluntary, as such we extend our thanks to maintainers, iss
 
 ## Development Guides
 
-#### Writing readable code 
+#### Writing readable code
 [Style guide](./guides/STYLE.md)
 
-#### Writing sane code 
+#### Writing sane code
 [Code standards](./guides/STANDARDS.md)
 
-#### Writing understandable code 
+#### Writing understandable code
 [Autodocumenting code](./guides/AUTODOC.md)
 
 #### Misc
@@ -136,15 +136,29 @@ There is no strict process when it comes to merging pull requests. Pull requests
 
 * If your pull request is accepted, the code you add no longer belongs exclusively to you but to everyone; everyone is free to work on it, but you are also free to support or object to any changes being made, which will likely hold more weight, as you're the one who added the feature. It is a shame this has to be explicitly said, but there have been cases where this would've saved some trouble.
 
-* Please explain why you are submitting the pull request, and how you think your change will be beneficial to the game. Failure to do so will be grounds for rejecting the PR.
-
 * If your pull request is not finished, you may open it as a draft for potential review. If you open it as a full-fledged PR make sure it is at least testable in a live environment. Pull requests that do not at least meet this requirement will be closed. You may request a maintainer reopen the pull request when you're ready, or make a new one.
 
 * While we have no issue helping contributors (and especially new contributors) bring reasonably sized contributions up to standards via the pull request review process, larger contributions are expected to pass a higher bar of completeness and code quality *before* you open a pull request. Maintainers may close such pull requests that are deemed to be substantially flawed. You should take some time to discuss with maintainers or other contributors on how to improve the changes.
 
 * After leaving reviews on an open pull request, maintainers may convert it to a draft. Once you have addressed all their comments to the best of your ability, feel free to mark the pull as `Ready for Review` again.
 
 * **Pull requests that include sprites must have in-game screenshots that were taken on your own test-server in the PR body.** For instance, if you're adding new clothes, a screenshot of those clothes being worn is expected in the PR body. Not every single direction needs to be displayed, but each icon that's actually being used in-game should be showcased.
+## Justifying Your Changes
+
+You must explain why you are submitting the pull request in the "Why It's Good For The Game" section of your pull request, and how you think your change will be beneficial to the game. Failure to do so will be grounds for rejecting your pull request wholesale, or requiring that you fix it before your pull request is merged. A reasonable justification for your changes is a requirement.
+
+Your "How This Contributes To The Skyrat Roleplay Experience" section must make a good faith and reasonable attempt to:
+* Assert and argue that the current state of affairs in the game is not good, and needs changing.
+* Assert and argue that your pull request will either fix or help fix the problems you described.
+* Assert and argue that any downsides introduced by your solution as a matter of design, if any, are worth it, and why they are worth it.
+
+More controversial changes have higher standards for justification to be considered reasonable. A bugfix for example does not typically require any effort at all in justification as its value to the game is usually self evident, however a major feature overhaul or balance change may require significant explanation to adequately justify its supposed benefit to the game.
+
+This is still a requirement if your pull request is supported and/or requested by maintainers before it is opened. This is still a requirement if your pull request is supported and/or requested by head coders before it is opened. The purpose of arguing for your changes is not to convince just the maintainer team of its merits, it is to document the "why" behind your changes to the game to a necessary level of detail. The reason behind a change must exist as it is the purpose of this codebase to improve the game, thus said reasoning must be adequately stated and explained.
+
+This is also still a requirement if your pull request has a corresponding design document that justifies your changes inside it. You must always properly justify changes (those that actually need justification) within the pull request, even if you also do it elsewhere. This is to ensure that:
+1. All reviewers can easily see the reasoning behind your changes on the pull request itself, no reliance on other sites required.
+2. The actual, manifested implementation of the idea behind the design document is being justified after said implementation is actually realized. This is in contrast to any reasoning put on the design document itself, which very well may have been made before any work was done on it, possibly even by an author different from the author of the pull request. Any idea in the design document may have had compromises put into it due to complications not seen in the original vision, thus the current state of the implementation (the pull request as it stands) must be defended, explained, and ultimately justified in and of itself. Of course, you should still list the design document the pull request is implementing, and may even use arguments from the design document if said arguments are applicable to the current reality of your proposed changes.
 
 ## Good Boy Points
 
@@ -158,7 +172,7 @@ Negative GBP increases the likelihood of a maintainer closing your PR. With that
 
 There is no benefit to having a higher positive GBP score, since GBP only comes into consideration when it is negative.
 
-You can see each tag and their GBP values [Here](https://github.com/tgstation/tgstation/blob/master/.github/gbp.toml). 
+You can see each tag and their GBP values [Here](https://github.com/tgstation/tgstation/blob/master/.github/gbp.toml).
 
 ## Porting features/sprites/sounds/tools from other codebases
 

.github/PULL_REQUEST_TEMPLATE.md

@@ -16,7 +16,7 @@
 
 <details>
 <summary>Screenshots/Videos</summary>
-  
+
 </details>
 
 ## Changelog

.github/gbp.toml

@@ -23,3 +23,4 @@ reset_label = "GBP: Reset"
 "Sound" = 3
 "Sprites" = 3
 "Unit Tests" = 6
+"Wallening Revert Recovery" = 10

.github/guides/RUNNING_A_SERVER.md

@@ -4,7 +4,7 @@ BYOND installed. You can get it from https://www.byond.com/download. Once you've
 that, extract the game files to wherever you want to keep them. This is a
 sourcecode-only release, so the next step is to compile the server files.
 
-Double-click `BUILD.bat` in the root directory of the source code. This'll take
+Double-click `BUILD.cmd` in the root directory of the source code. This'll take
 a little while, and if everything's done right you'll get a message like this:
 
 ```

.github/guides/STYLE.md

@@ -27,23 +27,6 @@ Do not use tabs/spaces for indentation in the middle of a code line. Not only is
 #define SPECIES_FELINID "felinid"
 ```
 
-### Comments
-
-You mustn't use box comments as headers for code, nor for anything else.
-While it may be slightly more aesthetically pleasing, it can cause problems with auto-documentation and makes it far harder to add anything after the fact.
-It's much easier to stick down a new line than have to balance a bunch of whitespaces and forward slashes. 
-
-```dm
-//////////////////////////////////
-// BAD. DON'T DO THIS. STOP IT. //
-//////////////////////////////////
-
-/*
-* GOOD!
-* CONTINUE TO DO THIS
-*/
-```
-
 ### Control statements
 (if, while, for, etc)
 
@@ -457,7 +440,7 @@ Pop-quiz, what does this do?
 give_pizza(TRUE, 2)
 ```
 
-Well, obviously the `TRUE` makes the pizza hot, and `2` is the number of toppings. 
+Well, obviously the `TRUE` makes the pizza hot, and `2` is the number of toppings.
 
 Code like this can be very difficult to read, especially since our LSP does not show argument names at this time. Because of this, you should prefer to use named arguments where the meaning is not otherwise obvious.
 
@@ -607,7 +590,7 @@ This is [a real bug that tends to come up](https://github.com/tgstation/tgstatio
 The same goes for arguments passed to a macro...
 
 ```
-// Guarantee 
+// Guarantee
 #define CALCULATE_TEMPERATURE(base) (T20C + (##base))
 ```
 

.github/guides/VISUALS.md

@@ -367,6 +367,8 @@ You can actually force it to use a particular mob's sight to avoid aspects of th
 - [Table of Contents](#table-of-contents)
 - [Reference Entry](https://www.byond.com/docs/ref/#/mob/var/see_in_dark)
 
+`/mob/var/see_in_dark` sets the radius of a square around the mob that cuts out BYOND darkness.
+
 This is why when you stand in darkness you can see yourself, and why you can see a line of objects appear when you use mesons (horrible effect btw).
 It's quite simple, but worth describing.
 

.github/workflows/autowiki.yml

@@ -26,7 +26,7 @@ jobs:
       uses: actions/cache@v4
       with:
         path: ~/BYOND
-        key: ${{ runner.os }}-byond-${{ secrets.CACHE_PURGE_KEY }}
+        key: ${{ runner.os }}-byond-${{ hashFiles('dependencies.sh') }}
     - name: Install rust-g
       if: steps.secrets_set.outputs.SECRETS_ENABLED
       run: |

.github/workflows/ci_suite.yml

@@ -13,21 +13,35 @@ on:
   merge_group:
     branches:
     - master
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
 jobs:
-  run_linters:
+  start_gate:
     if: ( !contains(github.event.head_commit.message, '[ci skip]') )
+    name: Start Gate
+    runs-on: ubuntu-latest
+    steps:
+      - name: Mandatory Empty Step
+        run: exit 0
+
+  run_linters:
     name: Run Linters
+    needs: start_gate
     runs-on: ubuntu-22.04
-    concurrency:
-      group: run_linters-${{ github.head_ref || github.run_id }}
-      cancel-in-progress: true
+    timeout-minutes: 5
+
     steps:
       - uses: actions/checkout@v4
       - name: Restore SpacemanDMM cache
         uses: actions/cache@v4
         with:
           path: ~/SpacemanDMM
-          key: ${{ runner.os }}-spacemandmm
+          key: ${{ runner.os }}-spacemandmm-${{ hashFiles('dependencies.sh') }}
+          restore-keys: |
+            ${{ runner.os }}-spacemandmm-
       - name: Restore Yarn cache
         uses: actions/cache@v4
         with:
@@ -53,33 +67,39 @@ jobs:
         uses: actions/cache@v4
         with:
           path: ~/.cargo
-          key: ${{ runner.os }}-rust
+          key: ${{ runner.os }}-rust-${{ hashFiles('tools/ci/ci_dependencies.sh')}}
+          restore-keys: |
+            ${{ runner.os }}-rust-
       - name: Restore Cutter cache
         uses: actions/cache@v4
         with:
           path: tools/icon_cutter/cache
           key: ${{ runner.os }}-cutter-${{ hashFiles('dependencies.sh') }}
+      - name: Install OpenDream
+        uses: robinraju/[email protected]
+        with:
+          repository: "OpenDreamProject/OpenDream"
+          tag: "latest"
+          fileName: "DMCompiler_linux-x64.tar.gz"
+          extract: true
       - name: Install Tools
         run: |
           pip3 install setuptools
           bash tools/ci/install_node.sh
           bash tools/ci/install_spaceman_dmm.sh dreamchecker
-          cargo install ripgrep --features pcre2
+          bash tools/ci/install_ripgrep.sh
           tools/bootstrap/python -c ''
       - name: Give Linters A Go
         id: linter-setup
         run: ':'
       - name: Run Grep Checks
         if: steps.linter-setup.conclusion == 'success' && !cancelled()
-        run: |
-          bash tools/ci/check_grep.sh
-          bash skyrat/tools/skyrat_check_grep.sh # SKYRAT EDIT ADDITION - checking modular_skyrat code
+        run: bash tools/ci/check_grep.sh
       - name: Ticked File Enforcement
         if: steps.linter-setup.conclusion == 'success' && !cancelled()
         run: |
           tools/bootstrap/python tools/ticked_file_enforcement/ticked_file_enforcement.py < tools/ticked_file_enforcement/schemas/tgstation_dme.json
           tools/bootstrap/python tools/ticked_file_enforcement/ticked_file_enforcement.py < tools/ticked_file_enforcement/schemas/unit_tests.json
-          tools/bootstrap/python tools/ticked_file_enforcement/ticked_file_enforcement.py < tools/ticked_file_enforcement/schemas/modular_skyrat.json # SKYRAT EDIT ADDITION - modular tick enforcement
       - name: Check Define Sanity
         if: steps.linter-setup.conclusion == 'success' && !cancelled()
         run: tools/bootstrap/python -m define_sanity.check
@@ -90,6 +110,9 @@ jobs:
         if: steps.linter-setup.conclusion == 'success' && !cancelled()
         shell: bash
         run: ~/dreamchecker 2>&1 | bash tools/ci/annotate_dm.sh
+      #- name: Run OpenDream - SKYRAT REMOVAL - DreamChecker is fine for us so far - SKYRAT TODO
+      #  if: steps.linter-setup.conclusion == 'success' && !cancelled()
+      #  run: ./DMCompiler_linux-x64/DMCompiler tgstation.dme --suppress-unimplemented --define=CIBUILDING | bash tools/ci/annotate_od.sh
       - name: Run Map Checks
         if: steps.linter-setup.conclusion == 'success' && !cancelled()
         run: |
@@ -115,20 +138,18 @@ jobs:
         run: tools/build/build --ci lint tgui-test
 
   compile_all_maps:
-    if: ( !contains(github.event.head_commit.message, '[ci skip]') )
     name: Compile Maps
-    needs: [collect_data]
+    needs: collect_data
     runs-on: ubuntu-22.04
-    concurrency:
-      group: compile_all_maps-${{ github.head_ref || github.run_id }}
-      cancel-in-progress: true
+    timeout-minutes: 5
+
     steps:
       - uses: actions/checkout@v4
       - name: Restore BYOND cache
         uses: actions/cache@v4
         with:
           path: ~/BYOND
-          key: ${{ runner.os }}-byond
+          key: ${{ runner.os }}-byond-${{ hashFiles('dependencies.sh') }}
       - name: Compile All Maps
         run: |
           bash tools/ci/install_byond.sh
@@ -141,16 +162,15 @@ jobs:
           max-required-client-version: ${{needs.collect_data.outputs.max_required_byond_client}}
 
   collect_data:
-    if: ( !contains(github.event.head_commit.message, '[ci skip]') )
     name: Collect data for other tasks
+    needs: start_gate
     runs-on: ubuntu-22.04
+    timeout-minutes: 5
     outputs:
       maps: ${{ steps.map_finder.outputs.maps }}
       alternate_tests: ${{ steps.alternate_test_finder.outputs.alternate_tests }}
       max_required_byond_client: ${{ steps.max_required_byond_client.outputs.max_required_byond_client }}
-    concurrency:
-      group: find_all_maps-${{ github.head_ref || github.run_id }}
-      cancel-in-progress: true
+
     steps:
       - uses: actions/checkout@v4
       - name: Find Maps
@@ -172,51 +192,40 @@ jobs:
           echo "max_required_byond_client=$(grep -Ev '^[[:blank:]]{0,}#{1,}|^[[:blank:]]{0,}$' .github/max_required_byond_client.txt | tail -n1)" >> $GITHUB_OUTPUT
 
   run_all_tests:
-    if: ( !contains(github.event.head_commit.message, '[ci skip]') )
     name: Integration Tests
-    needs: [collect_data]
+    needs: collect_data
+
     strategy:
       fail-fast: false
       matrix:
         map: ${{ fromJSON(needs.collect_data.outputs.maps).paths }}
-    concurrency:
-      group: run_all_tests-${{ github.head_ref || github.run_id }}-${{ matrix.map }}
-      cancel-in-progress: true
+
     uses: ./.github/workflows/run_integration_tests.yml
     with:
       map: ${{ matrix.map }}
       max_required_byond_client: ${{needs.collect_data.outputs.max_required_byond_client}}
 
   run_alternate_tests:
-    if: ( !contains(github.event.head_commit.message, '[ci skip]') && needs.find_all_maps.outputs.alternate_tests != '[]' )
+    if: needs.collect_data.outputs.alternate_tests != '[]'
     name: Alternate Tests
-    needs: [collect_data]
+    needs: collect_data
     strategy:
       fail-fast: false
       matrix:
         setup: ${{ fromJSON(needs.collect_data.outputs.alternate_tests) }}
-    concurrency:
-      group: run_all_tests-${{ github.head_ref || github.run_id }}-${{ matrix.setup.major }}.${{ matrix.setup.minor }}-${{ matrix.setup.map }}
-      cancel-in-progress: true
+
     uses: ./.github/workflows/run_integration_tests.yml
     with:
       map: ${{ matrix.setup.map }}
       major: ${{ matrix.setup.major }}
       minor: ${{ matrix.setup.minor }}
       max_required_byond_client: ${{needs.collect_data.outputs.max_required_byond_client}}
 
-  check_alternate_tests:
-    if: ( !contains(github.event.head_commit.message, '[ci skip]') && needs.find_all_maps.outputs.alternate_tests != '[]' )
-    name: Check Alternate Tests
-    needs: [run_alternate_tests]
-    runs-on: ubuntu-22.04
-    steps:
-      - run: echo Alternate tests passed.
-
   compare_screenshots:
-    if: ( !contains(github.event.head_commit.message, '[ci skip]') && (success() || failure()) )
-    needs: [run_all_tests, run_alternate_tests]
+    if: needs.collect_data.outputs.alternate_tests == '[]' || needs.run_alternate_tests.result == 'success'
+    needs: [ collect_data, run_all_tests, run_alternate_tests ]
     name: Compare Screenshot Tests
+    timeout-minutes: 15
     runs-on: ubuntu-22.04
     steps:
       - uses: actions/checkout@v4
@@ -251,13 +260,11 @@ jobs:
           path: artifacts/screenshot_comparisons
 
   test_windows:
-    if: ( !contains(github.event.head_commit.message, '[ci skip]') )
     name: Windows Build
     needs: [collect_data]
     runs-on: windows-latest
-    concurrency:
-      group: test_windows-${{ github.head_ref || github.run_id }}
-      cancel-in-progress: true
+    timeout-minutes: 5
+
     steps:
       - uses: actions/checkout@v4
       - name: Restore Yarn cache
@@ -276,3 +283,11 @@ jobs:
         with:
           dmb-location: tgstation.dmb
           max-required-client-version: ${{needs.collect_data.outputs.max_required_byond_client}}
+
+  completion_gate: # Serves as a non-moving target for branch rulesets
+    name: Completion Gate
+    needs: [ test_windows, compare_screenshots, compile_all_maps, run_linters ]
+    runs-on: ubuntu-latest
+    steps:
+      - name: Mandatory Empty Step
+        run: exit 0