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

[WIP][CR] Attempting to fix shotgun speedloaders: Speedloader adaptors #58580

Closed
wants to merge 8 commits into from

Conversation

catdach
Copy link
Contributor

@catdach catdach commented Jun 20, 2022

Summary

Bugfixes "fix shotgun speedloaders"

Purpose of change

Fixes #52595

Describe the solution

A new optional Json field for gunmod items: "speedloader_adaptor": [ "speedloader_item_id" ]

as a quick refresher on how speedloaders usually work, guns with integrated magazines(guns that have a pocket of type "magazine" rather than "magazine well") have the following optional field within their pocket data: "allowed_speedloaders": [ "speedloader_item_id" ]. In theory a mod with "speedloader_adaptor" will either replace, or add onto the end of, that data.

That's the idea anyway, I don't quite know how to implement this. Right now I'm basically throwing code at the wall to see what sticks. It seems like something that should be fairly simple for someone who knew what they were doing and/or were more familiar with the codebase than I am, so any help is appreciated.

Describe alternatives you've considered

  1. Trying to use pocket mod : I tried adding a line to the shotgun-speedloader-chute like
"pocket_mods": [ 
 {
 "pocket_type": "MAGAZINE",
 "ammo_restriction": { "shot": 8 },
 "allowed_speedloaders": [ "shot_speedloader6", "shot_speedloader8" ] 
 } 
] 

which comes close to being what I need, but it has some problems (see #53542). The main problem is the need to redefine "ammo_restriction" even if it wasn't the thing being changed

1a. asking for someone to change "ammo_restriction" to not need a count to be defined as it was already defined elsewhere in the item: (see #53375) essentially got shot down and had the opposite effect of the other places that it was defined being removed.

  1. trying to make "magazine_adaptor" actually work for speedboaters: The magazine adaptor code is pretty confusing and I'm not even sure if it works properly anymore since guns don't have a "magazines" list like they used to.

  2. restricting the speedloader chute to only ever be used on 8 round shotguns so that I can just use freaking "pocketmod": this seems like a quick and dirty non-solution, that feels more like I'm trying to hide the problem instead of fixing it.

Testing

It doesn't work yet (duh). Right now the game starts but crashes while loading into a world (not sure why, as no crash log is created).

Additional context

@github-actions github-actions bot added <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON labels Jun 20, 2022
@GuardianDll
Copy link
Member

GuardianDll commented Jun 20, 2022

Want to point the new 5.56 clips is somewhat related to this issue, i believe, you can use it in your tests
#58359

@github-actions github-actions bot added the json-styled JSON lint passed, label assigned by github actions label Jun 20, 2022
@catdach catdach changed the title [WP][CR] Attempting to fix shotgun speedloaders: Speedloader adaptors [WIP][CR] Attempting to fix shotgun speedloaders: Speedloader adaptors Jun 20, 2022
@github-actions github-actions bot added the Items: Containers Things that hold other things label Jul 16, 2022
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Sep 19, 2022
@Maleclypse
Copy link
Member

If this is ready for merge please resolve conflicts and bring out of draft.

@catdach
Copy link
Contributor Author

catdach commented Dec 5, 2022

Sorry, I haven't quite gotten this working properly yet, so it's still not ready for merge. When it is, I'll make sure to take it out of draft and resolve the conflicts.

@Maleclypse
Copy link
Member

Closed by #63287

@Maleclypse Maleclypse closed this Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Items: Containers Things that hold other things [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shotgun speedloader chute not work
3 participants