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: onDeEquip properly handled at logout/death #2625

Merged
merged 4 commits into from
May 24, 2024

Conversation

luanluciano93
Copy link
Contributor

We didn't deequip items on logout/death but we equip everytime the player login.
That way onDeEquip didn't work correctly as intended.
following example code shows what the problem is:

local test = MoveEvent()

function test.onEquip(player, item, slot, isCheck)
	player:setMaxHealth(player:getMaxHealth() + 100)
	return true
end

test:type("equip")
test:slot("ring")
test:id(9393)
test:level(100)
test:register()

function test.onDeEquip(player, item, slot, isCheck)
	player:setMaxHealth(player:getMaxHealth() - 100)
	return true
end

test:type("deequip")
test:slot("ring")
test:id(9393)
test:level(100)
test:register()

The above would have resulted on every logout/login that the HP would increase by 100 but not decrease on logout/death
which is now fixed

credits: EvilHero90 (based TFS PR)

Copy link
Contributor

@phacUFPE phacUFPE left a comment

Choose a reason for hiding this comment

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

I'm leaving a review that blocks the merge because it has an issue in the code that crash the builds. I'm leaving the solution, but GitHub Actions also complained about it.

src/creatures/players/player.cpp Outdated Show resolved Hide resolved
luanluciano93 and others added 2 commits May 13, 2024 03:36
Co-authored-by: Pedro Cruz <phac@cin.ufpe.br>
@elsongabriel elsongabriel merged commit 5f88c6d into opentibiabr:main May 24, 2024
21 checks passed
@luanluciano93 luanluciano93 deleted the deequip-fix branch May 24, 2024 15:03
libergod added a commit to libergod/canary that referenced this pull request May 26, 2024
libergod added a commit to libergod/canary that referenced this pull request Jun 2, 2024
@rafalolszewski94
Copy link

This still doesn't fix the issue.

@rafalolszewski94
Copy link

It should be

g_moveEvents().onPlayerEquip(getPlayer(), item, static_cast<Slots_t>(slot), false);

instead of

g_moveEvents().onPlayerDeEquip(getPlayer(), item, static_cast<Slots_t>(slot));

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

6 participants