Skip to content

Conversation

@itsschwer
Copy link

Changes

  • Split the binding for each component's configs into separate functions
  • Only generate configs for enabled components
    • Avoids cluttering users' config folder with unnecessary config files
    • Encourages that each component is self-contained
  • Remove config StatsDisplayEnable and replace with ComponentsStatsDisplay (previously effectively useless)
    • Makes StatsDisplay component more consistent with other components — see below for further suggestions
  • Check config ComponentsItemSorting is enabled before attempting to sort Command/Scrapper menus

Potential future changes

I wasn't sure if I should work on these areas, if you had existing plans or suggestions, or if I should open these as issues.

  • StatsDisplay.Hook() is empty and inconsistent with other components (hooking currently occurs in constructor?)
    • Should this be addressed?
  • Review coupling between CommandImprovements and ItemSorting
    • May be a good idea to move the sorting logic from CommandImprovements.cs to ItemSorting.cs, using another PickupPickerPanel_SetPickupOptions hook
  • Fix typos in config variable names
    • Misc: NotificiationsNotifications
    • Misc, AdvancedIcons: EquipementEquipment

Avoid cluttering users' config folder
To further improve consistency with other components, hooking functions should be moved from the constructor to the currently empty `StatsDisplay.Hook()`
Sorting logic should probably be moved to ItemSorting.cs
@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2024

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@itsschwer
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

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.

1 participant