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

Item Rename Framework #1329

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Item Rename Framework #1329

wants to merge 10 commits into from

Conversation

BrettMayson
Copy link
Contributor

@BrettMayson BrettMayson commented Apr 27, 2020

When merged this pull request will:

  • Add a way to rename items during the mission
  • Intended for unique items with IDs, but not required (Dogtags, TFAR, ACRE, etc)

Before
image

After
image

Known Issues:

Like the Item Context Menu, this does not work on corpses.

@BrettMayson BrettMayson marked this pull request as ready for review April 27, 2020 08:57
@PabstMirror

This comment has been minimized.

commy2
commy2 previously requested changes Apr 27, 2020
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.

@commy2

This comment has been minimized.

@BrettMayson

This comment has been minimized.

@commy2

This comment has been minimized.

@BrettMayson

This comment has been minimized.

@commy2

This comment has been minimized.

@commy2

This comment has been minimized.

@BrettMayson

This comment has been minimized.

@PabstMirror

This comment has been minimized.

@BrettMayson

This comment has been minimized.

@BrettMayson

This comment has been minimized.

@commy2

This comment has been minimized.

@commy2 commy2 added the WIP label May 14, 2020
@BrettMayson

This comment has been minimized.

…ntory;

Rename addRenamedItem function to renameInventoryItem
Add params with typecheck, hasInterface & empty class check & header comment for public use in renameInventoryItem function
Make functions & namespace local
Rename getInventoryClassname function to getInventoryItemData

Tweak initDisplayInventory function to:
  - Make Rename Item Framework work if Item Context Framework options are not present
  - Not not run if no ItemContextMenuOptions and renamedItems are present
  - Not not register Item Context Framework event handlers if ItemContextMenuOptions are not present
  - Not register Item Rename framework handlers if renamedItems are not present
  - Change exitWith to normal if for checking ctrlShown becouse exitWith will exit whole loop before checking rest of controls
@SzwedzikPL

This comment has been minimized.

@commy2

This comment has been minimized.

@SzwedzikPL
Copy link
Contributor

SzwedzikPL commented May 26, 2020

@commy2 Done. I tried earlier my luck with changing the tooltip but so far i've been unable to find a way to do that. It looks like it comes directly from the engine.

I've been also thinking about supporting nil as a value in CBA_fnc_renameInventoryItem so "" can be used to remove item name or picture. What do you think?

@commy2
Copy link
Contributor

commy2 commented May 27, 2020

Yeah, couldn't get the tooltips to work either. Probably some hard coded stuff managing the inventory ui tooltips that prevents scripted tooltips from working, sadly. Could always try to create custom tooltip controls though.

nil already converts to "" due to params command. Since empty stings make no sense (and for picture you could use the empty_ca.paa) it should treat "" as "revert to default".

@SzwedzikPL
Copy link
Contributor

Ok, for now i think vanilla tooltips are fine, we can always add some workaround for tooltips later.

Any other suggestions?
Also, is there a chance for this to release in 3.15.2 ?

@commy2
Copy link
Contributor

commy2 commented May 27, 2020

There're no plans for anything lol

@commy2 commy2 dismissed their stale review May 29, 2020 15:13

outdated

@commy2
Copy link
Contributor

commy2 commented May 29, 2020

Is this done?

@BrettMayson
Copy link
Contributor Author

Afaik, yes.

The only outstanding issue is the bug with corpses, the same issue as the item context menu framework.

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.

Errors when item in backpack on ground is renamed.

https://i.imgur.com/57Dt8Ck.png

(The error means _classname is undefined.)

Script:

["FirstAidKit", "TEST", "\a3\3DEN\Data\Controls\ctrlDefault\arrowEmpty_ca.paa"] call cba_fnc_renameInventoryItem;

@mharis001
Copy link
Contributor

What about allowing the text color to be changed as well.

@SzwedzikPL
Copy link
Contributor

SzwedzikPL commented May 29, 2020

@commy2 do you have any idea how to properly support items inside backpacks/containers on the ground? InventoryOpened EH doesn't work in such case, nor ContainerOpened. I found some ugly way using loops and everyContainer but it won't work if there's more than one - don't know which one is opened.

@commy2
Copy link
Contributor

commy2 commented May 30, 2020

Why does InventoryOpened not work?

@commy2
Copy link
Contributor

commy2 commented May 30, 2020

The correct event name is "ContainerOpened". You can get which one is opened by tracking double click on the container list.

Copy link
Contributor

@johnb432 johnb432 left a comment

Choose a reason for hiding this comment

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

@BrettMayson Can you resolve the merge conflicts?

  • You can't rename backpacks atm. It might be possible to add it without game engine changes, but I'm not certain.
  • Dogtags - Rename inventory items acemod/ACE3#10102 (comment)
    Which method is better - PFH, mouse EH or map EH? We should try to decide on a single method and stick with that imo, to make it more uniform.

};
};

// Reports classname, but only for magazines.
Copy link
Contributor

Choose a reason for hiding this comment

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

When I redid the inventory filtering in acemod/ACE3#9706, I found all item classnames are contained in lbData except for backpacks:
https://github.com/acemod/ACE3/blob/b714c8bce2628ff8d4795da8a7f2fc05bb82f474/addons/inventory/functions/fnc_forceItemListUpdate.sqf#L33

Workaround for backpacks is to get them using the display name and picture.

Copy link
Contributor

Choose a reason for hiding this comment

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

Workaround for backpacks is to get them using the display name and picture.

That would likely cause ambiguities for backpacks with linked items, as I doubt they would change the picture and display name for each variant.

addons/ui/fnc_renameInventoryItem.sqf Outdated Show resolved Hide resolved
addons/ui/fnc_initDisplayInventory.sqf Outdated Show resolved Hide resolved
PabstMirror and others added 2 commits July 5, 2024 00:44
Co-Authored-By: johnb432 <58661205+johnb432@users.noreply.github.com>
@johnb432
Copy link
Contributor

It might be possible to add it without game engine changes, but I'm not certain.

Not possible atm, made a ticket: https://feedback.bistudio.com/T183096

@PabstMirror PabstMirror added this to the 3.18.0 milestone Jul 28, 2024
@PabstMirror PabstMirror removed the WIP label Aug 8, 2024
@PabstMirror
Copy link
Contributor

I'd like to merge now even if backpacks aren't working,
thoughts?

@johnb432
Copy link
Contributor

johnb432 commented Aug 9, 2024

It might be possible to just add backpackCargo to https://github.com/CBATeam/CBA_A3/pull/1329/files#diff-32e5060e1f3d15868d5a7eb7582a521ea990945ef5d516056f0c38bf16799fa5R49-R56 and have it work like that. If so, it would be a more robust solution than what ACE atm.
EDIT: It works, but I'm not sure how robust this solution is fundamentally. Can DLC or other atypical items pose issues?

@johnb432
Copy link
Contributor

Can DLC or other atypical items pose issues?

Yes, atypical items can pose issues. Repro: I loaded ACE, CUP Weapons and RHS and put some mines from each mod into my backpack in the ACE arsenal (I had 13 items total in backpack, 1 base game training mine and 12 from CUP/RHS, I took 5 mines from 1 category, rest were 1 each). Restarted the game without CUP Weapons and RHS and went into the ACE arsenal. There, I still had 13 items, but 12 of them were "". In the backpack, 8 rows were filled with the "Premium item" thing that you get when you have non-existing item.
I added a backpack, renamed to "test" and added the backpack in my backpack. When I opened my backpack, the 3rd row was renamed from "" to "test".

_container = backpackContainer player;
weaponCargo _container + itemCargo _container + magazineCargo _container + backpackCargo _container;

returns ["TrainingMine_Mag","","","","","","","","","","","","","B_AssaultPack_rgr"], so it makes sense.

Any item renamed through the framework is affected by this: I tested it by trying to rename the training mine I had - this time it was the top most (again, makes sense).

There might be a way around this very specific issue though: getMagazineCargo is able to tell how many of each invalid classes there are. I'll experiment with the getXCargo commands to see if I can get it to work (would avoid us having to arrayIntersect the result).

@johnb432
Copy link
Contributor

johnb432 commented Aug 12, 2024

The getXCargo commands resolve the issue.

However, there are still 2 issues:

  1. I renamed the training mag to "test 2". Because _control lbData _index returns "", it also renames the first invalid item to "test 2". It's not a big issue, but still.
  2. More importantly: if you have 2 weapons of the same class, but they don't have the same attachments, then they won't stack in the inventory, but they are reported as the same class (getWeaponCargo returns 2, weaponCargo returns it twice). This creates a bad offset and renames the wrong backpack.

I don't think using inventory commands to determine the order of the UI is the way forward. Imo we should not support backpacks atm, remove the empty classname stuff and hope the BI ticket will be resolved.

addons/ui/fnc_getInventoryItemData.sqf Outdated Show resolved Hide resolved
addons/ui/fnc_renameInventoryItem.sqf Show resolved Hide resolved
Co-authored-by: johnb432 <58661205+johnb432@users.noreply.github.com>
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.

7 participants