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

Disposable - Updated cba_disposable_fnc_replaceMagazineCargo #1662

Merged

Conversation

johnb432
Copy link
Contributor

@johnb432 johnb432 commented May 4, 2024

When merged this pull request will:

  • Title.
  • Cleaned up FUNC(replaceMagazineCargo).
  • Now also checks if launchers can fit in vests and uniform and replaces them if possible.
    I think this case is rare, but it's technically possible so I think CBA should be able to handle it.
  • Now handles all containers within containers (previously only backpacks within containers).
  • Fixes a bug related to grenades not being throwable (https://feedback.bistudio.com/T74244), as this now only removes the necessary magazines and not every magazine.

@PabstMirror
Copy link
Contributor

looks good overall, I don't understand the "isBackpack") != -1; either
also wondering if addWeaponCargoGlobal will ignore slot restrictions, will test in a bit

@johnb432
Copy link
Contributor Author

johnb432 commented May 4, 2024

also wondering if addWeaponCargoGlobal will ignore slot restrictions, will test in a bit

It does - I'm guessing that code was to stop that then.
Do we want to keep the current behaviour? I don't like that stuff vanishes, just because it isn't allowed to be in a backpack. I'd prefer if it were dropped on the ground, but that becomes quite messy.

@PabstMirror
Copy link
Contributor

PabstMirror commented May 5, 2024

loadout for b_lat:
2 nlaws in the backpack (nlaws can't normally fit in backpacks)
20240504195434_1

But I agree that this is better than having things disappear
i.e. in vanilla you would get 3 shots, now you still have 3 shots (before this PR you would just have the 1)

I guess we could always look at a setting if people really have a problem with it? (I'd rather not have to)

@PabstMirror PabstMirror added this to the 3.18.0 milestone May 5, 2024
@Drofseh
Copy link
Contributor

Drofseh commented May 5, 2024

I think you should always get the number of shots the mission maker intended. So fill up the backpack if required.

@johnb432
Copy link
Contributor Author

johnb432 commented May 5, 2024

Currently nobody wants to change it, but if in future we want to modify this PR, then I would not revert the code to the old stuff. It didn't take into account if launchers could fit in uniforms or vests, which is probably rare but not impossible.

@johnb432 johnb432 changed the title Disposable - Fix magazines not being replaced by their disposable counterparts Disposable - Updated cba_disposable_fnc_replaceMagazineCargo May 8, 2024
@johnb432
Copy link
Contributor Author

johnb432 commented May 8, 2024

Discussion in discord (https://discord.com/channels/976165959041679380/976228110078992456/1237139353650597908) concluded that the PR in its original form was not wanted. I made a new PR #1664 where only the fix is present.

This PR now only brings the code up-to-date and also checks if launchers can fit in uniforms and vests and replaces them if possible.

@johnb432 johnb432 requested a review from PabstMirror May 8, 2024 08:11
@PabstMirror
Copy link
Contributor

pushed a small change allowing us to keep the same args for cba_disposable_fnc_replaceMagazineCargo
my worry is someone might be doing something like

  • add loadout to crate
  • call cba_disposable_fnc_replaceMagazineCargo

and the change in args might cause problems (even though it says Internal Function)

@johnb432
Copy link
Contributor Author

johnb432 commented Aug 8, 2024

and the change in args might cause problems (even though it says Internal Function)

I really feel that's a "them" problem.
If you want the changes to remain, I'll need to cleanup some leftover stuff.

@johnb432
Copy link
Contributor Author

johnb432 commented Aug 8, 2024

I really feel that's a "them" problem. If you want the changes to remain, I'll need to cleanup some leftover stuff.

The reason I added both the container and its type is because it doesn't work for uniforms and vests. If you do typeOf uniformContainer player, it returns something like "Supply40", which can't be used to determine if a launcher is supposed to fit in a uniform/vest or not.

Usually launchers don't fit in uniforms and vests, but I'd prefer to be thorough and take every possibility into account.

@PabstMirror PabstMirror merged commit 8d54f31 into CBATeam:master Aug 9, 2024
4 checks passed
@johnb432 johnb432 deleted the disposable-cleanup-magazine-replacement branch August 9, 2024 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants