-
Notifications
You must be signed in to change notification settings - Fork 34
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
DryDock update #68
DryDock update #68
Conversation
WalkthroughThe pull request introduces a new game map named "DryDock" by adding an entry to the existing list of map locations and creating a new configuration file for "DryDock Station." The Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
Resources/Prototypes/Maps/drydock.yml (1)
18-71
: LGTM! Consider clarifying disabled roles.The job distribution looks well-balanced across different departments. The use of ranges for some roles provides flexibility in player assignments.
Consider adding a comment to explain the purpose of roles with [0, 0] quantities (Psychologist, Boxer). Are these intentionally disabled for this map, or is there another reason for their inclusion?
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- Resources/Prototypes/Maps/Pools/default.yml (1 hunks)
- Resources/Prototypes/Maps/drydock.yml (1 hunks)
Additional comments not posted (2)
Resources/Prototypes/Maps/Pools/default.yml (1)
17-17
: LGTM! The "DryDock" map has been successfully added to the default map pool.The addition of "DryDock" to the
DefaultMapPool
is consistent with the PR objectives and correctly implemented. This change will make the new map available in the game's map rotation.To ensure the "DryDock" map is properly implemented, let's verify its configuration:
Resources/Prototypes/Maps/drydock.yml (1)
7-17
: LGTM! Verify emergency shuttle file.The station configuration looks good. The use of StandardNanotrasenStation prototype and custom name generator with 'WWDP' prefix seems appropriate.
Please run the following script to verify the existence of the emergency shuttle file:
Verification successful
Please run the following script to locate the emergency shuttle file across the repository:
LGTM! Emergency shuttle file verified.
The emergency shuttle file
NTES_Delta.yml
exists atResources/Maps/Shuttles/DeltaV/NTES_Delta.yml
, which correctly matches the path specified in the configuration.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the emergency shuttle file. # Test: Check if the emergency shuttle file exists fd -t f "NTES_Delta.yml" Maps/Shuttles/DeltaVLength of output: 153
Script:
#!/bin/bash # Description: Search for the emergency shuttle file in the repository. fd -t f "NTES_Delta.yml"Length of output: 70
@PointPNG проваленный линтер актуальный |
в Content.IntegrationTests/Tests/PostMapInitTest.cs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
Content.IntegrationTests/Tests/PostMapInitTest.cs (1)
64-65
: LGTM! Consider standardizing comments for consistency.The addition of "DryDock" to the
GameMaps
array is correct and aligns with the PR objectives. This change ensures that the new map will be included in theGameMapsLoadableTest
.For consistency, consider using the same comment style as the other entries. If "WWDP" represents a different project or category, it might be worth adding a brief explanation in a code comment above the array.
- "Submarine", //DeltaV - "DryDock" //WWDP + "Submarine", // DeltaV + "DryDock" // WWDP (World Wide Dream Project)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- Content.IntegrationTests/Tests/PostMapInitTest.cs (1 hunks)
🔇 Additional comments not posted (2)
Content.IntegrationTests/Tests/PostMapInitTest.cs (2)
64-65
: Address linter failures before mergingRemuchi mentioned that the linter has failed for this PR. While the changes in this file look good, please ensure that all linter issues are resolved before merging.
Run the project's linter on the entire codebase and fix any reported issues. If you need assistance with specific linter errors, please let me know, and I'll be happy to help.
64-65
: Verify complete integration of "DryDock" into map rotationWhile adding "DryDock" to the
GameMaps
array ensures it will be tested for loading, we should verify that all necessary steps have been taken to fully integrate it into the map rotation.Please run the following script to check for any other files that might need updating to include "DryDock" in the map rotation:
This script will help identify any additional files that might need to be updated to fully integrate "DryDock" into the map rotation.
@PointPNG не забудь закоментить senior джобки |
Co-authored-by: Remuchi <72476615+Remuchi@users.noreply.github.com>
Co-authored-by: Evgencheg <73418250+Evgencheg@users.noreply.github.com>
Co-authored-by: Evgencheg <73418250+Evgencheg@users.noreply.github.com>
Co-authored-by: Evgencheg <73418250+Evgencheg@users.noreply.github.com>
Co-authored-by: Evgencheg <73418250+Evgencheg@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Принял все исправления, тесты успешны, пляшем.
я бы ещё советовал проверить карту в игре, побегать посмотреть. Вдруг 🤷 |
Описание PR
Чек-лист:
typo:
Изменения
🆑 PointPNG