Salvage: Employees of the ITG - #5619
Open
Swonkus wants to merge 39 commits into
Open
Conversation
github-actions
Bot
added
S: Needs Review
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (23)
📒 Files selected for processing (99)
💤 Files with no reviewable changes (20)
🚧 Files skipped from review as they are similar to previous changes (76)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughSummaryMoves salvage operations from station Cargo to an Interstellar Trade Guild (ITG) base aboard ATS vessels. User and gameplay impact
Notable file changes
Risk areas
Guideline gaps
WalkthroughAdds the Interstellar Trade Guild salvage department and Salvage Medic role. Updates salvage access, equipment, computers, lockers, SOP documentation, migration rules, theft objectives, and station job rosters. ChangesITG Salvage Department
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟡 Moderate · up to This PR relocates salvage operations and changes their roles, equipment, access, objectives, and map behavior, but several current issues can leave salvage unable to operate correctly, preserve an objective that was intended to be removed, or alter existing airlock behavior unexpectedly. It is not merge-ready until these concrete configuration and behavior issues are fixed or explicitly accepted. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (78 skipped: 78 unsupported.) ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
Reopened as a new draft to keep it clean after a merge went bad |
mikeysaurus
reviewed
Aug 11, 2026
mikeysaurus
reviewed
Aug 11, 2026
mikeysaurus
reviewed
Aug 11, 2026
mikeysaurus
reviewed
Aug 11, 2026
…ways, might as well make it easier to bring them back when ITG variant no longer needed
4 tasks
CawsForConcern
added
T: Rework
and removed
S: Untriaged
Contributor
Author
|
@coderabbitai resume reviews |
Contributor
|
✅ Action performedReviews resumed. |
github-actions
Bot
added
the
S: Merge Conflict
Contributor
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Contributor
Author
|
Shard 1 is triggering as a result of removing the spawnpoints from the map, this failure is just part of how the update is |
github-actions
Bot
removed
the
S: Merge Conflict
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
github-actions
Bot
added
the
S: Merge Conflict
Contributor
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
github-actions
Bot
removed
the
S: Merge Conflict
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
Author
|
Changes are all still functional after final testing |
github-actions
Bot
added
the
S: Merge Conflict
Contributor
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
github-actions
Bot
removed
the
S: Merge Conflict
Contributor
Author
|
github-actions
Bot
added
the
S: Merge Conflict
Contributor
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Short description
This PR moves salvage from on station to fully off station, and into being employees of the Interstellar Trade Guild aboard the ATs. This PR specifically
Duplicate versions of salvage assets have been made to mass remove currently mapped salvage bays. ITG marked version appear on the ATS
Why we need to add this
Salvage currently struggle to fit into the round flow naturally as members of cargo. Current gameplay results in a rule/sop evasion, excessive intervention of salvagers in security affairs, and a limitation of salvage content due to their capabilities on station
Moving them to the ATS and making them a non-crew element seeks to de-restrict salvager gameplay and make them a more interesting asset for the crew to interact with.
This PR is in collaboration with #5522
Relevent PR Workshop
Design Document
Media (Video/Screenshots)
Checks
Changelog
🆑 Swonki, Born Stellar, Mikeysaurus, itsmaitake