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: ActionBar packets (sendEquipItem) + Tier in actionbar #1073

Merged
merged 7 commits into from
Jan 28, 2025

Conversation

kokekanon
Copy link
Collaborator

@kokekanon kokekanon commented Jan 24, 2025

Description

The function sendEquipItem (which only affects game_actionbar) was sending 2 packets:
according OTCR
u16 the itemId
u8 or u16 countOrSubType
But this is incorrect.
According to Canary, the 2 packets to send are:

u16 the itemId
u8 tier.
This error manifests when trying to equip an item with a tier through the action bar.
The client sends the itemId and count (1).
For example, attempting to equip an Helmet tier +5.
The server interprets itemId and tier = 1(count), failing to find the item.
As a result, it doesn't work.

Canary 13.40

2 packets:
U16 item ID
u8 Tier

https://github.com/opentibiabr/canary/blob/5781de718e5d22ce9e7f136151d8b4032c85843d/src/server/network/protocol/protocolgame.cpp#L1392-L1400

image

Tfs 10.98 blacktek

only packet U16 Item ID ( no have this feature GameThingUpgradeClassification)
https://github.com/Black-Tek/BlackTek-Server/blob/7130b41899fcb643b582e78a52ff27e891110d54/src/protocolgame.cpp#L983-L989

image

tfs forgottenserver-optimized SaiyansKing

only packet U16 Item ID ( no have this feature GameThingUpgradeClassification)

https://github.com/mehah/forgottenserver-optimized/blob/148baaa48c8ca159b16d0b632bd813749399b013/src/protocolgame.cpp#L1256-L1262

image

tfs 8.6

does not have this function

tfs 13.10

13.10 does not have many features, this is one of them

https://github.com/otland/forgottenserver/blob/b227678ab3584b8349913abfe4529292678a3fdf/src/protocolgame.cpp#L1300-L1308
image

Behavior

Actual

cvcaca

Expected

avavaaxc

Fixes

usser in discord

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested

  • login
  • logout
  • item with and without tier
  • clear item
  • 13.40 , 8.6 , 13.10, 10.98

Test Configuration:

  • Server Version: 13.40 , 8.6 , 13.10, 10.98
  • Client: this pr
  • Operating System: win 11

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I checked the PR checks reports
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • tier :

image

image

@kokekanon kokekanon changed the title fix: action bar packets sendEquipItem fix: ActionBar packets (sendEquipItem) + Tier in actionbar Jan 24, 2025
@Zbizu
Copy link
Contributor

Zbizu commented Jan 24, 2025

As far as I'm aware, this feature was introduced together with the exaltation forge so it can be enabled since 12.80 (or generally be a part of "forge packets" feature)

13.20 private repo

void ProtocolGame::parseEquipObject(NetworkMessage& msg)
{
	// action bar/hotkey equip
	uint16_t clientId = msg.get<uint16_t>();
	const ItemType& it = g_items.legacyGetItemTypeByClientId(clientId);
	if (it.id == 0) {
		return;
	}

	uint8_t tier = 0;
	if (it.classification > 0) {
		tier = msg.getByte();
	}

	g_gameThread.addTask(createTask((DISPATCHER_TASK_EXPIRATION, [=, playerID = player->getID(), isTiered = it.classification > 0]() { g_game.playerEquipItem(playerID, clientId, isTiered, tier); })));
}

@kokekanon kokekanon marked this pull request as draft January 24, 2025 05:13
@kokekanon kokekanon marked this pull request as ready for review January 24, 2025 19:11
else
msg->addU8(static_cast<uint8_t>(countOrSubType));
if (g_game.getFeature(Otc::GameThingUpgradeClassification))
msg->addU8(tier);
Copy link
Contributor

@Zbizu Zbizu Jan 25, 2025

Choose a reason for hiding this comment

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

while there are no items that have both: "cloth" and "liquid pool"/"liquid container" flags together in the vanilla client, the client is coded to send TIER or FLUID CLIENT ID in the protocol (this applies for both "equip" and "use" actions)

in short: the client should send either TIER or FLUID CLIENT ID in hotkey equip/use
(I've also tried combining classification with liquid container, but qt client refuses to load assets when both flags are present)

video (qt client, I added "cloth" flag with "arrow" slot to vials and set "equip" action on them):

2025-01-25.13-38-58.mp4

Copy link
Collaborator Author

@kokekanon kokekanon Jan 25, 2025

Choose a reason for hiding this comment

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

Im an 8.6 player, not using versions 13, my knowledge is limited, which item allows to be equip / unequip and have getCountOrSubType ? in default assets

image
image
image
image

@Zbizu
Copy link
Contributor

Zbizu commented Jan 25, 2025

equipping items works now
fluids are still broken (in fact they are at item assignment part)
after a few clicks in the action bar, multiuse stops working (this might be because I don't have any vials with this subtype in my inventory anymore)

2025-01-25.15-20-14.mp4

relogging changes fluid subtype

2025-01-25.15-21-15.mp4

@kokekanon
Copy link
Collaborator Author

when I made the modification, I didn't realize that tibiaQT doesn't show the tiers in the actionbar, ups me bad.
I don't use tibia 13, but I think it's better

@kokekanon
Copy link
Collaborator Author

kokekanon commented Jan 25, 2025

equipping items works now fluids are still broken (in fact they are at item assignment part) after a few clicks in the action bar, multiuse stops working (this might be because I don't have any vials with this subtype in my inventory anymore)

the bug I see is :

  • the count is not displayed in action bar UI
  • the item is not updated, when it reaches 0
  • the bug you mention applies only to vial (does not apply to other Count items )
  • several bugs in "assign object" depending on the item chosen .

for me, it needs to be done from 0🤣 A 6-year-old module was not designed for Tibia 13.
but the pr was only intended to fix the equip/unequip for tie items (Currently, in the main repo, if you equipped a tier item, it was not equipped.")

@Zbizu
Copy link
Contributor

Zbizu commented Jan 25, 2025

when I made the modification, I didn't realize that tibiaQT doesn't show the tiers in the actionbar, ups me bad. I don't use tibia 13, but I think it's better

Tibia 14 has this fixed if I remember correctly

@mehah mehah merged commit 90b4062 into mehah:main Jan 28, 2025
9 of 12 checks passed
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.

4 participants