Skip to content

Update mob and item list functionality #351

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

Merged
merged 5 commits into from
Apr 28, 2025

Conversation

Jasrags
Copy link
Contributor

@Jasrags Jasrags commented Apr 27, 2025

Description

This Pull Request introduces a new dynamic list rendering system and applies it to the mob and item listing commands. It also adds safe accessor methods for screen dimensions and improves handling when no items are found.

NOTE: This PR could be followed up with changes to improve DynamicList.

Changes

  • Added getters for screen width and height that provide sensible default values when the client hasn’t reported its dimensions, and cast from uint32 to int for downstream compatibility.
  • Added templates/layout.go containing the shared DynamicList function, as part of a larger refactor to centralize list rendering logic.
  • Changed both the mob list and item list commands to use DynamicList for output generation, and ensured case-insensitive sorting by lowercasing names before comparison.
  • Added a user-friendly message when an item search returns no results.
  • Added unit tests for DynamicList
Screenshot 2025-04-27 at 2 09 52 PM

Jasrags added 4 commits April 27, 2025 11:50
…if no screen information has been received and cast the native uint32 to int as that is the downstream type that is used.
… as well as fixing sort by tolowering the names
@Jasrags Jasrags added the enhancement New feature or request label Apr 27, 2025
@Jasrags Jasrags self-assigned this Apr 27, 2025
@Jasrags Jasrags requested a review from Volte6 as a code owner April 27, 2025 19:12
@Jasrags Jasrags linked an issue Apr 27, 2025 that may be closed by this pull request
@Jasrags Jasrags requested review from Copilot and removed request for Volte6 April 27, 2025 19:12
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the mob and item list functionalities by centralizing list rendering logic via a new DynamicList function and improving safe access to screen dimensions.

  • Introduced a DynamicList function in templates/layout.go for consistent formatted output.
  • Updated admin.mob.go and admin.item.go to use DynamicList and case-insensitive sorting.
  • Added safe getter methods for screen dimensions in clientsettings.go.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
internal/usercommands/admin.mob.go Refactored mob list command to use DynamicList and safe screen dimension access.
internal/usercommands/admin.item.go Refactored item list command to use DynamicList and improved search handling.
internal/templates/layout.go Added DynamicList implementation to centralize dynamic list rendering.
internal/connections/clientsettings.go Added default getters for screen width and height for client settings.
Comments suppressed due to low confidence (1)

internal/usercommands/admin.item.go:75

  • [nitpick] Consider renaming 'itmNames' to 'itemList' for clearer consistency with the naming used in the mob list command.
itmNames := []templates.NameDescription{}

@Jasrags Jasrags requested a review from Copilot April 27, 2025 19:22
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements a dynamic list-rendering system for mob and item listing commands and adds safe accessor methods for screen dimensions, along with improved user messaging when no items are found.

  • Refactored mob and item list commands to use the new DynamicList function for uniform output formatting.
  • Introduced getters for screen width and height with default values in the client settings.
  • Added unit tests to verify the behavior of the DynamicList function.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
internal/usercommands/admin.mob.go Refactored mob list command to use dynamic list rendering and case-insensitive sorting.
internal/usercommands/admin.item.go Updated item list command with dynamic list rendering and adjusted filtering behavior.
internal/templates/layout_test.go Added tests for the DynamicList function to verify its output handling.
internal/templates/layout.go Implemented the DynamicList function to generate formatted, wrapped list output.
internal/connections/clientsettings.go Introduced safe accessor methods with sensible defaults for screen dimensions.

@Jasrags Jasrags requested a review from Volte6 April 27, 2025 19:23
@Jasrags
Copy link
Contributor Author

Jasrags commented Apr 27, 2025

I do realize that the item and mob list functions could be refactored as well as they are doing the same thing but I did not include that in this PR

@Jasrags Jasrags merged commit 06317f0 into master Apr 28, 2025
1 check passed
@Jasrags Jasrags deleted the 332-list-mobs-by-vnum-in-mob-list-command branch April 28, 2025 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

List Mobs by VNUM in mob list Command
2 participants