Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions data/items/items.xml
Original file line number Diff line number Diff line change
Expand Up @@ -53153,6 +53153,15 @@ hands of its owner. Granted by TibiaRoyal.com"/>
</item>
<item id="28478" article="an" name="umbral master bow TEST">
<attribute key="primarytype" value="distance weapons"/>
<attribute key="slotType" value="two-handed" />
<attribute key="ammotype" value="arrow" />
<attribute key="hitchance" value="5" />
<attribute key="range" value="7" />
<attribute key="attack" value="6" />
<attribute key="weight" value="4000" />
<attribute key="extraattack" value="3" /> <!-- x3 rows as extra attacks -->
<attribute key="extraattackdelay" value="500" /> <!-- 5ms of delay between attacks -->

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix the comment typo for delay timing.

The comment states "5ms of delay" but the value is 500. In OT servers, timing values are typically in milliseconds, so this should be "500ms of delay between attacks" not "5ms".

📝 Proposed fix
-		<attribute key="extraattackdelay" value="500" /> <!-- 5ms of delay between attacks -->
+		<attribute key="extraattackdelay" value="500" /> <!-- 500ms of delay between attacks -->
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<attribute key="extraattackdelay" value="500" /> <!-- 5ms of delay between attacks -->
<attribute key="extraattackdelay" value="500" /> <!-- 500ms of delay between attacks -->
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@data/items/items.xml` at line 53163, Update the inline comment next to the
attribute with key="extraattackdelay" value="500" to correctly state "500ms of
delay between attacks" (instead of "5ms"); locate the attribute element for
extraattackdelay and edit only the comment text to reflect the correct
millisecond value.

<attribute key="extraattackchance" value="500" /> <!-- 5% chance to trigger auto attacks -->

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify the percentage calculation basis in the comment.

The comment states "5% chance" for a value of 500, which suggests the system uses a base of 10,000 for percentage calculations (500/10000 = 5%). While this is a common pattern in OT servers, the comment doesn't clarify this encoding, which could confuse developers trying to configure different chance values.

📝 Proposed fix
-		<attribute key="extraattackchance" value="500" /> <!-- 5% chance to trigger auto attacks -->
+		<attribute key="extraattackchance" value="500" /> <!-- 5% chance to trigger auto attacks (500/10000) -->
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<attribute key="extraattackchance" value="500" /> <!-- 5% chance to trigger auto attacks -->
<attribute key="extraattackchance" value="500" /> <!-- 5% chance to trigger auto attacks (500/10000) -->
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@data/items/items.xml` at line 53164, Update the inline comment for the
attribute key "extraattackchance" to state the encoding basis: explain that the
value is in hundredths of a percent (basis points) out of 10,000 (e.g., 500 =
500/10000 = 5%), so future editors understand how to compute percentages; modify
the comment on the line with <attribute key="extraattackchance" ... /> to
reflect this clarification.

<attribute key="script" value="moveevent;weapon">
<attribute key="weaponType" value="distance"/>
<attribute key="slot" value="hand"/>
Expand Down
151 changes: 151 additions & 0 deletions src/creatures/combat/combat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1574,6 +1574,157 @@
g_game().sendSingleSoundEffect(target->getPosition(), params.soundCastEffect, caster);
}
}

for (int32_t i = 1; i <= extraAttacks; ++i) {

Check failure on line 1578 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-debug)

‘extraAttacks’ was not declared in this scope

Check failure on line 1578 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Windows / Compile (CMake)

'extraAttacks': undeclared identifier

Check failure on line 1578 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - macOS / Compile (macOS)

use of undeclared identifier 'extraAttacks'

Check failure on line 1578 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-release)

‘extraAttacks’ was not declared in this scope
auto eventId = g_dispatcher().scheduleEvent(
i * extraDelay,

Check failure on line 1580 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-debug)

‘extraDelay’ was not declared in this scope

Check failure on line 1580 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Windows / Compile (CMake)

'extraDelay': undeclared identifier

Check failure on line 1580 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-release)

‘extraDelay’ was not declared in this scope
[caster, target, combatType, combatOrigin, distEffect, weaponRange]() {

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-debug)

‘weaponRange’ was not declared in this scope

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-debug)

‘distEffect’ was not declared in this scope; did you mean ‘Effect’?

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-debug)

‘combatOrigin’ was not declared in this scope; did you mean ‘CombatOrigin’?

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-debug)

‘combatType’ was not declared in this scope; did you mean ‘CombatType_t’?

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Windows / Compile (CMake)

'weaponRange': lambda capture variable not found

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Windows / Compile (CMake)

'distEffect': lambda capture variable not found

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Windows / Compile (CMake)

'combatOrigin': lambda capture variable not found

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Windows / Compile (CMake)

'combatType': lambda capture variable not found

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - macOS / Compile (macOS)

use of undeclared identifier 'weaponRange'

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - macOS / Compile (macOS)

use of undeclared identifier 'distEffect'

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - macOS / Compile (macOS)

use of undeclared identifier 'combatOrigin'

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - macOS / Compile (macOS)

use of undeclared identifier 'combatType'

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-release)

‘weaponRange’ was not declared in this scope

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-release)

‘distEffect’ was not declared in this scope; did you mean ‘Effect’?

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-release)

‘combatOrigin’ was not declared in this scope; did you mean ‘CombatOrigin’?

Check failure on line 1581 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-release)

‘combatType’ was not declared in this scope; did you mean ‘CombatType_t’?
Comment on lines +1578 to +1581

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Build failure: Missing variable declarations and setup block.

The code uses extraAttacks, extraDelay, combatType, combatOrigin, distEffect, and weaponRange without declaring them. The entire setup block before the extra attack loop is missing.

Based on the feature requirements and past review suggestions, this block should:

  1. Guard against recursive extra attacks using damage.isExtraAttack
  2. Retrieve the player and weapon from the caster
  3. Extract and clamp extra attack parameters
  4. Roll for extra attack chance before scheduling
🔧 Proposed fix: Add missing setup block before the loop
+	// Extra attack scheduling - skip if this is already an extra attack to prevent recursion
+	if (damage.isExtraAttack) {
+		return;
+	}
+
+	const auto &player = caster ? caster->getPlayer() : nullptr;
+	if (!player) {
+		return;
+	}
+
+	const auto &weapon = player->getWeapon(true);
+	if (!weapon || weapon->getExtraAttack() <= 0) {
+		return;
+	}
+
+	const int32_t extraAttacks = std::clamp<int32_t>(weapon->getExtraAttack(), 0, 10);
+	const int32_t extraDelay = std::max<int32_t>(0, weapon->getExtraAttackDelay());
+	const int32_t extraAttackChance = std::clamp<int32_t>(weapon->getExtraAttackChance(), 0, 10000);
+
+	if (extraAttackChance > 0 && uniform_random(1, 10000) > extraAttackChance) {
+		return;
+	}
+
+	const CombatType_t combatType = params.combatType;
+	const CombatOrigin combatOrigin = params.origin;
+	const uint16_t distEffect = params.distanceEffect;
+	const uint8_t weaponRange = weapon->getShootRange() > 0 ? weapon->getShootRange() : Item::items[weapon->getID()].shootRange;
+
 	for (int32_t i = 1; i <= extraAttacks; ++i) {
 		auto eventId = g_dispatcher().scheduleEvent(
 			i * extraDelay,
-			[caster, target, combatType, combatOrigin, distEffect, weaponRange]() {
+			[caster, target, combatType, combatOrigin, distEffect, weaponRange]() {
🧰 Tools
🪛 GitHub Check: Build - Linux / Compile (linux-debug)

[failure] 1581-1581:
‘weaponRange’ was not declared in this scope


[failure] 1581-1581:
‘distEffect’ was not declared in this scope; did you mean ‘Effect’?


[failure] 1581-1581:
‘combatOrigin’ was not declared in this scope; did you mean ‘CombatOrigin’?


[failure] 1581-1581:
‘combatType’ was not declared in this scope; did you mean ‘CombatType_t’?


[failure] 1580-1580:
‘extraDelay’ was not declared in this scope


[failure] 1578-1578:
‘extraAttacks’ was not declared in this scope

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/creatures/combat/combat.cpp` around lines 1578 - 1581, Add the missing
setup block before the extra-attack loop: first check and return if
damage.isExtraAttack to guard against recursion, then obtain the player and
weapon from caster (use the same accessor functions used elsewhere in
combat.cpp), compute extraAttacks and extraDelay by extracting and clamping
their values to allowed ranges, determine combatType, combatOrigin, distEffect
and weaponRange from the weapon/player state, and perform the extra-attack
chance roll (bail out if the roll fails) before entering the loop that calls
g_dispatcher().scheduleEvent; ensure the scheduled lambda still captures the
computed local variables (extraAttacks, extraDelay, combatType, combatOrigin,
distEffect, weaponRange) so they are in scope for the loop.

if (!caster || !target) {
return;
}

if (caster->isRemoved() || !caster->isAlive()) {
return;
}

if (target->isRemoved() || !target->isAlive()) {
return;
}

const auto &player = caster->getPlayer();
if (!player) {
return;
}

const Position currentCasterPos = caster->getPosition();
const Position targetPos = target->getPosition();

if (currentCasterPos.z != targetPos.z) {
return;
}

const uint32_t currentDist = std::max<uint32_t>(
Position::getDistanceX(currentCasterPos, targetPos),
Position::getDistanceY(currentCasterPos, targetPos)
);

auto currentTool = player->getWeapon();
if (!currentTool) {
return;
}

const auto &currentBow = player->getWeapon(true);
if (currentTool->getWeaponType() == WEAPON_AMMO && !currentBow) {
return;
}

uint8_t currentWeaponRange = weaponRange;

Check failure on line 1621 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-debug)

‘weaponRange’ is not captured

Check failure on line 1621 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Windows / Compile (CMake)

'weaponRange': undeclared identifier

Check failure on line 1621 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - macOS / Compile (macOS)

use of undeclared identifier 'weaponRange'

Check failure on line 1621 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-release)

‘weaponRange’ is not captured
if (currentBow && currentBow->getShootRange() > 0) {
currentWeaponRange = currentBow->getShootRange();
} else if (currentBow) {
currentWeaponRange = Item::items[currentBow->getID()].shootRange;
}

if (currentDist > currentWeaponRange) {
return;
}

if (!g_game().isSightClear(currentCasterPos, targetPos, true)) {
return;
}

std::shared_ptr<Item> currentAmmo = nullptr;
WeaponShared_ptr weaponToUse = nullptr;
std::shared_ptr<Item> ammoToRemove = nullptr;

if (currentTool->getWeaponType() == WEAPON_AMMO) {

currentAmmo = currentTool;
ammoToRemove = currentTool;

if (currentAmmo->getItemCount() < 1) {
return;
}

if (currentBow) {
weaponToUse = g_weapons().getWeapon(currentBow);
}

if (!weaponToUse) {
weaponToUse = g_weapons().getWeapon(currentTool);
}
} else {

weaponToUse = g_weapons().getWeapon(currentTool);
}

if (!weaponToUse) {
return;
}

std::shared_ptr<Item> damageTool = currentAmmo ? currentAmmo : currentTool;
if (!damageTool) {
return;
}

if (damageTool->getID() == 0) {
return;
}

uint16_t shootEffect = distEffect;

Check failure on line 1674 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-debug)

‘distEffect’ is not captured

Check failure on line 1674 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Windows / Compile (CMake)

'distEffect': undeclared identifier

Check failure on line 1674 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - macOS / Compile (macOS)

use of undeclared identifier 'distEffect'

Check failure on line 1674 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-release)

‘distEffect’ is not captured
if (shootEffect == CONST_ANI_NONE) {
const ItemType &toolType = Item::items[damageTool->getID()];
shootEffect = toolType.shootType;
}

if (shootEffect != CONST_ANI_NONE) {
g_game().addDistanceEffect(currentCasterPos, targetPos, shootEffect);
}

CombatParams extraParams;
extraParams.combatType = combatType;

Check failure on line 1685 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-debug)

‘combatType’ is not captured

Check failure on line 1685 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Windows / Compile (CMake)

'combatType': undeclared identifier

Check failure on line 1685 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - macOS / Compile (macOS)

use of undeclared identifier 'combatType'

Check failure on line 1685 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-release)

‘combatType’ is not captured
extraParams.origin = combatOrigin;

Check failure on line 1686 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-debug)

‘combatOrigin’ is not captured

Check failure on line 1686 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Windows / Compile (CMake)

'combatOrigin': undeclared identifier

Check failure on line 1686 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - macOS / Compile (macOS)

use of undeclared identifier 'combatOrigin'

Check failure on line 1686 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - Linux / Compile (linux-release)

‘combatOrigin’ is not captured
extraParams.blockedByArmor = true;
extraParams.blockedByShield = true;
extraParams.aggressive = true;

int32_t weaponDamage = 0;
try {
weaponDamage = std::abs(weaponToUse->getWeaponDamage(player, target, damageTool, true));
} catch (...) {

return;
}
Comment on lines +1692 to +1697

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Silent exception swallowing hides potential bugs.

The catch block silently returns without logging. If getWeaponDamage throws, you'll have no indication of what went wrong during debugging or in production logs.

🛠️ Proposed fix: Add error logging
 				try {
 					weaponDamage = std::abs(weaponToUse->getWeaponDamage(player, target, damageTool, true));
 				} catch (...) {
-
+					g_logger().error("[Combat::extraAttack] Exception in getWeaponDamage for player {}", player->getName());
 					return;
 				}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/creatures/combat/combat.cpp` around lines 1692 - 1697, The catch-all
around weaponToUse->getWeaponDamage(player, target, damageTool, true) silently
returns and hides errors; replace the empty catch with logging the exception and
context before returning (capture std::exception_ptr, rethrow to extract
std::exception::what() if possible, or log that an unknown exception occurred)
and include identifiers like weaponToUse, player, target, damageTool and
variable weaponDamage in the log so you can trace failures to getWeaponDamage.


CombatDamage extraDamage;
extraDamage.primary.type = combatType;

Check failure on line 1700 in src/creatures/combat/combat.cpp

View workflow job for this annotation

GitHub Actions / Build - macOS / Compile (macOS)

use of undeclared identifier 'combatType'
extraDamage.primary.value = normal_random(0, static_cast<int32_t>(weaponDamage));
extraDamage.secondary.type = weaponToUse->getElementType();
extraDamage.secondary.value = weaponToUse->getElementDamage(player, target, damageTool);
extraDamage.origin = combatOrigin;
extraDamage.isExtraAttack = true;

Combat::doCombatHealth(caster, target, currentCasterPos, extraDamage, extraParams);

if (ammoToRemove && g_configManager().getBoolean(REMOVE_WEAPON_AMMO)) {

if (ammoToRemove->getID() != 0) {
const uint16_t ammoCount = ammoToRemove->getItemCount();
if (ammoCount > 1) {
auto transformed = g_game().transformItem(ammoToRemove, ammoToRemove->getID(), ammoCount - 1);
(void)transformed;
} else if (ammoCount == 1) {
auto removed = g_game().internalRemoveItem(ammoToRemove);
(void)removed;
}
player->updateSupplyTracker(ammoToRemove);
}
}
Comment on lines +1709 to +1722

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Supply tracker called after item may be invalid.

updateSupplyTracker is called after the item has been transformed or removed. If internalRemoveItem succeeds, the ammoToRemove shared_ptr may reference an item no longer in the game world. While the shared_ptr keeps the object alive, updateSupplyTracker sends data to the client and party which expects a valid in-game item.

Consider calling the tracker before modification, or capturing the item ID/count beforehand.

🔧 Proposed fix: Track supply before modifying ammo
 				if (ammoToRemove && g_configManager().getBoolean(REMOVE_WEAPON_AMMO)) {
-
 					if (ammoToRemove->getID() != 0) {
+						// Track supply before modifying the item
+						player->updateSupplyTracker(ammoToRemove);
+
 						const uint16_t ammoCount = ammoToRemove->getItemCount();
 						if (ammoCount > 1) {
 							auto transformed = g_game().transformItem(ammoToRemove, ammoToRemove->getID(), ammoCount - 1);
 							(void)transformed;
 						} else if (ammoCount == 1) {
 							auto removed = g_game().internalRemoveItem(ammoToRemove);
 							(void)removed;
 						}
-						player->updateSupplyTracker(ammoToRemove);
 					}
 				}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (ammoToRemove && g_configManager().getBoolean(REMOVE_WEAPON_AMMO)) {
if (ammoToRemove->getID() != 0) {
const uint16_t ammoCount = ammoToRemove->getItemCount();
if (ammoCount > 1) {
auto transformed = g_game().transformItem(ammoToRemove, ammoToRemove->getID(), ammoCount - 1);
(void)transformed;
} else if (ammoCount == 1) {
auto removed = g_game().internalRemoveItem(ammoToRemove);
(void)removed;
}
player->updateSupplyTracker(ammoToRemove);
}
}
if (ammoToRemove && g_configManager().getBoolean(REMOVE_WEAPON_AMMO)) {
if (ammoToRemove->getID() != 0) {
// Track supply before modifying the item
player->updateSupplyTracker(ammoToRemove);
const uint16_t ammoCount = ammoToRemove->getItemCount();
if (ammoCount > 1) {
auto transformed = g_game().transformItem(ammoToRemove, ammoToRemove->getID(), ammoCount - 1);
(void)transformed;
} else if (ammoCount == 1) {
auto removed = g_game().internalRemoveItem(ammoToRemove);
(void)removed;
}
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/creatures/combat/combat.cpp` around lines 1709 - 1722, The call to
player->updateSupplyTracker(ammoToRemove) happens after g_game().transformItem
or g_game().internalRemoveItem may modify/remove the item, risking sending
invalid item data; fix by recording the needed supply info (e.g.,
ammoToRemove->getID() and ammoToRemove->getItemCount() or any supply-identifier)
or calling player->updateSupplyTracker with that captured info before performing
g_game().transformItem/ internalRemoveItem; update the logic around
ammoToRemove, g_game().transformItem, g_game().internalRemoveItem and
player->updateSupplyTracker so the tracker receives valid pre-modification data.

},
"Combat::extraAttack"
);
(void)eventId;
}
}

void Combat::doCombatHealth(const std::shared_ptr<Creature> &caster, const Position &position, const std::unique_ptr<AreaCombat> &area, CombatDamage &damage, const CombatParams &params) {
Expand Down
1 change: 1 addition & 0 deletions src/creatures/creatures_definitions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1725,6 +1725,7 @@ struct CombatDamage {

CombatOrigin origin = ORIGIN_NONE;
bool critical = false;
bool isExtraAttack = false;
int affected = 1;
bool extension = false;
std::string exString;
Expand Down
1 change: 1 addition & 0 deletions src/items/functions/item/item_parse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ void ItemParse::initParse(const std::string &stringValue, pugi::xml_node attribu
ItemParse::parseDefense(stringValue, valueAttribute, itemType);
ItemParse::parseExtraDefense(stringValue, valueAttribute, itemType);
ItemParse::parseAttack(stringValue, valueAttribute, itemType);
ItemParse::parseExtraAttack(stringValue, valueAttribute, itemType);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The function ItemParse::parseExtraAttack is declared in item_parse.hpp and called here, but its implementation is completely missing from item_parse.cpp. This will result in a compilation error (undefined reference).

Please add the implementation of parseExtraAttack to item_parse.cpp:

void ItemParse::parseExtraAttack(const std::string &stringValue, pugi::xml_attribute valueAttribute, ItemType &itemType) {
	if (stringValue == "extraattack") {
		itemType.extraAttack = pugi::cast<int32_t>(valueAttribute.value());
	} else if (stringValue == "extraattackdelay") {
		itemType.extraAttackDelay = pugi::cast<int32_t>(valueAttribute.value());
	} else if (stringValue == "extraattackchance") {
		itemType.extraAttackChance = pugi::cast<int32_t>(valueAttribute.value());
	}
}

Additionally, make sure to map these attributes in ItemParseAttributesMap inside item_parse.hpp to keep the attribute map synchronized.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

parseExtraAttack is called but not defined (linker break).

ItemParse::initParse invokes parseExtraAttack(...) on Line 32, but this translation unit has no implementation for it.

💡 Proposed fix
+void ItemParse::parseExtraAttack(const std::string &stringValue, pugi::xml_attribute valueAttribute, ItemType &itemType) {
+	if (stringValue == "extraattack") {
+		itemType.extraAttack = std::max<int32_t>(0, pugi::cast<int32_t>(valueAttribute.value()));
+	} else if (stringValue == "extraattackdelay") {
+		itemType.extraAttackDelay = std::max<int32_t>(0, pugi::cast<int32_t>(valueAttribute.value()));
+	} else if (stringValue == "extraattackchance") {
+		itemType.extraAttackChance = std::clamp<int32_t>(pugi::cast<int32_t>(valueAttribute.value()), 0, 10000);
+	}
+}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/items/functions/item/item_parse.cpp` at line 32, ItemParse::initParse
calls ItemParse::parseExtraAttack(stringValue, valueAttribute, itemType) but
there is no implementation for ItemParse::parseExtraAttack in this translation
unit, causing a linker error; add a matching definition for
ItemParse::parseExtraAttack with the same signature (parameters and
namespace/class scope) or remove the call if unused. Locate the declaration
(likely in item_parse.h) and implement the function in item_parse.cpp (or the
appropriate .cpp) with the expected behavior, ensuring the function signature
exactly matches the declaration (including const/ref qualifiers) so the linker
can resolve it; if the logic belongs elsewhere, forward the call to the correct
function or update initParse to use the existing implementation.

ItemParse::parseMantra(stringValue, valueAttribute, itemType);
ItemParse::parseRotateTo(stringValue, valueAttribute, itemType);
ItemParse::parseWrapContainer(stringValue, valueAttribute, itemType);
Expand Down
1 change: 1 addition & 0 deletions src/items/functions/item/item_parse.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ class ItemParse : public Items {
static void parseHealthAndMana(const std::string &stringValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
static void parseSkills(const std::string &stringValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
static void parseCriticalHit(const std::string &stringValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
static void parseExtraAttack(const std::string &stringValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
static void parseLifeAndManaLeech(const std::string &stringValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
static void parseMaxHitAndManaPoints(const std::string &stringValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
static void parseMagicLevelPoint(const std::string &stringValue, pugi::xml_attribute valueAttribute, ItemType &itemType);
Expand Down
12 changes: 12 additions & 0 deletions src/items/item.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,18 @@ class Item : virtual public Thing, public ItemProperties, public SharedObject {
return items[id].mantra;
}

int32_t getExtraAttack() const {
return items[id].extraAttack;
}

int32_t getExtraAttackDelay() const {
return items[id].extraAttackDelay;
}

int32_t getExtraAttackChance() const {
return items[id].extraAttackChance;
}

uint32_t getWorth() const;
uint32_t getForgeSlivers() const;
uint32_t getForgeCores() const;
Expand Down
4 changes: 4 additions & 0 deletions src/items/items.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,10 @@ class ItemType {
bool isWrapKit = false;
bool m_canBeUsedByGuests = false;

int32_t extraAttack = 0;
int32_t extraAttackDelay = 500;
int32_t extraAttackChance = 0;

// Monk
bool isDualWielding = false;
bool m_isMagicShieldPotion = false;
Expand Down
3 changes: 3 additions & 0 deletions src/items/items_definitions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,9 @@ enum ItemParseAttributes_t {
ITEM_PARSE_SKILLFIST,
ITEM_PARSE_CRITICALHITCHANCE,
ITEM_PARSE_CRITICALHITDAMAGE,
ITEM_PARSE_EXTRAATTACK,
ITEM_PARSE_EXTRAATTACKDELAY,
ITEM_PARSE_EXTRAATTACKCHANCE,
ITEM_PARSE_LIFELEECHCHANCE,
ITEM_PARSE_LIFELEECHAMOUNT,
ITEM_PARSE_MANALEECHCHANCE,
Expand Down
Loading