Skip to content

fix species name in station records #36217

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Oct 21, 2025
Merged

fix species name in station records #36217

merged 4 commits into from
Oct 21, 2025

Conversation

@lzk228
Copy link
Contributor

@lzk228 lzk228 commented Mar 30, 2025

About the PR

it literally takes the species id for station records instead of the name

Why / Balance

bug

Technical details

index prototype and send its name, instead of the prototype itself

Media

no

Requirements

Breaking changes

no

Changelog

no

@github-actions github-actions bot added size/XS Denotes a PR that changes 0-9 lines. S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Mar 30, 2025
beck-thompson
beck-thompson requested changes Apr 1, 2025
View reviewed changes
Tayrtahn
Tayrtahn reviewed Apr 1, 2025
View reviewed changes
Copy link
Member

@Tayrtahn Tayrtahn left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think it would be better to fix this on the display side, rather than the record creation side.

That is, continue using the species' protoId to create the station record, and instead do the prototype lookup when displaying the record, right before the Loc.GetString call (here)

@Killerqu00 Killerqu00 added P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. T: Cleanup Type: Code clean-up, without being a full refactor or feature D3: Low Difficulty: Some codebase knowledge required. S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. A: General Interactions Area: General in-game interactions that don't relate to another area. and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Apr 11, 2025
@MilonPL MilonPL added S: Awaiting Changes Status: Changes are required before another review can happen S: Stale Status: Stale with no activity, and may be closed after a week if there is no new activity. and removed S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. labels Apr 20, 2025
@MilonPL
Copy link
Contributor

MilonPL commented Apr 20, 2025

Is this still being worked on?

@lzk228
Copy link
Contributor Author

lzk228 commented Apr 20, 2025

yes

@github-actions github-actions bot added size/S Denotes a PR that changes 10-99 lines. Changes: UI Changes: Might require knowledge of UI design or code. and removed size/XS Denotes a PR that changes 0-9 lines. labels Apr 21, 2025
@lzk228 lzk228 requested a review from Tayrtahn April 21, 2025 19:12
@github-actions github-actions bot added S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. and removed S: Awaiting Changes Status: Changes are required before another review can happen labels Apr 21, 2025
@lzk228 lzk228 requested a review from beck-thompson April 21, 2025 19:12
beck-thompson
beck-thompson previously requested changes Apr 24, 2025
View reviewed changes
Copy link
Member

@beck-thompson beck-thompson left a comment

Choose a reason for hiding this comment

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

It seems to be mad and crashes the client

image

@beck-thompson beck-thompson added S: Awaiting Changes Status: Changes are required before another review can happen and removed S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. labels Apr 24, 2025
@beck-thompson beck-thompson self-assigned this Apr 24, 2025
@lzk228 lzk228 requested a review from beck-thompson May 9, 2025 10:53
@github-actions github-actions bot added S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. and removed S: Awaiting Changes Status: Changes are required before another review can happen labels May 9, 2025
@slarticodefast
Copy link
Member

I pushed some fixes:

  • The dependency injection was missing, causing an error.
  • You were indexing the SpeciesPrototype as an EntityPrototypes, causing another error.
  • You were displaying a loc string without resolving it.

In the future please make sure to test your changes in-game before requesting a review.

@slarticodefast slarticodefast added this pull request to the merge queue Oct 21, 2025
@PJBot PJBot added the S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. label Oct 21, 2025
Merged via the queue into space-wizards:master with commit 3bbc1e1 Oct 21, 2025
10 checks passed
@lzk228 lzk228 deleted the 03-30-fix-species branch October 21, 2025 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slarticodefast slarticodefast slarticodefast approved these changes

@Tayrtahn Tayrtahn Awaiting requested review from Tayrtahn

@beck-thompson beck-thompson Awaiting requested review from beck-thompson

+1 more reviewer

@Aeshus Aeshus Aeshus left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@beck-thompson beck-thompson

Labels

A: General Interactions Area: General in-game interactions that don't relate to another area. Changes: UI Changes: Might require knowledge of UI design or code. D3: Low Difficulty: Some codebase knowledge required. P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. S: Stale Status: Stale with no activity, and may be closed after a week if there is no new activity. size/S Denotes a PR that changes 10-99 lines. T: Cleanup Type: Code clean-up, without being a full refactor or feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@lzk228 @MilonPL @slarticodefast @Tayrtahn @Aeshus @beck-thompson @PJBot @Killerqu00