-
Notifications
You must be signed in to change notification settings - Fork 149
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
update CBA_fnc_uniqueUnitItems #1157
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be cheaper to do it at the end, after duplicate removal.
because
-
iterates over everything and copies everything.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or if we know where it comes from (I guess primary/secondary/handgunWeapon) we can do it right there.
Would be cheaper to do in L58 for just the 3 things, than do it for everything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It comes from
p/s/hWeapon
as well asp/s/hWeaponItems
.The magazine commands report empty arrays if no mag is loaded. I put the array subtraction below the last line that may add empty strings.CBA_fnc_binocularMagazine
reports empty string too.The placement is so that the array subtraction command is only executed when the
_weaponItems
flag is set, as this is the only place where the commands may add empty strings. Note that whilep/s/hWeaponItems
has the items in order muzzle/pointer/optics/bipod and fills the gaps with empty strings, assignedItems, while having an order, just changes in array size (and indices) if a specific slot is unoccupied.If I were to add it to the last line after the
arrayIntersect
command instead, I would slow down the function for every use even when this flag is set to false. I know that a smaller array would speed up the function slightly, but this would mean adding an additional binary command (-
) for all uses for a small improvement in C++ that itself wouldn't even be a problem in all other use cases. The priority is clear to me.The only potential optimization here would be moving the weapon(items/magazines) block above the uniform/vest/backpack block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another potential optimization approach would be creating the initial array as
[""]
and then usingpushBackUnique
everywhere. And then usingdeleteAt 0
at the end. However, this would needappendUnique
, because one source of empty strings are arrays here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe instead store _weaponItems seperately, then remove "" from it.
And only then append it to _allItems. That way you don't have to iterate over uniform/vest/backpack items to remove ""
Nah not with the number of appends we have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make a new array here
https://github.com/CBATeam/CBA_A3/pull/1157/files#diff-5281788de7c6251ec13f252782a5e72fR54
And then
_allItems append _newArray
after removing the ""There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would mean more commands in total. I don't think there will be any case where this actually would be faster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but only in the weaponsItems case
which I'd say is rare.