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

Add CBA_fnc_canAddItem #1330

Merged
merged 9 commits into from
Oct 10, 2020
Merged

Conversation

Dystopian
Copy link
Contributor

When merged this pull request will:

  • add function for checking if unit has enough free space in inventory to store item.

canAdd* commands can return false even if there is free space in unit inventory (see this ticket and other about canAdd). These commands take current unit load into account. Sometimes we don't need to check unit load e.g. when taking out ACE Earplugs or weapon accessories.

As usually function and variables names are subject to discuss.

@commy2
Copy link
Contributor

commy2 commented May 2, 2020

CBA_fnc_canAddItem?

@Dystopian
Copy link
Contributor Author

CBA_fnc_canAddItem?

I thought about it or even just CBA_fnc_canAdd but I didn't want to confuse it with canAdd command. Anyway for me naming here is not important so I accept any suggestions.

@commy2
Copy link
Contributor

commy2 commented May 2, 2020

The purpose is the same as the canAdd command though, and it is obvious that they behave differently, since one is a command, the other a function from CBA.

Co-authored-by: mharis001 <34453221+mharis001@users.noreply.github.com>
@commy2 commy2 added this to the 3.15.2 milestone May 2, 2020
@commy2 commy2 added the Feature label May 2, 2020
@Dystopian Dystopian changed the title Add CBA_fnc_unitHasSpace Add CBA_fnc_canAddItem May 4, 2020
@dedmen
Copy link
Contributor

dedmen commented May 5, 2020

This has the same issue as canAdd command.
Lets say you add 30 items with 1 space each.
In uniform, you have 12 spaces free, in vest you have 20 spaces free, in backpack you have 29 spaces free.

Yes you can add the 30 items, but canAdd returns false.

My idea for the solution would be to not check if count of items fits, but instead for uniform/vest/backpack instead count how many of the items could fit into each of them.
Sum the number up and check if the possible number of items that can fit is bigger than the requested count.

in my case
uniform could fit 12 items
+ vest can fit 20
+ backpack can fit 29
61 >= 30 -> return true.

instead of what you and canAdd command are doing
accumMass = 30*1

uniform: 12 < 30, false
vest: 20 < 30, false
backpack: 29 < 30, false
-> return false.

I also want to make this change to the canAdd command someday.

Copy link
Contributor

@commy2 commy2 left a comment

Choose a reason for hiding this comment

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

What Ded said.

@Dystopian
Copy link
Contributor Author

count made function a little bit more complex. Maybe function should return item count which can be put to unit? It would make function more universal.

@dedmen
Copy link
Contributor

dedmen commented May 6, 2020

I think instead of all the seperate exitWiths and stuff, its easier to just count up the total that fit and do one check at the end.
Though less efficient of course 🙃 (I prefer efficiency so just ignore what I said)

@commy2
Copy link
Contributor

commy2 commented May 7, 2020

lgtm.
Could add another abstraction layer by putting those container checks into a function and passing vest, vestLoad, checkVest and allowSlotVest etc. as arguments.
Would look nicer for sure.

@Dystopian
Copy link
Contributor Author

I thought about it. But for 3 containers I decided it's not worth it.

@commy2

This comment has been minimized.

@commy2
Copy link
Contributor

commy2 commented May 8, 2020

Even if you hate it, I am having fun.

#define CAN_ADD_TO(CONTAINER) _check##CONTAINER && {\
    DOUBLES(TYPE,CONTAINER) in _allowedSlots && {\
        _mass == 0 || {\
            _count = _count - floor (getContainerMaxLoad CONTAINER _unit * (1 - load##CONTAINER _unit) / _mass);\
            _count <= 0\
        }\
    }\
}


CAN_ADD_TO(UNIFORM) || {CAN_ADD_TO(VEST) || {CAN_ADD_TO(BACKPACK)}}

@PabstMirror PabstMirror merged commit 65e3698 into CBATeam:master Oct 10, 2020
@Dystopian Dystopian deleted the fnc_unitHasSpace branch October 11, 2020 20:23
@Freddo3000
Copy link
Contributor

Freddo3000 commented Oct 29, 2020

This has already been merged, but this problem seems to be tied to this: https://feedback.bistudio.com/T154476

canAdd returning false doesn't have to do with having over a certain amount of items, it has to do with having more than 1000 loadAbs, or 1.00 load on a unit, corresponding to maxSoldierLoad. It seems to possibly be a game bug that you can go over this limit.

@commy2
Copy link
Contributor

commy2 commented Oct 30, 2020

Sure, but this does not change anything in this PR, does it?

@Freddo3000
Copy link
Contributor

Not really, just adding additional details. Depending on whether the ticket is fixed later, this function may become redundant or invalid, though I wouldn't hold my breath at this stage of the game.

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.

None yet

6 participants