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

Core: no longer log ID ranges on generate #4013

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Berserker66
Copy link
Member

@Berserker66 Berserker66 commented Sep 29, 2024

What is this fixing or adding?

Once ID overlap is fine in clients, we don't need to spend processing on calculating and displaying this information

How was this tested?

with a local gen

If this makes graphical changes, please attach screenshots.

image

@github-actions github-actions bot added affects: core Issues/PRs that touch core and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Sep 29, 2024

del item_digits, location_digits, item_count, location_count
logger.info(f" {name:{longest_name}}: Items: {len(cls.item_names):{item_count}} "
f"Locations: {len(cls.location_names):{location_count}} ")
Copy link
Member

Choose a reason for hiding this comment

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

Do you happen to know what this space is for / if it can be removed?

Suggested change
f"Locations: {len(cls.location_names):{location_count}} ")
f"Locations: {len(cls.location_names):{location_count}}")

f"{max(cls.location_id_to_name):{location_digits}})")

del item_digits, location_digits, item_count, location_count
logger.info(f" {name:{longest_name}}: Items: {len(cls.item_names):{item_count}} "
Copy link
Member

Choose a reason for hiding this comment

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

I think the | delimiter between the Items and Locations was helpful here

Suggested change
logger.info(f" {name:{longest_name}}: Items: {len(cls.item_names):{item_count}} "
logger.info(f" {name:{longest_name}}: Items: {len(cls.item_names):{item_count}} | "

@Exempt-Medic Exempt-Medic added the is: enhancement Issues requesting new features or pull requests implementing new features. label Sep 30, 2024
@Exempt-Medic Exempt-Medic added the waiting-on: author Issue/PR is waiting for feedback or changes from its author. label Oct 13, 2024
Copy link
Contributor

@nicholassaylor nicholassaylor left a comment

Choose a reason for hiding this comment

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

I agree with Medic's reviews and would be willing to approve once they are addressed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: author Issue/PR is waiting for feedback or changes from its author. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants