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

Update create-item.lua #1277

Closed
wants to merge 4 commits into from
Closed

Update create-item.lua #1277

wants to merge 4 commits into from

Conversation

Tjudge1
Copy link
Contributor

@Tjudge1 Tjudge1 commented Aug 28, 2024

Fixed issues with spawning creature based items. Vermin, pets, eggs, fish, raw fish, and remains now spawn and stack correctly.

Fixes: DFHack/dfhack#4689
Fixes: DFHack/dfhack#4507
Fixes: DFHack/dfhack#3674

Fixed issues with spawning creature based items. Vermin, pets, eggs, fish, raw fish, and remains now spawn and stack correctly.
@Tjudge1
Copy link
Contributor Author

Tjudge1 commented Aug 28, 2024

pre-commit.ci autofix

@myk002
Copy link
Member

myk002 commented Aug 28, 2024

Could you edit the PR description to include lines like:

Fixes: DFHack/dfhack#4689

for the issues that this PR should fix? That will link this PR to issues and auto-close them when this PR is merged.

Copy link
Member

@myk002 myk002 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 add a line (or lines) to changelog.txt explaining (from a player's perspective) what has been fixed?

modtools/create-item.lua Outdated Show resolved Hide resolved
Comment on lines +339 to +344
if not typesThatUseCreaturesExceptCorpses[df.item_type[itemtype]] and df.item_type.attrs[itemtype].is_stackable then
return createItem({mattype, matindex}, {itemtype, itemsubtype}, quality, unit, description, count)
end
if typesThatUseCreaturesExceptCorpses[df.item_type[itemtype]] and df.item_type.attrs[itemtype].is_stackable then
return createItem({matindex, casteId}, {itemtype, itemsubtype}, quality, unit, description, count)
end
Copy link
Member

Choose a reason for hiding this comment

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

There is too much duplication here. How about this?

    if df.item_type.attrs[itemtype].is_stackable then
        local mat = typesThatUseCreaturesExceptCorpses[df.item_type[itemtype]] and {matindex, casteId} or {mattype, matindex}
        return createItem(mat, {itemtype, itemsubtype}, quality, unit, description, count)
    end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait, did you test this?

Copy link
Member

Choose a reason for hiding this comment

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

not tested, just written as a suggestion for code improvement. If you have two chunks of code that are semantically and structurally identical except for a small difference, it is usually better to factor that difference out and keep the structure simpler.

local items = {}
for _ = 1,count do
if itemtype == df.item_type.CORPSEPIECE or itemtype == df.item_type.CORPSE then
table.insert(items, createCorpsePiece(unit, bodypart, partlayerID, matindex, casteId, corpsepieceGeneric))
else
for _,item in ipairs(createItem({mattype, matindex}, {itemtype, itemsubtype}, quality, unit, description, 1)) do
table.insert(items, item)
if typesThatUseCreaturesExceptCorpses[df.item_type[itemtype]] then
Copy link
Member

Choose a reason for hiding this comment

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

you can do the same condensing here. Factor out the value of mat and avoid duplicating the for loop

Copy link
Contributor Author

@Tjudge1 Tjudge1 left a comment

Choose a reason for hiding this comment

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

Sure thing.

Removed editing comment
Added a description of changes made to 'modtools/create-item', which in turn fixes the issues with 'gui/create-item'.
Comment on lines +20 to +23
- `gui/create-item`: fix items of type "VERMIN", "PET", "REMANS", "FISH", "RAW FISH", and "EGG" no longer spawn creature item "nothing."
Items will now spawn correctly, and will be of the creature type and creature caste that selected by the user. Items of these types will also stack correctly when needed.
- `modtools/create-item`: fix items of type "VERMIN", "PET", "REMANS", "FISH", "RAW FISH", and "EGG" no longer spawn creature item "nothing"s.
Items will now spawn correctly, and will be of the creature type and creature caste that selected by the user. Items of these types will also stack correctly when needed.
Copy link
Member

Choose a reason for hiding this comment

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

this is the "Template for new versions" section -- the actual changelog lines should go in the "Future" version section below

@@ -17,6 +17,10 @@ Template for new versions:
## New Features

## Fixes
- `gui/create-item`: fix items of type "VERMIN", "PET", "REMANS", "FISH", "RAW FISH", and "EGG" no longer spawn creature item "nothing."
Copy link
Member

Choose a reason for hiding this comment

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

REMANS -> REMAINS

"no longer spawn creature item "nothing."" -> "being created without a valid creature type"

Comment on lines +339 to +344
if not typesThatUseCreaturesExceptCorpses[df.item_type[itemtype]] and df.item_type.attrs[itemtype].is_stackable then
return createItem({mattype, matindex}, {itemtype, itemsubtype}, quality, unit, description, count)
end
if typesThatUseCreaturesExceptCorpses[df.item_type[itemtype]] and df.item_type.attrs[itemtype].is_stackable then
return createItem({matindex, casteId}, {itemtype, itemsubtype}, quality, unit, description, count)
end
Copy link
Member

Choose a reason for hiding this comment

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

not tested, just written as a suggestion for code improvement. If you have two chunks of code that are semantically and structurally identical except for a small difference, it is usually better to factor that difference out and keep the structure simpler.

@Tjudge1 Tjudge1 closed this Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
2 participants