port Knuckle duster and fixing ntpd locker sprite #177
Conversation
|
Do not merge DNR this shit |
|
And i on phone in the middle of field while writing this pr guh |
📝 WalkthroughWalkthroughAdds knuckle duster weapons and variants (standard, brass, syndicate, quartermaster, bone crushers): new entity prototypes, sprite metadata, a construction recipe/graph for brass knuckles, an uplink catalog entry, loadout additions/enabled gloves group, and two localization entries. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Player
participant ConstructionUI as Construction UI
participant RecipeSystem as Recipe System
participant World as World / Spawner
Note over Player,ConstructionUI: Crafting brass knuckles
Player->>ConstructionUI: Select "Brass knuckle dusters"
ConstructionUI->>RecipeSystem: Start graph "ClothingHandsKnuckleDustersBrass"
RecipeSystem->>RecipeSystem: Verify materials (Brass x6)
RecipeSystem-->>Player: Begin doAfter(10s)
RecipeSystem->>World: Spawn entity `ClothingHandsKnuckleDustersBrass` at targetNode
World-->>Player: Item created (brass knuckles)
sequenceDiagram
autonumber
participant Player
participant Uplink as Uplink Store
participant Inventory
Note over Player,Uplink: Purchasing Syndicate Knuckle Dusters via uplink
Player->>Uplink: Purchase `UplinkGlovesKnuckleDusters` (Telecrystal 20)
Uplink->>Inventory: Grant `ClothingHandsKnuckleDustersSyndicate`
Inventory-->>Player: Item in inventory
sequenceDiagram
autonumber
participant Player
participant LoadoutUI as Loadout System
participant ItemGroups
Note over Player,LoadoutUI: Loadout selection includes new gloves
Player->>LoadoutUI: Open logistics officer loadouts
LoadoutUI->>ItemGroups: Query `LoadoutLogisticsOfficerGloves`
ItemGroups-->>LoadoutUI: Options include `LoadoutCommandQMBlackGlove`, `LoadoutCommandQMKnuckleDuster`
LoadoutUI-->>Player: Present glove options
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (1)
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 |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (12)
Resources/Textures/_Arcadis/Structure/locker.rsi/NTPD.pngis excluded by!**/*.pngResources/Textures/_Arcadis/Structure/locker.rsi/NTPD_door.pngis excluded by!**/*.pngResources/Textures/_Arcadis/Structure/locker.rsi/NTPD_open.pngis excluded by!**/*.pngResources/Textures/_Arcadis/Structure/locker.rsi/generic.pngis excluded by!**/*.pngResources/Textures/_Arcadis/Structure/locker.rsi/generic_door.pngis excluded by!**/*.pngResources/Textures/_Arcadis/Structure/locker.rsi/generic_open.pngis excluded by!**/*.pngResources/Textures/_Arcadis/Structure/locker.rsi/welded.pngis excluded by!**/*.pngResources/Textures/_Goobstation/Clothing/Hands/Gloves/knuckleduster.rsi/brassknuckleduster.pngis excluded by!**/*.pngResources/Textures/_Goobstation/Clothing/Hands/Gloves/knuckleduster.rsi/equipped-HAND.pngis excluded by!**/*.pngResources/Textures/_Goobstation/Clothing/Hands/Gloves/knuckleduster.rsi/goldenknuckleduster.pngis excluded by!**/*.pngResources/Textures/_Goobstation/Clothing/Hands/Gloves/knuckleduster.rsi/knuckleduster.pngis excluded by!**/*.pngResources/Textures/_Goobstation/Clothing/Hands/Gloves/knuckleduster.rsi/syndicateknuckleduster.pngis excluded by!**/*.png
📒 Files selected for processing (10)
Resources/Locale/en-US/store/uplink-catalog.ftl(1 hunks)Resources/Prototypes/CharacterItemGroups/Jobs/Logistics/logisticsOfficer.yml(1 hunks)Resources/Prototypes/Loadouts/Jobs/Logistics/logisticsOfficer.yml(1 hunks)Resources/Prototypes/Recipes/Construction/Graphs/weapons/brass_knuckles.yml(1 hunks)Resources/Prototypes/Recipes/Construction/weapons.yml(1 hunks)Resources/Prototypes/_Arc/Structures/Misc/lockers.yml(1 hunks)Resources/Prototypes/_Goobstation/Catalog/uplink_catalog.yml(1 hunks)Resources/Prototypes/_Goobstation/Entities/Clothing/Hands/gloves.yml(1 hunks)Resources/Textures/_Arcadis/Structure/locker.rsi/meta.json(1 hunks)Resources/Textures/_Goobstation/Clothing/Hands/Gloves/knuckleduster.rsi/meta.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packaging
- GitHub Check: build (ubuntu-latest)
- GitHub Check: YAML Linter
- GitHub Check: YAML map schema validator
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (12)
Resources/Prototypes/Loadouts/Jobs/Logistics/logisticsOfficer.yml (2)
13-37: AI summary inconsistency detected.The AI summary claims multiple loadout entries were removed (LoadoutCommandQMHeadSoft, LoadoutLogisticsOfficerNTPDA, LoadoutCommandQMNeckCloak, LoadoutQMWinterCoat, and uniform entries), but the provided code shows these entries still exist in lines 39-144. Only the Gloves section (lines 13-37) was added/modified based on the
~markers.
13-37: Entity ClothingHandsKnuckleDustersQM is defined and the loadout entries are properly structured.The entity
ClothingHandsKnuckleDustersQMis confirmed to exist in Resources/Prototypes/_Goobstation/Entities/Clothing/Hands/gloves.yml. The loadout configuration is correct and both glove loadout entries reference valid entities.Resources/Textures/_Arcadis/Structure/locker.rsi/meta.json (1)
1-32: LGTM!The RSI metadata is properly structured with appropriate states for the NTPD locker variants. The license and copyright information are correctly specified.
Resources/Prototypes/CharacterItemGroups/Jobs/Logistics/logisticsOfficer.yml (1)
27-34: LGTM!The character item group properly references the loadout entries added in
Resources/Prototypes/Loadouts/Jobs/Logistics/logisticsOfficer.yml. The structure is correct with appropriate max items limit.Resources/Prototypes/_Arc/Structures/Misc/lockers.yml (1)
10-28: LGTM! NTPD locker sprite fix implemented correctly.The sprite configuration properly references the new
_Arcadis/Structure/locker.rsiasset added in this PR. The layer structure follows the standard locker pattern, and the NTPD-specific states (lines 30-32) correctly reference the states defined in the texture metadata.Resources/Prototypes/Recipes/Construction/Graphs/weapons/brass_knuckles.yml (1)
1-13: Code changes approved.All verification checks passed. The entity
ClothingHandsKnuckleDustersBrassis properly defined ingloves.yml, and the construction recipe inweapons.ymlcorrectly references this graph. The construction graph structure is valid.Resources/Prototypes/_Goobstation/Catalog/uplink_catalog.yml (1)
161-170: All references verified — no issues found.Entity
ClothingHandsKnuckleDustersSyndicateis properly defined in the entities file, sprite file exists at the specified path, and sprite statesyndicateknuckledusteris confirmed in the RSI metadata. All catalog entry references are valid.Resources/Prototypes/_Goobstation/Entities/Clothing/Hands/gloves.yml (4)
1-31: Base entity structure is solid.The abstract base entity correctly defines the common melee weapon configuration with reasonable defaults: attackRate 1.5, 12 blunt damage, punch sounds/animation, and proper clothing slots. The component hierarchy is well-structured.
38-48: No issues found—construction graph is properly configured.The referenced construction graph file exists and is correctly configured. The
ClothingHandsKnuckleDustersBrassID matches the entity reference, the material requirements (6 Brass) are defined, and the entity mapping is accurate. The "Craftable Version" comment is legitimate and fully supported by the construction graph implementation.
1-101: Rewritten review comment:Clarify PR merge status before proceeding.
The sprite assets and construction graph dependencies referenced in this file are present and correctly configured:
- ✓ All sprite states exist:
knuckleduster,brassknuckleduster,syndicateknuckleduster,goldenknuckleduster- ✓ Construction graph file exists:
Resources/Prototypes/Recipes/Construction/Graphs/weapons/brass_knuckles.ymlHowever, the PR status requires confirmation. If the developer marked this PR as "do not merge," this should be explicitly resolved and the status updated before this can be merged.
87-101: Clarify attack speed and DPS calculations—review contains mathematical errors.The damage ratio comparison is accurate (50 is ~4.17× base and 3.125× syndicate), and the tag override appears intentional. However, the review contains two mathematical errors:
- Attack speed multiplier: attackRate is 2 versus base 1.5, which is 1.33× faster (33% increase), not double. Double attack speed would be 3.0.
- DPS calculation: 50 damage × 2 attackRate = 100 DPS maximum, not 200 DPS.
The underlying balance concern is reasonable to verify given the high relative damage, but the specific quantitative claims require correction. Review the actual DPS impact (100 DPS versus syndicate's 24 DPS) against centcomm equipment tier expectations.
Likely an incorrect or invalid review comment.
Resources/Prototypes/Recipes/Construction/weapons.yml (1)
177-186: All dependencies verified—construction recipe is complete and correct.The construction recipe references a properly defined entity, construction graph, and sprite asset. No issues found.
Resources/Textures/_Goobstation/Clothing/Hands/Gloves/knuckleduster.rsi/meta.json
Show resolved
Hide resolved
Added locked state
Phone webedit pt2
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Resources/Textures/_Arcadis/Structure/locker.rsi/locked.pngis excluded by!**/*.png
📒 Files selected for processing (1)
Resources/Textures/_Arcadis/Structure/locker.rsi/meta.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: YAML map schema validator
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: YAML Linter
- GitHub Check: Test Packaging
|
done you can merge this now |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Resources/Textures/Structures/Storage/closet.rsi/meta.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: YAML Linter
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Test Packaging
TheDoctor1977
left a comment
TheDoctor1977
left a comment
There was a problem hiding this comment.
seconding the bot’s reviews. most of those will actually cause problems if unresolved
TheDoctor1977
added
the
Status: Awaiting Changes
Eh consistency be damn you can blame gulo for the consistency issue when he put the name of the file. |
|
Because the file it self is capital letter so it not going to break or anything |
| - type: entity | ||
| parent: [ClothingHandsKnuckleDustersBase, BaseCommandContraband] | ||
| id: ClothingHandsKnuckleDustersQM | ||
| #suffix: DO NOT MAP - Arcadis turn em into normal command item |
There was a problem hiding this comment.
this should just be done as part of this PR. remove from the loadout and add to the LO’s locker fill
There was a problem hiding this comment.
what wrong letting it be in loadout. beside qm got the dollar thing as steal objective so should be fine.
There was a problem hiding this comment.
beside the knuckleduster arent steal objective. it just optional qm glove.
|
also fuck code rabbit i prob should try and fix the other test fail so that stupid ass rabbit wouldnt be rellied upon. |
TheDoctor1977
left a comment
TheDoctor1977
left a comment
There was a problem hiding this comment.
content approval
@eris-webserv review for code approval when you have the time
2 participants
Description
Dnr still havent fix it yet. My gotdamn dorm decided to do fire drill while making this prno longer applied i fix it.
Changelog
🆑