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

Add a test to check that lathe recipes are possible #33868

Merged

Conversation

Tayrtahn
Copy link
Member

@Tayrtahn Tayrtahn commented Dec 15, 2024

About the PR

Adds an integration test that makes sure that lathes are able to accept the materials needed by all of their recipes.

Why / Balance

To avoid issues like #32206 coming up in the future.

Technical details

Note that this is going to fail on master currently! As reported in #32206, reinforced glass is listed in the autolathe, but the recipe calls for materials that can't be added to an autolathe. Until that is fixed, this test will fail. Edit: see #33876

The test works by checking each lathe and building a hashset of the materials it accepts (found by checking every material-containing entity against the lathe's storage whitelist). The recipes are then checked to make sure that they don't call for any materials that aren't accepted.

There's also a check to make sure that recipes don't call for a material quantity greater than the capacity of the lathe. No lathes currently have a storage limit, but it's a simple enough check that it doesn't hurt to add it.

Media

Requirements

Breaking changes

Changelog

Not player-facing, so nah.

@github-actions github-actions bot added S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. size/M Denotes a PR that changes 100-999 lines. labels Dec 15, 2024
@Simyon264 Simyon264 added P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. T: New Feature Type: New feature or content, or extending existing content D2: Medium Difficulty: A good amount of codebase knowledge required. S: Needs Review Status: Requires additional reviews before being fully accepted A: Integration Tests Area: Integration tests, adding or fixing them 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 Dec 15, 2024
@Tayrtahn Tayrtahn added the S: Needs Content PR Merged Status: Requires an existing SS14 PR to be merged first. label Dec 15, 2024
Copy link
Member

@slarticodefast slarticodefast left a comment

Choose a reason for hiding this comment

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

Could you merge master so we can check if the test is successful now that the recipe is fixed?
Also the AllLatheRecipesValidTest should probably be moved from ResearchTest.cs to the new Lathetest.cs for better organisation.

Content.IntegrationTests/Tests/Lathe/LatheTest.cs Outdated Show resolved Hide resolved
@Tayrtahn Tayrtahn removed the S: Needs Content PR Merged Status: Requires an existing SS14 PR to be merged first. label Dec 15, 2024
@slarticodefast slarticodefast self-assigned this Dec 15, 2024
@slarticodefast slarticodefast 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 labels Dec 15, 2024
@github-actions github-actions bot added size/L Denotes a PR that changes 1000-4999 lines. and removed size/M Denotes a PR that changes 100-999 lines. labels Dec 16, 2024
@github-actions github-actions bot added S: Needs Review Status: Requires additional reviews before being fully accepted and removed S: Awaiting Changes Status: Changes are required before another review can happen labels Dec 16, 2024
@southbridge-fur
Copy link
Contributor

Frontier has been looking at limiting lathe storage, so the bit that checks against storage limits could be helpful.

Copy link
Member

@slarticodefast slarticodefast left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Very useful PR, thanks!

@slarticodefast slarticodefast merged commit cf73885 into space-wizards:master Dec 16, 2024
13 checks passed
@Tayrtahn Tayrtahn deleted the lathe-recipe-material-test branch December 21, 2024 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: General Interactions Area: General in-game interactions that don't relate to another area. A: Integration Tests Area: Integration tests, adding or fixing them D2: Medium Difficulty: A good amount of codebase knowledge required. P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. S: Needs Review Status: Requires additional reviews before being fully accepted size/L Denotes a PR that changes 1000-4999 lines. T: New Feature Type: New feature or content, or extending existing content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants