Skip to content
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

Fix gunmod checks #2737

Merged

Conversation

KheirFerrum
Copy link
Collaborator

@KheirFerrum KheirFerrum commented Apr 30, 2023

Summary

SUMMARY: Bugfixes "Fix gunmod installation checks for integral magazines."

Purpose of change

When gunmods are installed, warning for magazine adaptor appears in inappropriate situations. As the check does nothing within the same universe as what the warning is meant to indicate. It checks whether the gun's loaded magazine has a default ammo type. Meaning that if you could install a conversion kit while the gun was loaded (which is impossible), the check would pass.

Furthermore, it fails due to guns with integral magazines not returning a default ammo type, as they count as "not loaded" with a magazine. It also fails when unloaded guns are modified, even when the modification doesn't change the ammo type used by the gun.

Describe the solution

Rewrite the entire check so it provides appropriate warning.

Describe alternatives you've considered

  • Make the code only check if ammo type is changed, stating that a magazine adaptor may be required.
    • Would be a hell of a lot simpler, but I have standards.

Testing

  • Spawn M4A1, remove magazine, install any compatible gunmod that is not a conversion kit: waterproofing, shoulderstrap, suppressor, etc. Check that no warning fires.
    • With magazine in, install any compatible gunmod, check that no warning fires.
  • Via Json, add Bore mod location to Remington 1100.
    • Spawn Remington 1100 shotgun, install any compatible gunmod that is not a conversion kit. Check that no warning fires.
  • Spawn 7.62x51mm caliber conversion kit, install into M4A1 and Remington, check that warning only fires for M4A1.
  • Spawn .300 AAC Blackout AR-15 conversion kit. Install into M4A1, check that no warning fires.

Additional context

@github-actions github-actions bot added the src changes related to source code. label Apr 30, 2023
@KheirFerrum KheirFerrum force-pushed the Fix-gunmod-integral-magazine branch from 7e8b410 to 1f59aef Compare April 30, 2023 16:38
@KheirFerrum KheirFerrum force-pushed the Fix-gunmod-integral-magazine branch from 1f59aef to 43b5cf7 Compare April 30, 2023 16:40
@scarf005
Copy link
Member

scarf005 commented May 1, 2023

could you add instructions on how to test the PR?

@chaosvolt
Copy link
Member

Related issue: #649

Testing the main issue this seeks to fix, as quoted from the bug report:

1. Debug spawn in a mosin nagant or other firearm that's loaded directly.
2. Spawn in a shoulder strap, suppressor, whatever else will fit on your gun of choice.
3. Install the gunmod, making note of the warning message shown.
4. Unload and reload the gun to confirm it still accepts cartridges just fine.

If fixed, step three will differ in there being no warning message displayed.

Testing whether conversion kits still work SHOULD still be doable by spawning them in debug, as I found them still buried in json/obsoletion/items.json so search the item spawn menu for conversion kit

@KheirFerrum
Copy link
Collaborator Author

Related issue: #649

Testing the main issue this seeks to fix, as quoted from the bug report:

1. Debug spawn in a mosin nagant or other firearm that's loaded directly.
2. Spawn in a shoulder strap, suppressor, whatever else will fit on your gun of choice.
3. Install the gunmod, making note of the warning message shown.
4. Unload and reload the gun to confirm it still accepts cartridges just fine.

If fixed, step three will differ in there being no warning message displayed.

Testing whether conversion kits still work SHOULD still be doable by spawning them in debug, as I found them still buried in json/obsoletion/items.json so search the item spawn menu for conversion kit

My concern there is mainly that since they've been obsoleted for a while they may no longer be compatible. Furthermore, I'm not even sure what magazine adaptors exist and where we would test that.

Since it fires if the gun has no compatible magazine loaded.
@KheirFerrum KheirFerrum force-pushed the Fix-gunmod-integral-magazine branch from dfc7eab to 8d5a795 Compare May 2, 2023 01:19
@KheirFerrum KheirFerrum force-pushed the Fix-gunmod-integral-magazine branch from 8d5a795 to 865c4a6 Compare May 2, 2023 01:53
@scarf005 scarf005 self-assigned this May 11, 2023
Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that the tests work:

waterproofings were used for testing all non-conversion gunmods.

Spawn M4A1, remove magazine, install any compatible gunmod that is not a conversion kit: waterproofing, shoulderstrap, suppressor, etc. Check that no warning fires.

image

With magazine in, install any compatible gunmod, check that no warning fires.

image

Via Json, add Bore mod location to Remington 1100
Spawn Remington 1100 shotgun, install any compatible gunmod that is not a conversion kit. Check that no warning fires

image

Spawn 7.62x51mm caliber conversion kit, install into M4A1 and Remington, check that warning only fires for M4A1.

image

image

Spawn .300 AAC Blackout AR-15 conversion kit. Install into M4A1, check that no warning fires.

image

image

.300 BLK M4A1 can still be loaded with 5.56x45mm rounds (and it won't fire)

image

image

image

tracked in #2817

Testing conversion kit with shotguns

image

image

image

while the gunmods installation works, there's still 00 shot left after installing conversion kit to remington 1100. but i guess it's okay as the game does not let you shoot while incompatible ammos are loaded

#649

image

LGTM

@scarf005 scarf005 merged commit ab72eab into cataclysmbnteam:upload May 12, 2023
@KheirFerrum KheirFerrum deleted the Fix-gunmod-integral-magazine branch May 12, 2023 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect warning message on installing gunmods to guns with internal magazines
3 participants