-
Notifications
You must be signed in to change notification settings - Fork 274
Garand Ping #2399
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
Garand Ping #2399
Conversation
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
WalkthroughThe "mark 1 rifle" and "ceremonial rifle" entities were refactored by changing their parent from Changes
Suggested labels
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 0
🧹 Nitpick comments (1)
Resources/Prototypes/Nyanotrasen/Entities/Objects/Weapons/Guns/Snipers/snipers.yml (1)
25-28
: Approved: ChamberMagazineAmmoProvider integrated.
The new auto-eject component looks solid. Ensure the sound asset/Audio/Weapons/Guns/EmptyAlarm/M1_empty_alarm.ogg
is present in the repo and functions as intended. Optionally, for consistency, you could add this provider to the ceremonial rifle (unless its behavior is meant to differ).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Resources/Audio/Weapons/Guns/EmptyAlarm/M1_empty_alarm.ogg
is excluded by!**/*.ogg
,!Resources/Audio/**
and included by none
📒 Files selected for processing (1)
Resources/Prototypes/Nyanotrasen/Entities/Objects/Weapons/Guns/Snipers/snipers.yml
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test Packaging
- GitHub Check: build (ubuntu-latest)
- GitHub Check: YAML Linter
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (1)
Resources/Prototypes/Nyanotrasen/Entities/Objects/Weapons/Guns/Snipers/snipers.yml (1)
20-20
: Approved: Magazine capacity boosted to 8 rounds.
The BallisticAmmoProvider capacity increase is correctly applied. Verify that the weapon’s reload logic, UI, and any ammo-pool constraints align with the new capacity to avoid runtime errors or balance issues.
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
…/Ammunition/Magazine/markone Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
…/Ammunition/Magazine/markone.yaml Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
Signed-off-by: Raikyr0 <Kurohana@hotmail.com.au>
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
♻️ Duplicate comments (4)
Resources/Prototypes/Nyanotrasen/Entities/Objects/Weapons/Guns/Snipers/snipers.yml (4)
44-45
: Duplicate: Rename entity ID after switching to Rifle parent.The ceremonial rifle also keeps
WeaponSniperCeremonial
despite now inheriting fromBaseWeaponRifle
—same confusion applies.
50-52
: Approve: Sprite layering on ceremonial rifle.Consistent with the grand rifle, the
layers
addition is correct and conforms to your visual layering scheme.
62-70
: Duplicate: Verify ItemSlots configuration and tags.Repeat the check for the ceremonial variant’s
MagazineMarkOne
andCartridgeLightRifle
configurations.
78-81
: Duplicate: Verify auto-ejection sound asset path.Confirm the same empty-alarm sound exists for the ceremonial rifle’s auto-eject.
🧹 Nitpick comments (1)
Resources/Prototypes/Nyanotrasen/Entities/Objects/Weapons/Guns/Snipers/snipers.yml (1)
3-4
: Critical: Rename entity ID after switching to Rifle parent.You’ve refactored the parent to
BaseWeaponRifle
but retainedid: WeaponSniperGrand
. This mismatch sabotages clarity. Rename to something likeWeaponRifleGrand
to reflect its new class.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Resources/Audio/Weapons/Guns/EmptyAlarm/M1_empty_alarm.ogg
is excluded by!**/*.ogg
,!Resources/Audio/**
and included by noneResources/Audio/Weapons/Guns/EmptyAlarm/M1_empty_alarm2.ogg
is excluded by!**/*.ogg
,!Resources/Audio/**
and included by none
📒 Files selected for processing (6)
Resources/Prototypes/DeltaV/Catalog/Fills/Crates/armory.yml
(1 hunks)Resources/Prototypes/Entities/Structures/Machines/lathe.yml
(3 hunks)Resources/Prototypes/Nyanotrasen/Entities/Objects/Weapons/Guns/Ammunition/Magazine/markone.yml
(1 hunks)Resources/Prototypes/Nyanotrasen/Entities/Objects/Weapons/Guns/Snipers/snipers.yml
(3 hunks)Resources/Prototypes/Recipes/Lathes/security.yml
(1 hunks)Resources/Prototypes/tags.yml
(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- Resources/Prototypes/DeltaV/Catalog/Fills/Crates/armory.yml
- Resources/Prototypes/tags.yml
- Resources/Prototypes/Nyanotrasen/Entities/Objects/Weapons/Guns/Ammunition/Magazine/markone.yml
- Resources/Prototypes/Entities/Structures/Machines/lathe.yml
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Test Packaging
🔇 Additional comments (4)
Resources/Prototypes/Recipes/Lathes/security.yml (2)
1458-1465
: Ensure recipe parent consistency for MarkOne magazine.Most caseless rifle magazine recipes declare
parent: BaseAmmoRecipe
to inherit default ammo behaviors. The newMagazineLightRifleMarkOne
recipe omits this—make sure that omission is deliberate and aligns with your ammo framework.
1450-1457
:✅ Verification successful
As Biesel Navy inspector: Validate MarkOne magazine entity definitions exist.
Before this lathe recipe can function, you must confirm that
MagazineLightRifleMarkOneEmpty
is defined under your magazine prototypes. A missing definition here will cause the factory lines to sputter out.
🏁 Script executed:
#!/bin/bash # Verify that the magazine entity exists rg -R "id: MagazineLightRifleMarkOneEmpty" -n Resources/Prototypes/Nyanotrasen/Entities/Objects/Weapons/Guns/Ammunition/MagazineLength of output: 535
Lieutenant, our previous scan misfired with the invalid
-R
flag. Reissuing proper orders to sweep the entireResources/Prototypes
directory for theMagazineLightRifleMarkOneEmpty
definition:
🏁 Script executed:
#!/bin/bash # Correctly search for the magazine entity definition across the Resources/Prototypes directory rg -n "id: MagazineLightRifleMarkOneEmpty" Resources/PrototypesLength of output: 290
Magazine prototype verified
Crew, the
MagazineLightRifleMarkOneEmpty
entity is declared inResources/Prototypes/Nyanotrasen/Entities/Objects/Weapons/Guns/Ammunition/Magazine/markone.yml
(line 2), so the lathe recipe will function as intended. No further action required.Resources/Prototypes/Nyanotrasen/Entities/Objects/Weapons/Guns/Snipers/snipers.yml (2)
9-11
: Approve: Sprite layering on grand rifle.The added
layers
entry cleanly attaches toenum.GunVisualLayers.Base
, enabling correct draw order. Indentation and syntax look flawless.
37-40
: Verify auto-ejection sound asset path.The
soundAutoEject
points to/Audio/Weapons/Guns/EmptyAlarm/M1_empty_alarm.ogg
. Ensure this file is present in your asset tree or the empty-ping won’t sound.rg "M1_empty_alarm.ogg" -n Assets/Audio/Weapons/Guns/EmptyAlarm
Description
Adds the iconic Garand Ping to the Mark 1 Rifles
Changes it so Mark 1 Rifles are now Rifle Parent instead of Sniper
Changed it so it uses a en-block magazine (nightmare coding) which you can print from sec, ammo and emagged autolathe.
Thanks to @ItsDzordz for making sure it didn't explode and testing.
Changelog
🆑