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

Start powering the spreadsheet from organizer columns #10680

Merged
merged 10 commits into from
Aug 12, 2024

Conversation

bhollis
Copy link
Contributor

@bhollis bhollis commented Aug 9, 2024

This is maybe overkill, but it's something I've wanted to do for a long time. With this change, the inventory spreadsheets are entirely driven by the column definitions from the Organizer. This wasn't quite as smooth as I had imagined, because I wanted to preserve the output of the spreadsheets as they were, which required a bit more custom stuff than I had wanted. For example, every column needs a custom non-localized name, and the order of columns needs to be handled in a separate list. However, my goal was to make it so you can't accidentally add a column to Organizer without it appearing in Spreadsheets and vice versa, and that goal was achieved.

Some actual changes:

  1. The CSVs are sorted by item name now, instead of whatever order from inventory.
  2. Ghosts have extra columns - year/season and energy.
  3. Added a "Foundry" column to the Organizer.
  4. Removed "Guard Efficiency" stat which was deprecated.
  5. The output file names are slightly different (destiny-weapon.csv vs destinyWeapons.csv)
  6. A few other minor things I'll remember as I re-review the code.

Comment on lines +330 to +346
isSpreadsheet &&
c({
id: 'hash',
header: 'Hash',
csv: 'Hash',
value: (i) => i.hash,
}),
isSpreadsheet &&
c({
id: 'id',
header: 'Id',
csv: 'Id',
value: (i) => `"${i.id}"`,
}),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to add a few spreadsheet-only columns.

@@ -588,9 +706,11 @@ export function getColumns(
header: t('Organizer.Columns.Level'),
value: (item) => item.craftedInfo?.level,
defaultSort: SortDirection.DESC,
csvVal: (value) => ['Crafted Level', value ?? 0],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general I want columns to return undefined for their value if the column doesn't apply. But then annoyingly the CSV was inconsistent about whether a column was undefined or zeroed.

* This builds stat infos for all the stats that are relevant to a particular category of items.
* It will return the same result for the same category, since all items in a category share stats.
*/
export function buildStatInfo(items: DimItem[]): {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved from ItemTable.tsx

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

Successfully merging this pull request may close these issues.

2 participants