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

Allow auto-equipping items when the inventory item frees space for the equipped item #7496

Merged
merged 4 commits into from
Oct 21, 2024

Conversation

ephphatha
Copy link
Contributor

@ephphatha ephphatha commented Oct 20, 2024

New behaviour:
https://github.com/user-attachments/assets/3b7ab495-59f9-4315-a986-18a8b876c40d

Main thing of note is the item that used to be equipped will fill the gap left by the item being equipped if it fits.

Commit 2324108 is some prep that made it easier to reason about the steps required, I tried to keep the same behaviour/sound feedback. What I was attempting to show at about 10 seconds into this clip was trying to equip the short sword when there's no room to remove the two-handed sword would play the "I have no room" speech but the screen recording didn't capture sound :(

This builds on #7494, I've split them to hopefully make things easier to review.

@AJenbo
Copy link
Member

AJenbo commented Oct 21, 2024

Am I right in thinking that it would fail to equip the two hand sword in this case?
image

@ephphatha
Copy link
Contributor Author

Yes, that would fail in this PR as it attempts to unequip the right hand item (in this case a 2x3 shield) before lifting the left hand item to holdItem. I didn't want to try get too clever by searching for matching items in that case but it could be handled better.

specifically where the larger item is in the right hand, this would fail previously in certain circumstances.
@ephphatha
Copy link
Contributor Author

2024-10-21T205524_devilutionx.mp4

@AJenbo AJenbo merged commit 5758e56 into diasurgical:master Oct 21, 2024
23 checks passed
@ephphatha ephphatha deleted the item/swap branch October 21, 2024 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants