[BUG] In-inventory crafting not working?

nzkfc commented 1 year ago

Summary

Combining items that have been setup as inventory crafting doesn't work

Reproduction

Spawn lockpick and screwdriverset
Drag one onto the other
Nothing happens

Expected behavior

It should combine into an advancedlockpick as per the qb-core\shared\items.lua combine code

Actual behavior

Makes a noise, does nothing.

Additional context

https://youtu.be/Z3Myt8sNYUk

Last Updated

Today

Custom Resources

100% pure QB reciepie as of today

Resource Rename

Nope

tom-osborne commented 1 year ago

@nzkfc According to items.lua dropping a screwdriverset onto a lockpick should create an advancedlockpick However, dropping a lockpick onto a screwdriverset will do nothing.

Can you confirm you were doing it the right way around? i.e. dropping a screwdriverset onto a lockpick

Edit: sorry just seen your video demonstrating both methods.

nzkfc commented 1 year ago

I've tried it both ways in multiple slots and players report also who use it daily, but I will retest tonight on fresh as well

On Fri, 11 Nov 2022, 2:12 am Stan, @.***> wrote:

@nzkfc https://github.com/nzkfc According to items.lua dropping a screwdriverset onto a lockpick should create an advancedlockpick However, dropping a lockpick onto a screwdriverset will do nothing.

Can you confirm you were doing it the right way around? i.e. dropping a screwdriverset onto a lockpick

— Reply to this email directly, view it on GitHub https://github.com/qbcore-framework/qb-inventory/issues/377#issuecomment-1310262769, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGHUHI5PC4TZB5ZEBPH2UTWHTYEXANCNFSM6AAAAAAR3EXLX4 . You are receiving this because you were mentioned.Message ID: @.***>

nzkfc commented 1 year ago

Yep still happening on fresh pull as of today. I also tested gatecrack and electronickit and they don't combine either to get the trojan_usb item.

mikepauer commented 1 year ago

It broke with #376 as that's the only recent change I've pulled into my inventory, and it's broken on my install too.

tom-osborne commented 1 year ago

Thanks, will check it out

nzkfc commented 1 year ago

It broke with #376 as that's the only recent change I've pulled into my inventory, and it's broken on my install too.

@tom-osborne I have reverted the change from #376 on a fresh pull and re-tested, can confirm that the issue still exists so I don't believe 376 caused it.

mikepauer commented 1 year ago

It broke with #376 as that's the only recent change I've pulled into my inventory, and it's broken on my install too.

@tom-osborne I have reverted the change from #376 on a fresh pull and re-tested, can confirm that the issue still exists so I don't believe 376 caused it.

Interesting, as I reverted ours to just (fromData.amount == $toAmount) and right away combine started working again, and swapping item slots started working again (with 376 combine wasn't working, and dragging non-unique items from one slot to another was no longer swapping them).

tom-osborne commented 1 year ago

Reverting fixes combining for me. Debugging now.

tom-osborne commented 1 year ago

@nzkfc @mikepauer Please test #378

nzkfc commented 1 year ago

You sure you didn't have the other fix that someone put into PR to try fix the items going poof?

On Sat, 12 Nov 2022, 11:51 am mikepauer, @.***> wrote:

It broke with #376 https://github.com/qbcore-framework/qb-inventory/pull/376 as that's the only recent change I've pulled into my inventory, and it's broken on my install too.

@tom-osborne https://github.com/tom-osborne I have reverted the change from #376 https://github.com/qbcore-framework/qb-inventory/pull/376 on a fresh pull and re-tested, can confirm that the issue still exists so I don't believe 376 caused it.

Interesting, as I reverted ours to just (fromData.amount == $toAmount) and right away combine started working again, and swapping item slots started working again (with 376 combine wasn't working, and dragging non-unique items from one slot to another was no longer swapping them).

— Reply to this email directly, view it on GitHub https://github.com/qbcore-framework/qb-inventory/issues/377#issuecomment-1312248325, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGHUHMCMZC6QQQ2ZBOMTYDWH3EXDANCNFSM6AAAAAAR3EXLX4 . You are receiving this because you were mentioned.Message ID: @.***>

tom-osborne commented 1 year ago

What?

nzkfc commented 1 year ago

Oh it didn't quote the other guy, I was checking with him if he was using your PR in conjunction with #348 which was open for a bit when this issue was logged (but also broke stacking)

On Sat, 12 Nov 2022, 1:00 pm Stan, @.***> wrote:

What?

— Reply to this email directly, view it on GitHub https://github.com/qbcore-framework/qb-inventory/issues/377#issuecomment-1312274333, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGHUHJJDNHZRD4XZWBKT53WH3MZ7ANCNFSM6AAAAAAR3EXLX4 . You are receiving this because you were mentioned.Message ID: @.***>

nzkfc commented 1 year ago

@nzkfc @mikepauer Please test #378

Pulled another fresh build just in case.

Testing:

Dragged items onto each other, both the wrong way and correct way, can confirm correct way now works, makes the "Combine" and "Switch" buttons appear in middle of inventory and you can now craft (combine) the items.
Tested crafting table just to double check, all ok
Re-tested the original fix for items going missing, all working as expected. Did an extra test of filling a boot with pistols and trying to move more in or dragging more in, it correctly detects it's full andd stops you from doing so.

Looks like this is good to go, again thanks for the solid fixes here @tom-osborne

qbcore-framework / qb-inventory