Skip to content

Make /give and /item not give more items than needed#3287

Merged
QuiCM merged 1 commit intoPryaxis:general-develfrom
JustULTA:give-item
Apr 17, 2026
Merged

Make /give and /item not give more items than needed#3287
QuiCM merged 1 commit intoPryaxis:general-develfrom
JustULTA:give-item

Conversation

@JustULTA
Copy link
Copy Markdown
Contributor

This makes previously unstackable items still being given in stacks of 1 if amount is not specified (all other items are still given in max amounts by default). Also makes it consistent with vanilla's journey mode item duplication.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 17, 2026

Greptile Summary

This PR refines the default item quantity behavior in /item and /give by splitting the previously combined itemAmount == 0 || itemAmount > maxStack guard into two separate cases: over-max clamping and a new default-zero branch that uses Item.OnlyNeedOneInInventory() to decide between giving 1 or maxStack. The change aligns TShock's admin item-giving commands with Terraria's Journey Mode duplication behavior for utility/unique items.

Confidence Score: 5/5

Safe to merge — the change is minimal, symmetric across both commands, and all edge cases (maxStack == 1 weapons/armor, explicit 0 input) produce results identical to the previous code.

No P0 or P1 findings. The two-branch refactor is logically equivalent to the old code for all cases where OnlyNeedOneInInventory() returns false or maxStack == 1, and correctly limits the default quantity for utility/unique items (OnlyNeedOneInInventory() == true, maxStack > 1) to 1, matching Journey Mode behavior.

No files require special attention.

Important Files Changed

Filename Overview
TShockAPI/Commands.cs Two symmetric changes to /item (line 6312) and /give (line 6456): the old combined guard is replaced with a correct two-branch pattern that calls OnlyNeedOneInInventory() only when no amount was specified; logic is sound and edge cases (explicit 0, maxStack == 1 weapons) produce identical results to the prior code.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["/item or /give called"] --> B{itemAmount\nspecified?}
    B -- "No (itemAmount == 0)" --> C{itemAmount > maxStack?}
    B -- "Yes (itemAmount > 0)" --> C
    C -- "Yes" --> D[itemAmount = maxStack]
    C -- "No, and itemAmount == 0" --> E{item.OnlyNeedOneInInventory?}
    C -- "No, and itemAmount > 0" --> F[Keep user-specified amount]
    E -- "true" --> G[itemAmount = 1]
    E -- "false" --> H[itemAmount = maxStack]
    D --> I[GiveItemCheck]
    F --> I
    G --> I
    H --> I
Loading

Reviews (1): Last reviewed commit: "Make /give and /item not give more items..." | Re-trigger Greptile

Copy link
Copy Markdown
Member

@ACaiCat ACaiCat 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

@QuiCM QuiCM merged commit b41bd4b into Pryaxis:general-devel Apr 17, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants