PR #154
PR #154
Conversation
WalkthroughThe pull request encompasses a variety of changes across multiple files, primarily focusing on UI modifications in Changes
Possibly related PRs
Suggested labels
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (10)
📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
size/L
Changes: Map
Changes: Sprite
Changes: YML
Changes: UI
labels
|
RSI Diff Bot; head commit 03cd8b2 merging into c635b1a Resources/Textures/NES/Clothing/OuterClothing/Coats/labcoat.rsi
Resources/Textures/Stray/Clothing/Head/Hardsuits/sciencehead.rsi
Resources/Textures/Stray/Clothing/OuterClothing/Hardsuits/science.rsi
Resources/Textures/Stray/Ecclessia/Clothing/Head/ArmoredHood.rsi
Resources/Textures/NES/Clothing/Hardsuits/rnd.rsi
Resources/Textures/NES/Clothing/Head/Hardsuits/rnd.rsi
Resources/Textures/NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
Resources/Prototypes/Stray/Entities/Clothing/Head/hardsuit-helmet.yml (1)
27-28: Remove unprofessional commentThe warning comment about sprite changes should be removed as it's not appropriate for the codebase. If there are concerns about unauthorized changes, these should be addressed through proper channels like PR reviews or team discussions.
Resources/Prototypes/Stray/Entities/Clothing/OuterClothing/hardsuits.yml (1)
51-53: Review armor value increases for balanceThe significant increase in armor values (0.9 for Blunt, Slash, and Piercing) makes this hardsuit notably powerful. As noted in the comment, this makes it superior to the utility suit in all aspects, which might create balance issues.
Consider:
- Maintaining some trade-offs between different suit types
- Documenting the reasoning for these balance changes
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (36)
Resources/Textures/NES/Clothing/Hardsuits/rnd.rsi/equipped-OUTERCLOTHING.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/Hardsuits/rnd.rsi/icon.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/Hardsuits/rnd.rsi/inhand-left.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/Hardsuits/rnd.rsi/inhand-right.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/Head/Hardsuits/rnd.rsi/equipped-HELMET.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/Head/Hardsuits/rnd.rsi/icon.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat.rsi/equipped-OUTERCLOTHING-reptilian.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat.rsi/equipped-OUTERCLOTHING-vox.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat.rsi/equipped-OUTERCLOTHING.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat.rsi/icon-open.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat.rsi/icon.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat.rsi/open-equipped-OUTERCLOTHING-reptilian.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat.rsi/open-equipped-OUTERCLOTHING-vox.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat.rsi/open-equipped-OUTERCLOTHING.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi/equipped-OUTERCLOTHING.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi/icon-open.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi/icon.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi/inhand-left.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi/inhand-right.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi/open-equipped-OUTERCLOTHING.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi/open-inhand-left.pngis excluded by!**/*.pngResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi/open-inhand-right.pngis excluded by!**/*.pngResources/Textures/Stray/Clothing/OuterClothing/Hardsuits/science.rsi/equipped-OUTERCLOTHING-reptilian.pngis excluded by!**/*.pngResources/Textures/Stray/Clothing/OuterClothing/Hardsuits/science.rsi/equipped-OUTERCLOTHING-vox.pngis excluded by!**/*.pngResources/Textures/Stray/Clothing/OuterClothing/Hardsuits/science.rsi/equipped-OUTERCLOTHING.pngis excluded by!**/*.pngResources/Textures/Stray/Clothing/OuterClothing/Hardsuits/science.rsi/icon.pngis excluded by!**/*.pngResources/Textures/Stray/Clothing/OuterClothing/Hardsuits/science.rsi/inhand-left.pngis excluded by!**/*.pngResources/Textures/Stray/Clothing/OuterClothing/Hardsuits/science.rsi/inhand-right.pngis excluded by!**/*.pngResources/Textures/Stray/Ecclessia/Clothing/Head/ArmoredHood.rsi/equipped-head-light.pngis excluded by!**/*.pngResources/Textures/Stray/Ecclessia/Clothing/Head/ArmoredHood.rsi/icon-flash.pngis excluded by!**/*.pngResources/Textures/Stray/Ecclessia/Clothing/Head/ArmoredHood.rsi/icon.pngis excluded by!**/*.pngResources/Textures/Stray/Ecclessia/Clothing/Head/ArmoredHood.rsi/light-overlay.pngis excluded by!**/*.pngResources/Textures/Stray/Ecclessia/Clothing/Head/ArmoredHood.rsi/off-equipped-HELMET-shadowkin.pngis excluded by!**/*.pngResources/Textures/Stray/Ecclessia/Clothing/Head/ArmoredHood.rsi/off-equipped-HELMET.pngis excluded by!**/*.pngResources/Textures/Stray/Ecclessia/Clothing/Head/ArmoredHood.rsi/on-equipped-HELMET-shadowkin.pngis excluded by!**/*.pngResources/Textures/Stray/Ecclessia/Clothing/Head/ArmoredHood.rsi/on-equipped-HELMET.pngis excluded by!**/*.png
📒 Files selected for processing (12)
Content.Client/Lobby/UI/LobbyGui.xaml(2 hunks)Resources/Maps/NES/Port.yml(0 hunks)Resources/Prototypes/Entities/Clothing/OuterClothing/coats.yml(2 hunks)Resources/Prototypes/Maps/Pools/default.yml(2 hunks)Resources/Prototypes/Stray/Entities/Clothing/Head/hardsuit-helmet.yml(1 hunks)Resources/Prototypes/Stray/Entities/Clothing/OuterClothing/hardsuits.yml(1 hunks)Resources/Prototypes/_NES/Maps/port.yml(1 hunks)Resources/Textures/NES/Clothing/Hardsuits/rnd.rsi/meta.json(1 hunks)Resources/Textures/NES/Clothing/Head/Hardsuits/rnd.rsi/meta.json(1 hunks)Resources/Textures/NES/Clothing/OuterClothing/Coats/labcoat.rsi/meta.json(0 hunks)Resources/Textures/NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi/meta.json(3 hunks)Resources/Textures/Stray/Ecclessia/Clothing/Head/ArmoredHood.rsi/meta.json(0 hunks)
💤 Files with no reviewable changes (3)
- Resources/Textures/Stray/Ecclessia/Clothing/Head/ArmoredHood.rsi/meta.json
- Resources/Textures/NES/Clothing/OuterClothing/Coats/labcoat.rsi/meta.json
- Resources/Maps/NES/Port.yml
✅ Files skipped from review due to trivial changes (2)
- Resources/Textures/NES/Clothing/Head/Hardsuits/rnd.rsi/meta.json
- Resources/Textures/NES/Clothing/Hardsuits/rnd.rsi/meta.json
🔇 Additional comments (12)
Resources/Prototypes/Maps/Pools/default.yml (2)
51-51: LGTM: Formatting fix for Aspid entry.
The newline addition for the Aspid entry improves file formatting consistency.
4-4: Consider map placement impact on rotation.
The new map Nes14Port has been added at the beginning of the list. Verify if the placement order affects map rotation logic or selection probability.
Resources/Textures/NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi/meta.json (2)
13-15: LGTM: Sprite state additions for open variants.
The new states for open variants of the labcoat follow consistent naming patterns and include proper direction counts. This aligns with the PR's sprite update objectives.
Also applies to: 21-22, 31-38
4-4: Verify copyright attribution change.
The copyright has been changed from tgstation to "NES-Station : Aserovich". Please ensure this change complies with the original asset's license terms and attribution requirements.
✅ Verification successful
Let me analyze the search results and generate additional verification scripts to check the copyright attribution change.
This will help us verify:
- The history of copyright changes in this specific file
- If there are other files with NES-Station attribution
- If there are other labcoat files with tgstation attribution
This information will help determine if this is a consistent change and follows proper attribution patterns.
Let me try one more verification to check the labcoat files with proper path patterns:
This will help us:
- Find all labcoat RSI directories
- See the full content of the file in question
- Find any tgstation copyright patterns in the codebase
Based on the search results, I can now generate the final response:
Copyright attribution change appears appropriate
The file is located in the NES directory (Resources/Textures/NES/...) and there are numerous other files in the NES directory that use the "NES-Station" copyright attribution. This appears to be a consistent pattern for assets in the NES directory. Additionally, there is no evidence of this specific labcoat sprite previously having a tgstation copyright, as it seems to be original content for the NES collection.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other tgstation copyrights that might need similar updates
rg -l "Taken from tgstation" --type json
Length of output: 67142
Script:
#!/bin/bash
# Check the git history of the specific file to see copyright changes
git log -p -- Resources/Textures/NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi/meta.json
# Search for any other NES-Station copyrights
rg -l "NES-Station" --type json
# Check if there are any tgstation copyrights specifically in labcoat files
rg -l "tgstation" --type json -- "**/labcoat*.rsi/meta.json"
Length of output: 8487
Script:
#!/bin/bash
# Search for labcoat files with proper path patterns
fd "labcoat.*\.rsi" -t d
# Check content of the specific meta.json file
cat Resources/Textures/NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi/meta.json
# Search for any tgstation copyright in the codebase
rg "copyright.*tgstation" --type json
Length of output: 65910
Resources/Prototypes/_NES/Maps/port.yml (1)
11-15: LGTM: Station name configuration.
The station name setup with the Port template and FD prefix creator looks good.
Content.Client/Lobby/UI/LobbyGui.xaml (2)
37-37: Well-structured button grid layout!
The grid container provides a clean vertical organization for the buttons with consistent margins and styling.
100-100: Consistent visual styling!
The increased thickness of the gold line (10) maintains visual consistency with other separators in the UI.
Resources/Prototypes/Entities/Clothing/OuterClothing/coats.yml (2)
154-156: Consistent sprite path updates!
The sprite paths are correctly updated for both Sprite and Clothing components, aligning with the PR's sprite update objectives.
154-156: Verify new sprite resources exist.
Let's ensure all the new sprite resources are present in the repository.
Also applies to: 256-258
✅ Verification successful
Sprite resources verified and found
All referenced sprite resources are present in the repository with their complete set of required textures and metadata files:
Resources/Textures/NES/Clothing/OuterClothing/Coats/labcoat.rsiResources/Textures/NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the new sprite resources exist
echo "Checking for new sprite resources..."
for path in "NES/Clothing/OuterClothing/Coats/labcoat.rsi" "NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi"; do
if fd -p "$path" Resources/Textures/; then
echo "✓ Found: $path"
else
echo "✗ Missing: $path"
fi
done
Length of output: 2356
Resources/Prototypes/Stray/Entities/Clothing/Head/hardsuit-helmet.yml (1)
30-30: Verify impact of parent class change and sprite updates
The change from ClothingHeadHardsuitWithLightBase to ClothingHeadEVAHelmetBase and the removal of the PointLight component represents a significant change in functionality. Additionally, the sprite path has been updated to a new location.
Let's verify these changes:
Also applies to: 36-36, 38-38
Resources/Prototypes/Stray/Entities/Clothing/OuterClothing/hardsuits.yml (2)
60-61: Document speed reduction rationale
While the comment explains the intent to discourage station-wide mobility, such significant gameplay changes should be:
- Documented more formally
- Tested to ensure they don't negatively impact intended use cases
- Discussed with the team to ensure alignment with game design goals
Let's check for similar speed modifiers in other suits:
✅ Verification successful
Based on the search results, I can now generate the final response since I have enough context about speed modifiers across the codebase:
Speed modifier is within established range
The 0.65 speed modifier is well within the established range for hardsuits in the codebase:
- Most hardsuits range from 0.6 to 1.0 for both walk and sprint modifiers
- Similar heavy suits like bomb suits use 0.7
- Combat/security hardsuits commonly use 0.8-0.85
- The 0.65 modifier aligns with other restricted mobility suits
The comment's intent to limit station-wide mobility is consistent with how speed modifiers are used throughout the codebase to balance equipment capabilities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all hardsuit speed modifiers for comparison
rg "ClothingSpeedModifier" -A 2
Length of output: 54485
42-42: Verify sprite asset consistency
The sprite path changes align with the helmet changes, ensuring visual consistency. However, we should verify that all required sprite states exist in the new location.
Also applies to: 44-44
✅ Verification successful
Sprite assets verified and consistent
The sprite path NES/Clothing/Hardsuits/rnd.rsi exists and contains all the required states for a hardsuit:
- Base icon state
- Equipped state (OUTERCLOTHING)
- Both left and right inhand states
The corresponding helmet sprite at NES/Clothing/Head/Hardsuits/rnd.rsi also exists with its required states. The paths in the YAML file correctly match the actual file structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the new sprite assets exist and have all required states
fd "rnd.rsi/meta.json$" -x cat {} | jq '.states[].name'
Length of output: 59
Script:
#!/bin/bash
# First, let's check if the sprite file exists in the expected location
fd "rnd.rsi$"
# Let's also check for any similar sprite files in case there's a naming pattern
fd ".rsi$" | grep -i "hardsuit\|rnd"
# Additionally, let's check the directory structure around where we expect the sprite
find . -type d -name "NES" -o -name "Clothing" -o -name "Hardsuits"
Length of output: 12082
Script:
#!/bin/bash
# Let's check the contents of both the hardsuit and helmet RSI files to verify states
echo "=== Hardsuit RSI states ==="
cat "Resources/Textures/NES/Clothing/Hardsuits/rnd.rsi/meta.json" 2>/dev/null | jq -r '.states[].name'
echo -e "\n=== Helmet RSI states ==="
cat "Resources/Textures/NES/Clothing/Head/Hardsuits/rnd.rsi/meta.json" 2>/dev/null | jq -r '.states[].name'
# Also check if the path in the YAML matches the actual file structure
echo -e "\n=== Checking YAML path validity ==="
grep -r "NES/Clothing/Hardsuits/rnd.rsi" Resources/Prototypes/
Length of output: 774
| minPlayers: 1 | ||
| maxPlayers: 10 |
There was a problem hiding this comment.
Adjust player limits to match PR description.
The PR description mentions this station is designed for 6 players, but the configuration allows 1-10 players. Consider adjusting these limits to better reflect the intended design:
minPlayersshould probably be higher than 1 to ensure minimal staffingmaxPlayersshould align with the stated 6-player design
| Captain: [ 1, 1 ] | ||
| #service | ||
| Janitor: [ 1, 1 ] | ||
| ServiceWorker: [ 1, 1 ] | ||
| #engineering | ||
| StationEngineer: [ 1, 1 ] | ||
| TechnicalAssistant: [ 1, 1 ] | ||
| #medical | ||
| Paramedic: [ 1, 1 ] | ||
| MedicalIntern: [ 1, 1 ] | ||
| #science | ||
| ResearchAssistant: [ 1, 1 ] | ||
| Scientist: [ 1, 1 ] | ||
| #security | ||
| HeadOfSecurity: [ 1, 1 ] | ||
| SecurityCadet: [ 1, 1 ] | ||
| SecurityOfficer: [ 1, 1 ] | ||
| #supply | ||
| CargoTechnician: [ 1, 1 ] | ||
| SalvageSpecialist: [ 1, 1 ] |
There was a problem hiding this comment.
Review job configuration - Current setup is too restrictive.
The current job configuration has several issues:
- Most roles are set to exactly one player [1, 1], which is very restrictive
- The sum of minimum required staff (13 roles × 1 minimum each) exceeds even the maximum player limit (10)
- This rigid configuration doesn't allow for flexible role distribution
Consider:
- Reducing minimum requirements for non-essential roles
- Allowing more flexibility in maximum slots
- Prioritizing essential roles
Example adjustment:
availableJobs:
- Captain: [ 1, 1 ]
+ Captain: [ 1, 1 ] # Keep one required captain
#service
- Janitor: [ 1, 1 ]
- ServiceWorker: [ 1, 1 ]
+ Janitor: [ 0, 1 ]
+ ServiceWorker: [ 0, 1 ]
#engineering
- StationEngineer: [ 1, 1 ]
- TechnicalAssistant: [ 1, 1 ]
+ StationEngineer: [ 1, 2 ] # At least one engineer
+ TechnicalAssistant: [ 0, 1 ]Committable suggestion skipped: line range outside the PR's diff.
| sprite: NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi # NES14-Changes, fuck DV | ||
| - type: Clothing | ||
| sprite: DeltaV/Clothing/OuterClothing/Coats/epicoat.rsi # DeltaV - Epistemic lab coat | ||
| sprite: NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi # NES14-Changes, fuck DV |
There was a problem hiding this comment.
Remove unprofessional comments.
While the sprite path updates are correct, please remove the unprofessional comment. Consider replacing it with a technical explanation of the change if needed.
- sprite: NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi # NES14-Changes, fuck DV
+ sprite: NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi # Updated to NES14 sprite path📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sprite: NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi # NES14-Changes, fuck DV | |
| - type: Clothing | |
| sprite: DeltaV/Clothing/OuterClothing/Coats/epicoat.rsi # DeltaV - Epistemic lab coat | |
| sprite: NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi # NES14-Changes, fuck DV | |
| sprite: NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi # Updated to NES14 sprite path | |
| - type: Clothing | |
| sprite: NES/Clothing/OuterClothing/Coats/labcoat_rnd.rsi # Updated to NES14 sprite path |
О чем PR
Почему
Технические детали
Медиа
Summary by CodeRabbit
New Features
Nes14Portwith player support and station configurations.Bug Fixes
Documentation
Chores