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

CBA_fnc_removeWeaponCargo deletes MX LSW #725

Closed
Entih opened this issue Jul 18, 2017 · 21 comments
Closed

CBA_fnc_removeWeaponCargo deletes MX LSW #725

Entih opened this issue Jul 18, 2017 · 21 comments
Labels
Milestone

Comments

@Entih
Copy link

Entih commented Jul 18, 2017

Arma 3 Version: 1.72 (stable)
CBA Version: 3.4.0 (stable)

Mods:

- CBA_A3
- ACE
- Overthrow

Description:

  • When the function CBA_fnc_removeWeaponCargo is used on a vehicle inventory containing the LMG variant of the MX rifle (baseWeapon name arifle_MX_SW_F), the bipods are properly stripped, but the weapon itself is replaced with a non-functional "dummy" item.

This appears to be an issue with the function CBA_fnc_getNonPresetClass, which returns an empty string for arifle_MX_SW_F and all variants.

Screenshot of post-function-call inventory: https://i.imgur.com/fTHi9Ob.jpg

Steps to reproduce:

  • Utilize debug console to remove a weapon from a container holding type MX LSW or MX SW, utilizing the CBA_fnc_removeWeaponCargo function. Ensure at least one of these weapons remains in the cargo after the function call, such that it will be stripped by the function.

Where did the issue occur?

  • Self-hosted LAN, Editor, Anywhere

RPT log file:

  • Issue does not incur RPT file response, apart from the following when viewing the resultant blank item in inventory:
    Warning Message: Picture equip\w\w_.paa not found
@jonpas
Copy link
Member

jonpas commented Jul 18, 2017

Must be because addBackpackCargo just ignores it when you pass it "", addWeaponCargo actually tries to add it though.

@jonpas
Copy link
Member

jonpas commented Jul 18, 2017

Nope, problem is actually in CBA_fnc_weaponComponents:

["arifle_mx_f"] call CBA_fnc_weaponComponents
-> ["arifle_mx_f"]

["arifle_mx_sw_f"] call CBA_fnc_weaponComponents
-> ["","bipod_01_f_snd"]

@jonpas
Copy link
Member

jonpas commented Jul 18, 2017

arifle_mx_sw_f has no public parent class without LinkedItems. 😟
So I guess the function is correct.

@jonpas
Copy link
Member

jonpas commented Jul 18, 2017

Unsure how to go about this, since there is no base class without linked items, you can't pass a weapon to retain attachments, you can only fully remove it. Another way would be adding another argument to remove or keep attachments instead of figuring that out from non-preset class - that would also be consistent with how I want to add support for keeping contents of backpacks/wearables in removeBackpackCargo/removeItemCargo. Basically for all functions it would be "keep contents", with "contents" being either attachments or things inside a container.

@commy2 @Killswitch00 thoughts?

@654wak654
Copy link
Contributor

654wak654 commented Jul 18, 2017

The MX SW might not be the only weapon effected by this either, did you test with the other machineguns that come with bipods?

@Dorbedo
Copy link
Contributor

Dorbedo commented Jul 18, 2017

How about evaluating the difference between the attachments of the baseWeapon and the provided weapon?

btw: I'm missing the support of the manual baseWeapon value inside cba_fnc_weaponComponents.

@commy2
Copy link
Contributor

commy2 commented Jul 18, 2017

Since

arifle_mx_sw_f has no public parent class without LinkedItems

is the case, there is no way to add this weapon without bipod attachment to a box with scripting.

@commy2
Copy link
Contributor

commy2 commented Jul 18, 2017

baseWeapon is ignored by weaponComponents, because the entry does nothing and most addons probably don't support it.
BIS_fnc_weaponComponents also ignores it.

@jonpas
Copy link
Member

jonpas commented Jul 18, 2017

Yes commy, but we want to remove it, problem is the function can't make out if attachments should be retained or not, because I based that on comparison of non-preset equals passed. I think the best way is another param, that way you could pass preset or non-preset and always get same results based on "keep attachments" param.

Same would then be for other functions for "keep contents".

@commy2
Copy link
Contributor

commy2 commented Jul 18, 2017

Imo the result should be:

["arifle_mx_sw_f"] call CBA_fnc_weaponComponents
-> ["arifle_mx_sw_f"]

So unlike the BIS version, it should not list "bipod_01_f_snd", because that is an element of the most basic weapon.

@jonpas
Copy link
Member

jonpas commented Jul 18, 2017

That's not what the function is.

Reports class name of base weapon without attachments and all attachments belonging to a pre equipped weapon.

@commy2
Copy link
Contributor

commy2 commented Jul 18, 2017

Well, there is no base weapon without attachments in some cases.

@jonpas
Copy link
Member

jonpas commented Jul 18, 2017

Exactly, which is why it returns "" when such base weapon doesn't exist, but the second part is still there, and all is correct.

Those functions do exactly what they are supposed to. The only problem is how we handle them in removeXCargo functions, as I assumed there is always a non-preset weapon in existence ... clearly not. Instead of deciphering if attachments should be removed or not by checking if preset weapon is equal to weapon found in box, I suggest another param "keep attachments", that would solve everything and also make it more readable and straightforward to the user.

@commy2
Copy link
Contributor

commy2 commented Jul 18, 2017

I also think about usefulness and I'd prefer a function that reported the most basic weapon and all other attachments.

@jonpas
Copy link
Member

jonpas commented Jul 18, 2017

But we can't change what it returns now, that would break backwards compatibility on anyone relying that it returns "" when there is no base weapon.

That doesn't solve the issue with removeXCargo anyways, so what are your thoughts on another param, yes or no?

@commy2
Copy link
Contributor

commy2 commented Jul 18, 2017

I have no idea how your keep attachments proposal would work, as it seems to me to be impossible to add this weapon without bipod.

@Dorbedo
Copy link
Contributor

Dorbedo commented Jul 18, 2017

I think, the best way would be a rewrite of cba_fnc_removeWeaponCargo, since it should not separate the bipod of "arifle_mx_sw_f". Therefore the CBA_fnc_getNonPresetClass should not be used.
I consider the current behavior as a bug.

@jonpas
Copy link
Member

jonpas commented Jul 18, 2017

I have no idea how your keep attachments proposal would work, as it seems to me to be impossible to add this weapon without bipod.

It's removeWEAPONcargo, weapon will be removed either way, no one is even trying to add it back. It's simply a question from logic handling perspective.

I think, the best way would be a rewrite of cba_fnc_removeWeaponCargo, since it should not separate the bipod of "arifle_mx_sw_f". Therefore the CBA_fnc_getNonPresetClass should not be used.
I consider the current behavior as a bug.

I added that in #706 and specifically asked if that is wanted or not, then it simply got merged without any answer, so I assumed it was wanted. I would still prefer a param for that rather. And with this "not every weapon has base class without preset attachments" it'll have to be changed to that anyways it seems.

@commy2
Copy link
Contributor

commy2 commented Jul 18, 2017

Well, then I don't know. I don't have any use for either personally.

@jonpas
Copy link
Member

jonpas commented Jul 18, 2017

Actually, there is a point you made @commy2 we were relying on adding non-preset weapon, because a preset weapon can have custom attachments on them, and just readding a preset weapon would make them disappear (that is the other weapons in the container, not the one we are removing, since they all get removed and then readded). The only way I see to do that now is to read LinkedItems and compare to current attachments, but there is still no way to add it without the bipod (in this case) in case it was removed.

I don't see what we can do about this, since there is no way to add a weapon with specific attachments to the container, might as well make a note that attachments will reappear on those weapons (bipod in this case)?

@commy2
Copy link
Contributor

commy2 commented Jul 18, 2017

Yes, there is no way around that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants