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

1L Silver Ingot should have "material" property of silver instead of lead #73263

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

Kefpull
Copy link
Contributor

@Kefpull Kefpull commented Apr 25, 2024

Summary

Bugfixes "1L Silver Ingot should have "material" property of silver instead of lead"

Purpose of change

I was stockpiling silver in game and noticed that the tier 1 silver ingot had the wrong material property. While it's unlikely people would encounter this outside of maximum item spawn rate, it's still not right. (Unless it is, then a note should be added)

Reproduction steps:
1: spawn in or craft a 1L size silver ingot.

Describe the solution

data/json/items/resources/metal.json : change material property of 1l_silver from
"material": [ "lead" ], to
"material": [ "silver" ],

Karol1223's metals audit ( #64568 ) fixed a lot of the metal prices, but some of the properties were still broken.

Drew4484's metal ingots pull ( #56598 ) fixed all of the metal ingot materials except this one for some reason.

Describe alternatives you've considered

If this is intended behavior, there should be a note to explain why it is different from every other metal ingot.

Testing

Spawning the item using the debug menu now shows "silver" as the material.

Additional context

Blaming Cataclysm-DDA_data_json_items_resources_metal json at master · CleverRaven_Cataclysm-DDA - Personal - Microsoft​ Edge 4_25_2024 10_58_08 AM

…lead

Was stockpiling silver in game and noticed that the tier 1 silver ingot had the wrong material property. While it's unlikely people would encounter this outside of maximum item spawns, it's still not right. (Unless it is, then a note should be added)
Copy link
Contributor

You are creating a pull request with the master branch as the head branch. This is likely a mistake unless you really know what you are doing. You may read https://docs.github.com/en/get-started/quickstart/contributing-to-projects#creating-a-branch-to-work-on for a typical workflow of contributing to a project on GitHub.

@github-actions github-actions bot added [JSON] Changes (can be) made in JSON <Bugfix> This is a fix for a bug (or closes open issue) new contributor astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Apr 25, 2024
@Karol1223
Copy link
Contributor

Drew4484's metal ingots pull ( #56598 ) fixed all of the metal ingot materials except this one for some reason.

Because I only added 1l silver a year later in #64568. It's my bad. Cargo culting is a pain.

@Maleclypse Maleclypse merged commit 49ac595 into CleverRaven:master Apr 26, 2024
21 of 26 checks passed
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions new contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants