creature:removeCondition doesn't work properly with userdata

[Erza]

Before creating an issue, please ensure:

• [x] This is a bug in the software that resides in this repository, and not a support matter (use https://otland.net/forums/support.16/ for support)
• [x] This issue is reproducible without changes to the C++ code in this repository

Steps to reproduce (include any configuration/script required to reproduce)

1. Add the following test code at data/scripts/talkactions/condition.lua

   
   local condition = Condition(CONDITION_HASTE)
   condition:setParameter(CONDITION_PARAM_TICKS, 10000)
   condition:setFormula(0.3, -12, 0.3, -12)

local talk = TalkAction("/test") function talk.onSay(player, words, param) if player:hasCondition(CONDITION_HASTE) then print("remove") player:removeCondition(condition) else print("add") player:addCondition(condition) end return false end

talk:register()

2. Run the `/test` command twice in game within 10 seconds, the condition should be added on the first execution and then removed on the second

### Expected behaviour
The haste condition should be added and removed respectively

### Actual behaviour
The haste condition is only added, but removing fails, even though `remove` is being printed in the console

### Related commits
https://github.com/otland/forgottenserver/commit/923f448f402392ee0c27632326da901947a724a8
https://github.com/otland/forgottenserver/commit/5d946f178e3f9e1ba48ba96e47ca7291bc4b6a4c

### Problematic code
It fails to find the condition with `std::find` in `Creature::removeCondition`
```cpp
void Creature::removeCondition(Condition* condition, bool force/* = false*/)
{
    auto it = std::find(conditions.begin(), conditions.end(), condition);
    if (it == conditions.end()) {
        return;
    }

[nekiro]

But thats your lua code logic problem, you have to use "creature:getCondition()" to pass it to removeCondition to remove proper condtion. Ofc it won't remove proper condition if you are passing a condition that is cloned. If you remove by type and all that, then you do it instead of passing userdata. You basically implemented removing by type from userdata, it makes no sense.

Tldr; works as intended to me, not a bug

[Erza]

I see, makes sense what you're saying.

You basically implemented removing by type from userdata, it makes no sense.

I think both ways have a right to exist and should work just fine, with the PR that is the case, with the current implementation it's not

local condition = Condition(CONDITION_HASTE)
condition:setParameter(CONDITION_PARAM_TICKS, 10000)
condition:setParameter(CONDITION_PARAM_SUBID, 2)
condition:setFormula(0.3, -12, 0.3, -12)

local talk = TalkAction("/test")
function talk.onSay(player, words, param)
    if player:hasCondition(CONDITION_HASTE) then
        print("remove")
        player:removeCondition(condition)
    else
        print("add")
        player:addCondition(condition)
    end
    return false
end

talk:register()

local conditionType = CONDITION_HASTE
local subId = 2

local condition = Condition(conditionType)
condition:setParameter(CONDITION_PARAM_TICKS, 10000)
condition:setParameter(CONDITION_PARAM_SUBID, subId)
condition:setFormula(0.3, -12, 0.3, -12)

local talk = TalkAction("/test")
function talk.onSay(player, words, param)
    local playerCondition = player:getCondition(conditionType, CONDITIONID_COMBAT, subId)
    if playerCondition then
        print("remove")
        player:removeCondition(playerCondition)
    else
        print("add")
        player:addCondition(condition)
    end
    return false
end

talk:register()

Imagine something like

local condition = Condition(CONDITION_HASTE)
condition:setParameter(CONDITION_PARAM_TICKS, 10000)
condition:setParameter(CONDITION_PARAM_SUBID, 2)
condition:setFormula(0.3, -12, 0.3, -12)

local players = game.getSpectators(player:getPosition(), false, true, 10, 10, 10, 10)
for _, player in pairs(players) do
    player:addCondition(condition)
end

....

for _, player in pairs(players) do
     player:removeCondition(condition)
end

We already know all of the condition's parameters already, just let the engine fetch the correct one from the creature instead of having to bother with that in Lua. Look at it this way: Why shouldn't you be able to remove the same condition that you've added?

[nekiro]

IMO this is complicating the code for no reason, so I'm against this change, you can wait for what the others devs think though. It's also illogical to me to pass random userdata thinking it will remove the one condition that you want it to remove.

[Codinablack]

In my opinion, we have a proper way to remove the condition already and there is not a reason to complicate code and possibly introduce bugs where none exist (not at all implying anything about the code in question), when the functionality that is being added is easily achievable in a simple lua function and does not require a source edit to make it possible to do

player:removeCondition(condition)

Can we reach a consensus on this one, its been open awhile.