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

Fix buy gump item names #1436

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

brndd
Copy link
Contributor

@brndd brndd commented Oct 31, 2021

Some servers send the names of items in the buy gump in 0xD6 MegaCliloc packets before opening the buy gump. ClassicUO was deleting and then recreating any existing items in the packet handler for the 0x3C UpdateContainedItems packet, which was causing the names sent in the MegaCliloc packets to be lost. This would result in a bug where opening a buy gump, closing it, and then immediately opening it again would display incorrect item names.

This PR fixes that, but it may have some side effects because I am honestly not sure why we were recreating these items in the first place. It seemed pointless but perhaps it serves some unknown purpose.

@andreakarasho
Copy link
Collaborator

It's necessary to delete items to avoid data duplications or fake items.
If i remember well i think 0xD6 [cliloc packet] is sent suddenly an item/container update.

@brndd
Copy link
Contributor Author

brndd commented Nov 1, 2021

The problem is that the 0xD6 packets are sent and processed before we delete the items, so the data contained in them gets deleted, resulting in incorrect item names. I think simply clearing the container (as done in this PR) should be enough to avoid fake or duplicate items, and all the items seem to get deleted permanently after the shopkeeper walks out of range or something anyway.

@andreakarasho
Copy link
Collaborator

i wonder if forcing a new Cliloc request will fix the issue

@brndd
Copy link
Contributor Author

brndd commented Nov 3, 2021

Then we'll end up sending a bunch of extra packets for IMO no good reason when the original client doesn't.

Is it not enough to just clear the container? The buy list is populated by items in the container, so we don't need to delete them from existence as long as we remove them from the container.

@brndd
Copy link
Contributor Author

brndd commented Sep 17, 2022

This is still an issue.

@andreakarasho andreakarasho changed the base branch from dev to main November 7, 2022 19:23
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.

None yet

2 participants